Title: [205956] releases/WebKitGTK/webkit-2.14/Source/WebCore
Revision
205956
Author
carlo...@webkit.org
Date
2016-09-15 00:59:10 -0700 (Thu, 15 Sep 2016)

Log Message

Merge r205852 - [GTK] Crash of WebProcess on the last WebView disconnect (take two)
https://bugs.webkit.org/show_bug.cgi?id=161842

Reviewed by Michael Catanzaro.

The problem is that when PlatformDisplayX11 is destroyed, the sharing GL context is deleted and its destructor
makes a downcast of PlatformDisplay to get the native X11 display. We could simply keep a pointer to the native
X11 display in GLContextGLX, got at construction time from the PlatformDisplay, and ensure the sharing GL
context is deleted before the native X11 display is closed.

* platform/graphics/PlatformDisplay.h: Make m_sharingGLContext protected.
* platform/graphics/glx/GLContextGLX.cpp:
(WebCore::GLContextGLX::GLContextGLX): Initialize m_x11Display.
(WebCore::GLContextGLX::~GLContextGLX): Use m_x11Display and remove confusing comment about possible crash with
nviedia closed drivers.
(WebCore::GLContextGLX::defaultFrameBufferSize): Use m_x11Display.
(WebCore::GLContextGLX::makeContextCurrent): Ditto.
(WebCore::GLContextGLX::swapBuffers): Ditto.
(WebCore::GLContextGLX::swapInterval): Ditto.
(WebCore::GLContextGLX::cairoDevice): Ditto.
* platform/graphics/glx/GLContextGLX.h:
* platform/graphics/x11/PlatformDisplayX11.cpp:
(WebCore::PlatformDisplayX11::~PlatformDisplayX11): Delete the sharing GL context before closing the display.

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog (205955 => 205956)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog	2016-09-15 07:50:59 UTC (rev 205955)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog	2016-09-15 07:59:10 UTC (rev 205956)
@@ -1,3 +1,29 @@
+2016-09-12  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        [GTK] Crash of WebProcess on the last WebView disconnect (take two)
+        https://bugs.webkit.org/show_bug.cgi?id=161842
+
+        Reviewed by Michael Catanzaro.
+
+        The problem is that when PlatformDisplayX11 is destroyed, the sharing GL context is deleted and its destructor
+        makes a downcast of PlatformDisplay to get the native X11 display. We could simply keep a pointer to the native
+        X11 display in GLContextGLX, got at construction time from the PlatformDisplay, and ensure the sharing GL
+        context is deleted before the native X11 display is closed.
+
+        * platform/graphics/PlatformDisplay.h: Make m_sharingGLContext protected.
+        * platform/graphics/glx/GLContextGLX.cpp:
+        (WebCore::GLContextGLX::GLContextGLX): Initialize m_x11Display.
+        (WebCore::GLContextGLX::~GLContextGLX): Use m_x11Display and remove confusing comment about possible crash with
+        nviedia closed drivers.
+        (WebCore::GLContextGLX::defaultFrameBufferSize): Use m_x11Display.
+        (WebCore::GLContextGLX::makeContextCurrent): Ditto.
+        (WebCore::GLContextGLX::swapBuffers): Ditto.
+        (WebCore::GLContextGLX::swapInterval): Ditto.
+        (WebCore::GLContextGLX::cairoDevice): Ditto.
+        * platform/graphics/glx/GLContextGLX.h:
+        * platform/graphics/x11/PlatformDisplayX11.cpp:
+        (WebCore::PlatformDisplayX11::~PlatformDisplayX11): Delete the sharing GL context before closing the display.
+
 2016-09-06  Carlos Garcia Campos  <cgar...@igalia.com>
 
         [GTK] Crash of WebProcess on the last WebView disconnect

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/PlatformDisplay.h (205955 => 205956)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/PlatformDisplay.h	2016-09-15 07:50:59 UTC (rev 205955)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/PlatformDisplay.h	2016-09-15 07:59:10 UTC (rev 205956)
@@ -80,6 +80,8 @@
     EGLDisplay m_eglDisplay;
 #endif
 
+    std::unique_ptr<GLContext> m_sharingGLContext;
+
 private:
     static std::unique_ptr<PlatformDisplay> createPlatformDisplay();
 
@@ -90,7 +92,6 @@
     int m_eglMajorVersion { 0 };
     int m_eglMinorVersion { 0 };
 #endif
-    std::unique_ptr<GLContext> m_sharingGLContext;
 };
 
 } // namespace WebCore

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp (205955 => 205956)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp	2016-09-15 07:50:59 UTC (rev 205955)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp	2016-09-15 07:59:10 UTC (rev 205956)
@@ -180,6 +180,7 @@
 
 GLContextGLX::GLContextGLX(PlatformDisplay& display, XUniqueGLXContext&& context, XID window)
     : GLContext(display)
