Changeset 209064 in webkit

Timestamp:

Nov 28, 2016 11:34:44 PM (7 years ago)

Author:

Carlos Garcia Campos

Message:

[GTK] Crash in WebCore::PlatformDisplayX11::supportsXComposite when running under Wayland
​https://bugs.webkit.org/show_bug.cgi?id=164917

Reviewed by Michael Catanzaro.

WebKitGTK+ appplications are expected to call gtk_init(), because WebKitGTK+, like GTK+ itself, requires a
display to work. We currently fallback to create a X11 display when X11 is enabled in cases where GTK+ doesn't
have a default display (gtk_init() wasn't called or failed). That's why we end up creating an X11 display under
Wayland when both Wayland and X11 option are enabled. The code assumes X11 display creation will always work if
X11 is enabled, but that's not true now that we support also Wayland at runtime. So, we should try to get a
native display before creating the PlatformDisplay. Rendering will not work in any case when gtk_init() is not
called, but in most of the cases those applications are not actually going to render anything, so this way at
least we will not crash.

• platform/graphics/PlatformDisplay.cpp:

(WebCore::PlatformDisplay::createPlatformDisplay): Use create() method for X11 and Wayland if we couldn't get a
native display from GTK+. If everything fails create a display with no native.
(WebCore::PlatformDisplay::PlatformDisplay): Add NativeDisplayOwned parameter.

• platform/graphics/PlatformDisplay.h:
• platform/graphics/wayland/PlatformDisplayWayland.cpp:

(WebCore::PlatformDisplayWayland::create): Try to create a native Wayland display or return nullptr.
(WebCore::PlatformDisplayWayland::PlatformDisplayWayland): Initialize NativeDisplayOwned parameter.
(WebCore::PlatformDisplayWayland::~PlatformDisplayWayland): Destroy the display if owned.
(WebCore::PlatformDisplayWayland::initialize): Return early if native display is nullptr.

• platform/graphics/wayland/PlatformDisplayWayland.h:
• platform/graphics/x11/PlatformDisplayX11.cpp:

(WebCore::PlatformDisplayX11::create): Try to create a native X11 display or return nullptr.
(WebCore::PlatformDisplayX11::PlatformDisplayX11): Use NativeDisplayOwned now.
(WebCore::PlatformDisplayX11::~PlatformDisplayX11): Ditto.

• platform/graphics/x11/PlatformDisplayX11.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/PlatformDisplay.cpp (modified) (2 diffs)
• platform/graphics/PlatformDisplay.h (modified) (1 diff)
• platform/graphics/wayland/PlatformDisplayWayland.cpp (modified) (3 diffs)
• platform/graphics/wayland/PlatformDisplayWayland.h (modified) (1 diff)
• platform/graphics/x11/PlatformDisplayX11.cpp (modified) (2 diffs)
• platform/graphics/x11/PlatformDisplayX11.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-11-28  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Crash in WebCore::PlatformDisplayX11::supportsXComposite when running under Wayland
+        https://bugs.webkit.org/show_bug.cgi?id=164917
+
+        Reviewed by Michael Catanzaro.
+
+        WebKitGTK+ appplications are expected to call gtk_init(), because WebKitGTK+, like GTK+ itself, requires a
+        display to work. We currently fallback to create a X11 display when X11 is enabled in cases where GTK+ doesn't
+        have a default display (gtk_init() wasn't called or failed). That's why we end up creating an X11 display under
+        Wayland when both Wayland and X11 option are enabled. The code assumes X11 display creation will always work if
+        X11 is enabled, but that's not true now that we support also Wayland at runtime. So, we should try to get a
+        native display before creating the PlatformDisplay. Rendering will not work in any case when gtk_init() is not
+        called, but in most of the cases those applications are not actually going to render anything, so this way at
+        least we will not crash.
+
+        * platform/graphics/PlatformDisplay.cpp:
+        (WebCore::PlatformDisplay::createPlatformDisplay): Use create() method for X11 and Wayland if we couldn't get a
+        native display from GTK+. If everything fails create a display with no native.
+        (WebCore::PlatformDisplay::PlatformDisplay): Add NativeDisplayOwned parameter.
+        * platform/graphics/PlatformDisplay.h:
+        * platform/graphics/wayland/PlatformDisplayWayland.cpp:
+        (WebCore::PlatformDisplayWayland::create): Try to create a native Wayland display or return nullptr.
+        (WebCore::PlatformDisplayWayland::PlatformDisplayWayland): Initialize NativeDisplayOwned parameter.
+        (WebCore::PlatformDisplayWayland::~PlatformDisplayWayland): Destroy the display if owned.
+        (WebCore::PlatformDisplayWayland::initialize): Return early if native display is nullptr.
+        * platform/graphics/wayland/PlatformDisplayWayland.h:
+        * platform/graphics/x11/PlatformDisplayX11.cpp:
+        (WebCore::PlatformDisplayX11::create): Try to create a native X11 display or return nullptr.
+        (WebCore::PlatformDisplayX11::PlatformDisplayX11): Use NativeDisplayOwned now.
+        (WebCore::PlatformDisplayX11::~PlatformDisplayX11): Ditto.
+        * platform/graphics/x11/PlatformDisplayX11.h:
+
 2016-11-28  Matt Baker  <mattbaker@apple.com>
 

trunk/Source/WebCore/platform/graphics/PlatformDisplay.cpp

@@ -89 +89 @@
 #endif
 
-#if PLATFORM(X11)
-    return std::make_unique<PlatformDisplayX11>();
+#if PLATFORM(WAYLAND)
+    if (auto platformDisplay = PlatformDisplayWayland::create())
+        return platformDisplay;
+#endif
+
+#if PLATFORM(X11)
+    if (auto platformDisplay = PlatformDisplayX11::create())
+        return platformDisplay;
+#endif
+
+    // If at this point we still don't have a display, just create a fake display with no native.
+#if PLATFORM(WAYLAND)
+    return std::make_unique<PlatformDisplayWayland>(nullptr);
+#endif
+#if PLATFORM(X11)
+    return std::make_unique<PlatformDisplayX11>(nullptr);
 #endif
 
@@ -119 +133 @@
 }
 
-PlatformDisplay::PlatformDisplay()
-#if USE(EGL)
-    : m_eglDisplay(EGL_NO_DISPLAY)
+PlatformDisplay::PlatformDisplay(NativeDisplayOwned displayOwned)
+    : m_nativeDisplayOwned(displayOwned)
+#if USE(EGL)
+    , m_eglDisplay(EGL_NO_DISPLAY)
 #endif
 {

trunk/Source/WebCore/platform/graphics/PlatformDisplay.h

@@ -71 +71 @@
 
 protected:
-    PlatformDisplay();
+    enum class NativeDisplayOwned { No, Yes };
+    explicit PlatformDisplay(NativeDisplayOwned = NativeDisplayOwned::No);
 
     static void setSharedDisplayForCompositing(PlatformDisplay&);
+
+    NativeDisplayOwned m_nativeDisplayOwned { NativeDisplayOwned::No };
 
 #if USE(EGL)

trunk/Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.cpp

@@ -51 +51 @@
 };
 
-PlatformDisplayWayland::PlatformDisplayWayland(struct wl_display* display)
+std::unique_ptr<PlatformDisplay> PlatformDisplayWayland::create()
+{
+    struct wl_display* display = wl_display_connect(getenv("DISPLAY"));
+    if (!display)
+        return nullptr;
+
+    return std::make_unique<PlatformDisplayWayland>(display, NativeDisplayOwned::Yes);
+}
+
+PlatformDisplayWayland::PlatformDisplayWayland(struct wl_display* display, NativeDisplayOwned displayOwned)
+    : PlatformDisplay(displayOwned)
 {
     initialize(display);
@@ -58 +68 @@
 PlatformDisplayWayland::~PlatformDisplayWayland()
 {
+    if (m_nativeDisplayOwned == NativeDisplayOwned::Yes)
+        wl_display_destroy(m_display);
 }
 
@@ -63 +75 @@
 {
     m_display = display;
+    if (!m_display)
+        return;
+
     m_registry.reset(wl_display_get_registry(m_display));
     wl_registry_add_listener(m_registry.get(), &s_registryListener, this);

trunk/Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.h

@@ -37 +37 @@
 class PlatformDisplayWayland : public PlatformDisplay {
 public:
-    PlatformDisplayWayland(struct wl_display*);
+    static std::unique_ptr<PlatformDisplay> create();
+    PlatformDisplayWayland(struct wl_display*, NativeDisplayOwned = NativeDisplayOwned::No);
     virtual ~PlatformDisplayWayland();
 

trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp

@@ -43 +43 @@
 namespace WebCore {
 
-PlatformDisplayX11::PlatformDisplayX11()
-    : m_display(XOpenDisplay(nullptr))
+std::unique_ptr<PlatformDisplay> PlatformDisplayX11::create()
 {
-    m_ownedDisplay = m_display != nullptr;
+    Display* display = XOpenDisplay(getenv("DISPLAY"));
+    if (!display)
+        return nullptr;
+
+    return std::make_unique<PlatformDisplayX11>(display, NativeDisplayOwned::Yes);
 }
 
-PlatformDisplayX11::PlatformDisplayX11(Display* display)
-    : m_display(display)
-    , m_ownedDisplay(false)
+PlatformDisplayX11::PlatformDisplayX11(Display* display, NativeDisplayOwned displayOwned)
+    : PlatformDisplay(displayOwned)
+    , m_display(display)
 {
 }
@@ -61 +64 @@
     m_sharingGLContext = nullptr;
 #endif
-    if (m_ownedDisplay)
+    if (m_nativeDisplayOwned == NativeDisplayOwned::Yes)
         XCloseDisplay(m_display);
 }

trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.h

@@ -38 +38 @@
 class PlatformDisplayX11 final : public PlatformDisplay {
 public:
-    PlatformDisplayX11();
-    PlatformDisplayX11(Display*);
+    static std::unique_ptr<PlatformDisplay> create();
+    PlatformDisplayX11(Display*, NativeDisplayOwned = NativeDisplayOwned::No);
     virtual ~PlatformDisplayX11();
 
@@ -53 +53 @@
 #endif
 
-    Display* m_display;
-    bool m_ownedDisplay;
+    Display* m_display { nullptr };
     mutable std::optional<bool> m_supportsXComposite;
     mutable std::optional<bool> m_supportsXDamage;