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+

Description Michael Catanzaro 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.
Comment 1 Carlos Garcia Campos 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.
Comment 2 Michael Catanzaro 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+.
Comment 3 Carlos Garcia Campos 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.
Comment 4 Carlos Garcia Campos 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?
Comment 5 Michael Catanzaro 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.
Comment 6 Michael Catanzaro 2015-07-30 13:20:35 PDT
Bug #147453
Comment 7 Carlos Garcia Campos 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.
Comment 8 Zan Dobersek 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?
Comment 9 Carlos Garcia Campos 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.
Comment 10 Carlos Garcia Campos 2015-07-31 05:24:19 PDT
Committed r187641: <http://trac.webkit.org/changeset/187641>
Comment 11 Michael Catanzaro 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.