+    , m_x11Display(downcast<PlatformDisplayX11>(m_display).native())
     , m_context(WTFMove(context))
     , m_window(window)
 {
@@ -188,6 +189,7 @@
 
 GLContextGLX::GLContextGLX(PlatformDisplay& display, XUniqueGLXContext&& context, XUniqueGLXPbuffer&& pbuffer)
     : GLContext(display)
+    , m_x11Display(downcast<PlatformDisplayX11>(m_display).native())
     , m_context(WTFMove(context))
     , m_pbuffer(WTFMove(pbuffer))
 {
@@ -196,6 +198,7 @@
 
 GLContextGLX::GLContextGLX(PlatformDisplay& display, XUniqueGLXContext&& context, XUniquePixmap&& pixmap, XUniqueGLXPixmap&& glxPixmap)
     : GLContext(display)
+    , m_x11Display(downcast<PlatformDisplayX11>(m_display).native())
     , m_context(WTFMove(context))
     , m_pixmap(WTFMove(pixmap))
     , m_glxPixmap(WTFMove(glxPixmap))
@@ -219,10 +222,8 @@
         m_cairoDevice = nullptr;
     }
 
-    // This may be necessary to prevent crashes with NVidia's closed source drivers. Originally
-    // from Mozilla's 3D canvas implementation at: http://bitbucket.org/ilmari/canvas3d/
     glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0);
-    glXMakeCurrent(downcast<PlatformDisplayX11>(m_display).native(), None, None);
+    glXMakeCurrent(m_x11Display, None, None);
 
     m_context = nullptr;
 }
@@ -240,7 +241,7 @@
     int x, y;
     Window rootWindow;
     unsigned int width, height, borderWidth, depth;
-    if (!XGetGeometry(downcast<PlatformDisplayX11>(m_display).native(), m_window, &rootWindow, &x, &y, &width, &height, &borderWidth, &depth))
+    if (!XGetGeometry(m_x11Display, m_window, &rootWindow, &x, &y, &width, &height, &borderWidth, &depth))
         return IntSize();
 
     return IntSize(width, height);
@@ -254,20 +255,19 @@
     if (glXGetCurrentContext() == m_context.get())
         return true;
 
-    Display* display = downcast<PlatformDisplayX11>(m_display).native();
     if (m_window)
-        return glXMakeCurrent(display, m_window, m_context.get());
+        return glXMakeCurrent(m_x11Display, m_window, m_context.get());
 
     if (m_pbuffer)
-        return glXMakeCurrent(display, m_pbuffer.get(), m_context.get());
+        return glXMakeCurrent(m_x11Display, m_pbuffer.get(), m_context.get());
 
-    return ::glXMakeCurrent(display, m_glxPixmap.get(), m_context.get());
+    return ::glXMakeCurrent(m_x11Display, m_glxPixmap.get(), m_context.get());
 }
 
 void GLContextGLX::swapBuffers()
 {
     if (m_window)
-        glXSwapBuffers(downcast<PlatformDisplayX11>(m_display).native(), m_window);
+        glXSwapBuffers(m_x11Display, m_window);
 }
 
 void GLContextGLX::waitNative()
@@ -277,7 +277,7 @@
 
 void GLContextGLX::swapInterval(int interval)
 {
-    if (!hasSGISwapControlExtension(downcast<PlatformDisplayX11>(m_display).native()))
+    if (!hasSGISwapControlExtension(m_x11Display))
         return;
     glXSwapIntervalSGI(interval);
 }
@@ -288,7 +288,7 @@
         return m_cairoDevice;
 
 #if ENABLE(ACCELERATED_2D_CANVAS) && CAIRO_HAS_GLX_FUNCTIONS
-    m_cairoDevice = cairo_glx_device_create(downcast<PlatformDisplayX11>(m_display).native(), m_context.get());
+    m_cairoDevice = cairo_glx_device_create(m_x11Display, m_context.get());
 #endif
 
     return m_cairoDevice;

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.h (205955 => 205956)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.h	2016-09-15 07:50:59 UTC (rev 205955)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.h	2016-09-15 07:59:10 UTC (rev 205956)
@@ -29,6 +29,7 @@
 typedef unsigned char GLubyte;
 typedef unsigned long XID;
 typedef void* ContextKeyType;
+typedef struct _XDisplay Display;
 
 namespace WebCore {
 
@@ -64,6 +65,7 @@
     static std::unique_ptr<GLContextGLX> createPbufferContext(PlatformDisplay&, GLXContext sharingContext = nullptr);
     static std::unique_ptr<GLContextGLX> createPixmapContext(PlatformDisplay&, GLXContext sharingContext = nullptr);
 
+    Display* m_x11Display { nullptr };
     XUniqueGLXContext m_context;
     XID m_window { 0 };
     XUniqueGLXPbuffer m_pbuffer;

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp (205955 => 205956)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp	2016-09-15 07:50:59 UTC (rev 205955)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/x11/PlatformDisplayX11.cpp	2016-09-15 07:59:10 UTC (rev 205956)
@@ -35,8 +35,14 @@
 
 #if USE(EGL)
 #include <EGL/egl.h>
+#include <EGL/eglplatform.h>
 #endif
 
+// FIXME: this needs to be here, after eglplatform.h, to avoid EGLNativeDisplayType to be defined as wl_display.
+// Since we support Wayland and X11 to be built at the same time, but eglplatform.h defines are decided at compile time
+// we need to ensure we only include eglplatform.h from X11 or Wayland specific files.
+#include "GLContext.h"
+
 namespace WebCore {
 
 PlatformDisplayX11::PlatformDisplayX11()
@@ -53,6 +59,8 @@
 
 PlatformDisplayX11::~PlatformDisplayX11()
 {
+    // Clear the sharing context before releasing the display.
+    m_sharingGLContext = nullptr;
     if (m_ownedDisplay)
         XCloseDisplay(m_display);
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to