Closed Bug 1237853 Opened 8 years ago Closed 3 years ago

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

Categories

(Core Graveyard :: Plug-ins, defect, P3)

defect

Tracking

(e10s+)

RESOLVED WONTFIX
Tracking Status
e10s + ---

People

(Reporter: xidorn, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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
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
FWIW, this issue can be workaround if you explicitly remove the plugin from the document before closing the window.
tracking-e10s: --- → ?
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)
(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
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)
Blocks: e10s-plugins
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)
Priority: -- → P3
Blocks: 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?
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)
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.
Just tried this test again. Passes locally, pushing to try at https://treeherder.mozilla.org/#/jobs?repo=try&revision=285e886e722719f096d2d9c9845a443180e450e9, if that passes, will land and close.
It looks like this test passes now, though I'm not sure how valid it is still. :xidorn, do you have any input? Seems like it should be landed if it's still testing things we use?
Flags: needinfo?(xidorn+moz)
I don't... I totally have no idea about the status of the related stuff.

There are questions to ask, e.g. do we still support plugins? Do we still support windowed plugins? If not, probably the test no longer makes any sense and we can just close this as WORKSFORME or INVALID.

If we still support that, then probably we can land and close it...
Flags: needinfo?(xidorn+moz)
Assignee: kyle → nobody
Resolving as wont fix, plugin support deprecated in Firefox 85.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: