Bug 164917

Summary: [GTK] Crash in WebCore::PlatformDisplayX11::supportsXComposite when running under Wayland
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, mcatanzaro
Priority: P2    
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
See Also: https://bugzilla.redhat.com/show_bug.cgi?id=1387180
Attachments:
Description Flags
Patch
none
Try to fix the build on the bots
none
Fix EFL build
none
Fix EFL build mcatanzaro: review+

Description Michael Catanzaro 2016-11-18 05:13:08 PST
Crash in WebCore::PlatformDisplayX11::supportsXComposite when running under Wayland. This is an epiphany-search-provider crash. I wonder how we created a PlatformDisplayX11 at all:

Truncated backtrace:
Thread no. 1 (10 frames)
 #0 XQueryExtension at QuExt.c:43
 #1 XInitExtension at InitExt.c:47
 #2 XCompositeExtAddDisplay at Xcomposite.c:111
 #3 XCompositeFindDisplay at Xcomposite.c:208
 #4 XCompositeQueryExtension at Xcomposite.c:219
 #5 WebCore::PlatformDisplayX11::supportsXComposite at /usr/src/debug/webkitgtk-2.14.1/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp:80
 #6 WebKit::WebPreferences::platformInitializeStore at /usr/src/debug/webkitgtk-2.14.1/Source/WebKit2/UIProcess/gtk/WebPreferencesGtk.cpp:63
 #7 WebKit::WebPreferences::create at /usr/src/debug/webkitgtk-2.14.1/Source/WebKit2/UIProcess/WebPreferences.cpp:43
 #8 WebKit::WebPreferences::createWithLegacyDefaults at /usr/src/debug/webkitgtk-2.14.1/Source/WebKit2/UIProcess/WebPreferences.cpp:48
 #9 WebKit::WebPageGroup::WebPageGroup at /usr/src/debug/webkitgtk-2.14.1/Source/WebKit2/UIProcess/WebPageGroup.cpp:93

Full backtrace downstream.
Comment 1 Carlos Garcia Campos 2016-11-24 05:58:21 PST
Why does ephy search provider use WebKit at all? WebKit requires a display to work, so when the shared display is created we try to guess the display to create. In the GTK+ port we ask gtk+ first for the default display, if the application called gtk_init we will always have either a X11 or Wayland display. If application didn't call gtk_init, we create our own display that is X11 display if webkit is built with X11 support. We should try to connect to wayland first and then to X11, like gtk+ does.
Comment 2 Carlos Garcia Campos 2016-11-24 06:58:37 PST
Created attachment 295408 [details]
Patch

I think this should work.
Comment 3 Carlos Garcia Campos 2016-11-24 08:18:50 PST
I don't know why it fails to build in the bots, it builds fine locally.
Comment 4 Carlos Garcia Campos 2016-11-24 08:20:53 PST
Created attachment 295412 [details]
Try to fix the build on the bots
Comment 5 Carlos Garcia Campos 2016-11-24 08:30:36 PST
Created attachment 295413 [details]
Fix EFL build
Comment 6 Carlos Garcia Campos 2016-11-24 08:41:13 PST
Created attachment 295415 [details]
Fix EFL build
Comment 7 Michael Catanzaro 2016-11-24 12:40:20 PST
(In reply to comment #1)
> Why does ephy search provider use WebKit at all?

https://bugzilla.gnome.org/show_bug.cgi?id=741595

Not sure if it can be avoided because that was two years ago and the bug doesn't have a backtrace. :(
Comment 8 Michael Catanzaro 2016-11-24 12:43:53 PST
Ah here's a backtrace: https://bugzilla.redhat.com/show_bug.cgi?id=1174007

Looks like the search provider was trying to import bookmarks from Firefox. Wow.
Comment 9 Michael Catanzaro 2016-11-25 08:45:01 PST
Comment on attachment 295415 [details]
Fix EFL build

View in context: https://bugs.webkit.org/attachment.cgi?id=295415&action=review

Thanks.

> Source/WebCore/platform/graphics/PlatformDisplay.h:74
> +    PlatformDisplay(NativeDisplayOwned = NativeDisplayOwned::No);

It doesn't matter much because the constructor is protected and not public, but without the explicit keyword you've created an implicit conversion between NativeDisplayOwned and PlatformDisplay, which is definitely undesired. So please add the explicit keyword before landing. Also, consider getting rid of the default argument. Our code will be more robust to future modification if you have to always pass NativeDisplayOwned when creating the display.
Comment 10 Carlos Garcia Campos 2016-11-28 23:35:11 PST
Committed r209064: <http://trac.webkit.org/changeset/209064>