Bug 147297

Summary: [GTK] NPAPI code crashes on Wayland
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, mcatanzaro, tpopela, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=1246710
https://bugzilla.redhat.com/show_bug.cgi?id=1246711
https://bugzilla.redhat.com/show_bug.cgi?id=1151769
https://bugzilla.redhat.com/show_bug.cgi?id=1248230
https://bugs.webkit.org/show_bug.cgi?id=164910
Bug Depends on:    
Bug Blocks: 81456    
Attachments:
Description Flags
Patch
none
Different patch zan: review+

Michael Catanzaro
Reported 2015-07-25 15:36:33 PDT
With ENABLE_WAYLAND_TARGET=ON, we crash on extensions.gnome.org (with the GNOME Shell browser plugin installed) in WebPageProxy::createPluginContainer, attempting to use GtkSocket, which is illegal because it can only be used with a GdkX11Display. I guess we need to disable NPAPI support entirely when running under Wayland, unless someone volunteers to write code to make this work.
Attachments
Patch (2.02 KB, patch)
2015-07-29 23:51 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Different patch (3.15 KB, patch)
2015-07-31 03:36 PDT, Carlos Garcia Campos 		zan: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2015-07-26 00:31:52 PDT
We used to disable NPAPI plugins when building the wayland target, I guess now we need to do a runtime check.
Michael Catanzaro
Comment 2 2015-07-29 18:09:59 PDT
My plan is to build without NPAPI support for F23+, unless somebody wants to fix this with runtime checks, but it seems there are very many places that need the checks. I don't care about X11 support in F23+.
Carlos Garcia Campos
Comment 3 2015-07-29 23:46:43 PDT
(In reply to comment #2) > My plan is to build without NPAPI support for F23+, unless somebody wants to > fix this with runtime checks, but it seems there are very many places that > need the checks. I don't care about X11 support in F23+. I guess we shouldn't care to fix bugs upstream that only happen in Fedora then.
Carlos Garcia Campos
Comment 4 2015-07-29 23:51:40 PDT
Created attachment 257822 [details] Patch I guess this should be enough, I haven't tested it though. Could someone with wayland confirm this patch works?
Michael Catanzaro
Comment 5 2015-07-30 13:00:47 PDT
(In reply to comment #3) > I guess we shouldn't care to fix bugs upstream that only happen in Fedora > then. I don't understand; if I disable NPAPI in Fedora, then the bug would not occur in Fedora, but only in other distros using Wayland. (In reply to comment #4) > Created attachment 257822 [details] > Patch > > I guess this should be enough, I haven't tested it though. Could someone > with wayland confirm this patch works? Thanks. When I looked at the plugin code, I thought it would need runtime checks in very many places besides this one. But if just that one check will work, then that is awesome. But I can't confirm it works, since my development build is crashing on start under Wayland. I will file another bug about this; it must be a recent regression, since this worked a few weeks ago.
Michael Catanzaro
Comment 6 2015-07-30 13:20:35 PDT
Bug #147453
Carlos Garcia Campos
Comment 7 2015-07-31 03:36:25 PDT
Created attachment 257917 [details] Different patch Zan suggested me to do that event earlier, so that not even the WebCore frame loader creates the plugin. So, I'm disabling the enable-plugins settings unconditionally for wayland.
Zan Dobersek
Comment 8 2015-07-31 04:00:08 PDT
Comment on attachment 257917 [details] Different patch View in context: https://bugs.webkit.org/attachment.cgi?id=257917&action=review > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1600 > + if (WebCore::PlatformDisplay::sharedDisplay().type() == WebCore::PlatformDisplay::Type::Wayland) > + return; Should this throw a warning?
Carlos Garcia Campos
Comment 9 2015-07-31 04:46:55 PDT
(In reply to comment #8) > Comment on attachment 257917 [details] > Different patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257917&action=review > > > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1600 > > + if (WebCore::PlatformDisplay::sharedDisplay().type() == WebCore::PlatformDisplay::Type::Wayland) > > + return; > > Should this throw a warning? I don't think so, this will always be called on construction, because the default value of the setting is TRUE and all properties in WebKitSettings are construct properties.
Carlos Garcia Campos
Comment 10 2015-07-31 05:24:19 PDT
Committed r187641: <http://trac.webkit.org/changeset/187641>
Michael Catanzaro
Comment 11 2015-07-31 06:28:21 PDT
Thanks. I think we should probably force disable the enable-java setting as well, in case some application queries that and gets confused that plugins are disabled but Java supposedly is enabled.
Note You need to log in before you can comment on or make changes to this bug.