[e10s] Closing window with windowed plugin exists crashes the test plugin

NEW
Assigned to

Status

()

Core
Plug-ins
P3
normal
2 years ago
8 months ago

People

(Reporter: xidorn, Assigned: qdot)

Tracking

(Blocks: 6 bugs)

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
The test plugin crashes on "plug removed" error [1] on Linux when closing a window which has a windowed plugin inside on e10s.

It is observed when I'm working on bug 1191597. This issue causes a premafail on leakcheck because of subtest fullscreen-plugins.

[1] https://dxr.mozilla.org/mozilla-central/rev/e0bcd16e1d4b99ba3e542149d0d41e0f60c54b5c/dom/plugins/test/testplugin/nptest_gtk2.cpp#264
(Reporter)

Comment 1

2 years ago
Created attachment 8705428 [details] [diff] [review]
patch of testcase

This includes a patch of the testcase. If you run this test with --e10s, you'll see something similiar to this log when the test finishes:
[NPAPI 23492] ###!!! ASSERTION: plug removed: 'glib assertion', file /data/mozilla/central/toolkit/xre/nsSigHandlers.cpp, line 140
#01: g_logv (/lib/x86_64-linux-gnu/libglib-2.0.so.0)
#02: g_log (/lib/x86_64-linux-gnu/libglib-2.0.so.0)
#03: DeleteWidget (/data/mozilla/central/dom/plugins/test/testplugin/nptest_gtk2.cpp:264)
#04: gtk_marshal_VOID__UINT_STRING (/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0)
#05: g_closure_invoke (/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0)
#06: g_signal_handler_disconnect (/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0)
#07: g_signal_emit_valist (/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0)
#08: g_signal_emit (/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0)
#09: gtk_widget_translate_coordinates (/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0)
#10: gtk_plug_get_socket_window (/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0)
#11: gtk_print_backend_destroy (/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0)
#12: gdk_x11_pixmap_get_drawable_impl (/usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0)
#13: gdk_x11_screen_supports_net_wm_hint (/usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0)
#14: gdk_event_get_graphics_expose (/usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0)
#15: gdk_event_get_graphics_expose (/usr/lib/x86_64-linux-gnu/libgdk-x11-2.0.so.0)
#16: g_main_context_dispatch (/lib/x86_64-linux-gnu/libglib-2.0.so.0)
#17: g_main_context_dispatch (/lib/x86_64-linux-gnu/libglib-2.0.so.0)
#18: g_main_context_iteration (/lib/x86_64-linux-gnu/libglib-2.0.so.0)
#19: base::MessagePumpForUI::RunWithDispatcher(base::MessagePump::Delegate*, base::MessagePumpForUI::Dispatcher*) (/data/mozilla/central/ipc/chromium/src/base/message_pump_glib.cc:192)
#20: MessageLoop::RunInternal() (/data/mozilla/central/ipc/chromium/src/base/message_loop.cc:235)
#21: ~AutoRunState (/data/mozilla/central/ipc/chromium/src/base/message_loop.cc:520)
#22: XRE_InitChildProcess (/data/mozilla/central/toolkit/xre/nsEmbedFunctions.cpp:629)
#23: content_process_main(int, char**) (/data/mozilla/central/ipc/app/../contentproc/plugin-container.cpp:240)
#24: __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6)
#25: _start (/data/mozilla/central/obj-x86_64-unknown-linux-gnu/dist/bin/plugin-container)
#26: ??? (???:???)
** (plugin-container:23492): ERROR **: plug removed

and consequently:
TEST-UNEXPECTED-FAIL | leakcheck | plugin process: missing output line for total leaks!
TEST-INFO | leakcheck | missing output line from log file /tmp/tmp_ba7hj.mozrunner/runtests_leaks_plugin_pid23492.log
(Reporter)

Comment 2

2 years ago
FWIW, this issue can be workaround if you explicitly remove the plugin from the document before closing the window.
(Reporter)

Updated

2 years ago
tracking-e10s: --- → ?
(Reporter)

Comment 3

2 years ago
It could potentially be a bit more serious rather than a bug just affects testing. If any existing plugin still depends on the behavior of bug 485125, this bug could make the plugin crash unexpectedly.
Flags: needinfo?(jmathies)
(Reporter)

Comment 4

2 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #2)
> FWIW, this issue can be workaround if you explicitly remove the plugin from
> the document before closing the window.

Per bug 1239258, this workaround is not reliable enough. It just makes the permafail to be an intermittent one.
Blocks: 1239258
Sounds like re-parenting the plugin socket window (in the plugin host process) may  not be happening before the parent window is destroyed.
http://hg.mozilla.org/mozilla-central/rev/53728d1ce5eb#l1.45

Comment 6

2 years ago
I did a little investigating into this. Bug 485125 is a bit cryptic but from what I understand of the work that landed there it sounds like plugins might not like having the parent window (socket) destroyed before the plugin instance side of things (the plug) get torn down. The bug was filed back in 2009 against Flash, so there's a chance this is no longer an issue with whatever version of Flash linux makes use of today. Crashstats isn't much help here either as crash numbers for crashes involving libflashplayer.so are very low.

Plugin windows are managed by PluginWidgetParent and PluginWidgetChild (PluginWidget.ipdl) and can be torn down from either side, either by chrome when a tab is destroyed or by content when content unloads. From bug 485125 it sounds like we need to find a way keep the widget held by PluginWidgetParent which is parented to the browser alive for as long as the plugin instance is alive. Gah! I'm not sure how to do this but could spend some (more) time hacking on a potential fix. The test plugin test case here is reliable in reproducing the problem.

Anyway, this took me a while to get to, sorry about that. Clearing ni so this falls back into triage for discussion.
Flags: needinfo?(jmathies)

Updated

2 years ago
Blocks: 874016

Comment 7

2 years ago
Kyle is this something you could help with?
Flags: needinfo?(kyle)
I can give it a shot at some point but my backlog is growing pretty quick, heh.
Assignee: nobody → kyle
Flags: needinfo?(kyle)

Updated

2 years ago
tracking-e10s: ? → +
Priority: -- → P3
Blocks: 1253154

Updated

a year ago
Duplicate of this bug: 1253154
Blocks: 1309420
Blocks: 1331904
Blocks: 1330553
we would like to fix bug 1331904, but assume this is a root cause, is there anyone that can pick this bug up soon?

Comment 11

a year ago
I'm finishing patches for bug 1338172 now.
bsmedberg do you just want to take this bug then, since the fix will probably cascade from bug 1338172?
Flags: needinfo?(benjamin)

Comment 13

a year ago
Not really, but you could reassign it to nobody.
Flags: needinfo?(benjamin)
Eh, since there's a test case attached I'll hold onto it until bug 1338172 lands, make sure the test passes after that, then will land the test myself.
You need to log in before you can comment on or make changes to this bug.