- Revision
- 209064
- Author
- carlo...@webkit.org
- Date
- 2016-11-28 23:34:44 -0800 (Mon, 28 Nov 2016)
Log 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:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (209063 => 209064)
--- trunk/Source/WebCore/ChangeLog 2016-11-29 07:27:01 UTC (rev 209063)
+++ trunk/Source/WebCore/ChangeLog 2016-11-29 07:34:44 UTC (rev 209064)
@@ -1,3 +1,36 @@
+2016-11-28 Carlos Garcia Campos <cgar...@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 <mattba...@apple.com>
Web Inspector: Debugger should have an option for showing asynchronous call stacks
Modified: trunk/Source/WebCore/platform/graphics/PlatformDisplay.cpp (209063 => 209064)
--- trunk/Source/WebCore/platform/graphics/PlatformDisplay.cpp 2016-11-29 07:27:01 UTC (rev 209063)
+++ trunk/Source/WebCore/platform/graphics/PlatformDisplay.cpp 2016-11-29 07:34:44 UTC (rev 209064)
@@ -88,10 +88,24 @@
return std::make_unique<PlatformDisplayWin>();
#endif
+#if PLATFORM(WAYLAND)
+ if (auto platformDisplay = PlatformDisplayWayland::create())
+ return platformDisplay;
+#endif
+
#if PLATFORM(X11)
- return std::make_unique<PlatformDisplayX11>();
+ 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
+
ASSERT_NOT_REACHED();
return nullptr;
}
@@ -118,9 +132,10 @@
s_sharedDisplayForCompositing = &display;
}
-PlatformDisplay::PlatformDisplay()
+PlatformDisplay::PlatformDisplay(NativeDisplayOwned displayOwned)
+ : m_nativeDisplayOwned(displayOwned)
#if USE(EGL)
- : m_eglDisplay(EGL_NO_DISPLAY)
+ , m_eglDisplay(EGL_NO_DISPLAY)
#endif
{
}
Modified: trunk/Source/WebCore/platform/graphics/PlatformDisplay.h (209063 => 209064)
--- trunk/Source/WebCore/platform/graphics/PlatformDisplay.h 2016-11-29 07:27:01 UTC (rev 209063)
+++ trunk/Source/WebCore/platform/graphics/PlatformDisplay.h 2016-11-29 07:34:44 UTC (rev 209064)
@@ -70,10 +70,13 @@
#endif
protected:
- PlatformDisplay();
+ enum class NativeDisplayOwned { No, Yes };
+ explicit PlatformDisplay(NativeDisplayOwned = NativeDisplayOwned::No);
static void setSharedDisplayForCompositing(PlatformDisplay&);
+ NativeDisplayOwned m_nativeDisplayOwned { NativeDisplayOwned::No };
+
#if USE(EGL)
virtual void initializeEGLDisplay();
Modified: trunk/Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.cpp (209063 => 209064)
--- trunk/Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.cpp 2016-11-29 07:27:01 UTC (rev 209063)
+++ trunk/Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.cpp 2016-11-29 07:34:44 UTC (rev 209064)
@@ -50,18 +50,33 @@
}
};
-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);
}
PlatformDisplayWayland::~PlatformDisplayWayland()
{
+ if (m_nativeDisplayOwned == NativeDisplayOwned::Yes)
+ wl_display_destroy(m_display);
}
void PlatformDisplayWayland::initialize(wl_display* display)
{
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);
wl_display_roundtrip(m_display);
Modified: trunk/Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.h (209063 => 209064)
--- trunk/Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.h 2016-11-29 07:27:01 UTC (rev 209063)
+++ trunk/Source/WebCore/platform/graphics/wayland/PlatformDisplayWayland.h 2016-11-29 07:34:44 UTC (rev 209064)
@@ -36,7 +36,8 @@
class PlatformDisplayWayland : public PlatformDisplay {
public:
- PlatformDisplayWayland(struct wl_display*);
+ static std::unique_ptr<PlatformDisplay> create();
+ PlatformDisplayWayland(struct wl_display*, NativeDisplayOwned = NativeDisplayOwned::No);
virtual ~PlatformDisplayWayland();
struct wl_display* native() const { return m_display; }
Modified: trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp (209063 => 209064)
--- trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp 2016-11-29 07:27:01 UTC (rev 209063)
+++ trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp 2016-11-29 07:34:44 UTC (rev 209064)
@@ -42,15 +42,18 @@
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)
{
}
@@ -60,7 +63,7 @@
// Clear the sharing context before releasing the display.
m_sharingGLContext = nullptr;
#endif
- if (m_ownedDisplay)
+ if (m_nativeDisplayOwned == NativeDisplayOwned::Yes)
XCloseDisplay(m_display);
}
Modified: trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.h (209063 => 209064)
--- trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.h 2016-11-29 07:27:01 UTC (rev 209063)
+++ trunk/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.h 2016-11-29 07:34:44 UTC (rev 209064)
@@ -37,8 +37,8 @@
class PlatformDisplayX11 final : public PlatformDisplay {
public:
- PlatformDisplayX11();
- PlatformDisplayX11(Display*);
+ static std::unique_ptr<PlatformDisplay> create();
+ PlatformDisplayX11(Display*, NativeDisplayOwned = NativeDisplayOwned::No);
virtual ~PlatformDisplayX11();
Display* native() const { return m_display; }
@@ -52,8 +52,7 @@
void initializeEGLDisplay() override;
#endif
- Display* m_display;
- bool m_ownedDisplay;
+ Display* m_display { nullptr };
mutable std::optional<bool> m_supportsXComposite;
mutable std::optional<bool> m_supportsXDamage;
mutable std::optional<int> m_damageEventBase;
