Bug 147297 - [GTK] NPAPI code crashes on Wayland
Summary: [GTK] NPAPI code crashes on Wayland
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 81456
  Show dependency treegraph
 
Reported: 2015-07-25 15:36 PDT by Michael Catanzaro
Modified: 2016-11-30 10:01 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.02 KB, patch)
2015-07-29 23:51 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Different patch (3.15 KB, patch)
2015-07-31 03:36 PDT, Carlos Garcia Campos
zan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.