Merge r249947 - [GTK] Crash closing web view while hardware accelerat…

…ion is enabled https://bugs.webkit.org/show_bug.cgi?id=200856 Reviewed by Michael Catanzaro. The crash happens when destroying the WaylandCompositor::Surface because the web view GL context is used to release the texture, but the GL context is no longer valid after web view unrealize. AcceleratedBackingStoreWayland should handle the web view unrealize to destroy the GL context. It will be created on demand again after the web view is realized. * UIProcess/API/gtk/WebKitWebViewBase.cpp: (webkitWebViewBaseRealize): Notify AcceleratedBackingStore. (webkitWebViewBaseUnrealize): Ditto. * UIProcess/gtk/AcceleratedBackingStore.h: (WebKit::AcceleratedBackingStore::realize): Added. (WebKit::AcceleratedBackingStore::unrealize): Added. * UIProcess/gtk/AcceleratedBackingStoreWayland.cpp: (WebKit::AcceleratedBackingStoreWayland::realize): In case of using WaylandCompositor, call WaylandCompositor::bindWebPage() to bind the WebPageProxy to the Wayland surface. (WebKit::AcceleratedBackingStoreWayland::unrealize): Destroy GL resources and the GL context. (WebKit::AcceleratedBackingStoreWayland::tryEnsureGLContext): Do not try to create the GL context if the web view is not realized. (WebKit::AcceleratedBackingStoreWayland::displayBuffer): Remove the code to initialize the texture. (WebKit::AcceleratedBackingStoreWayland::paint): And add it here. * UIProcess/gtk/AcceleratedBackingStoreWayland.h: * UIProcess/gtk/WaylandCompositor.cpp: (WebKit::WaylandCompositor::Surface::setWebPage): Return early if given page is the current one already. (WebKit::WaylandCompositor::bindWebPage): Set the surface WebPageProxy. (WebKit::WaylandCompositor::unbindWebPage): Unset the surface WebPageProxy. * UIProcess/gtk/WaylandCompositor.h: * WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp: (WebKit::DrawingAreaCoordinatedGraphics::enterAcceleratedCompositingMode): When restoring a previous layer tree host, always call resumeRendering() to balance the suspendRendering() called in exitAcceleratedCompositingMode().
WebKit · Sep 23, 2019 · 317b9c9 · 317b9c9
1 parent 1b6e871
commit 317b9c9
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 16 deletions.
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
@@ -1,3 +1,39 @@
+2019-09-17  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Crash closing web view while hardware acceleration is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=200856
+
+        Reviewed by Michael Catanzaro.
+
+        The crash happens when destroying the WaylandCompositor::Surface because the web view GL context is used to
+        release the texture, but the GL context is no longer valid after web view
+        unrealize. AcceleratedBackingStoreWayland should handle the web view unrealize to destroy the GL context. It
+        will be created on demand again after the web view is realized.
+
+        * UIProcess/API/gtk/WebKitWebViewBase.cpp:
+        (webkitWebViewBaseRealize): Notify AcceleratedBackingStore.
+        (webkitWebViewBaseUnrealize): Ditto.
+        * UIProcess/gtk/AcceleratedBackingStore.h:
+        (WebKit::AcceleratedBackingStore::realize): Added.
+        (WebKit::AcceleratedBackingStore::unrealize): Added.
+        * UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:
+        (WebKit::AcceleratedBackingStoreWayland::realize): In case of using WaylandCompositor, call
+        WaylandCompositor::bindWebPage() to bind the WebPageProxy to the Wayland surface.
+        (WebKit::AcceleratedBackingStoreWayland::unrealize): Destroy GL resources and the GL context.
+        (WebKit::AcceleratedBackingStoreWayland::tryEnsureGLContext): Do not try to create the GL context if the web
+        view is not realized.
+        (WebKit::AcceleratedBackingStoreWayland::displayBuffer): Remove the code to initialize the texture.
+        (WebKit::AcceleratedBackingStoreWayland::paint): And add it here.
+        * UIProcess/gtk/AcceleratedBackingStoreWayland.h:
+        * UIProcess/gtk/WaylandCompositor.cpp:
+        (WebKit::WaylandCompositor::Surface::setWebPage): Return early if given page is the current one already.
+        (WebKit::WaylandCompositor::bindWebPage): Set the surface WebPageProxy.
+        (WebKit::WaylandCompositor::unbindWebPage): Unset the surface WebPageProxy.
+        * UIProcess/gtk/WaylandCompositor.h:
+        * WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp:
+        (WebKit::DrawingAreaCoordinatedGraphics::enterAcceleratedCompositingMode): When restoring a previous layer tree
+        host, always call resumeRendering() to balance the suspendRendering() called in exitAcceleratedCompositingMode().
+
 2019-09-16  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         REGRESSION(r249142): [GTK] Epiphany delayed page loads continue indefinitely

diff --git a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
@@ -422,13 +422,19 @@ static void webkitWebViewBaseRealize(GtkWidget* widget)
     gdk_window_set_user_data(window, widget);
 
     gtk_im_context_set_client_window(priv->inputMethodFilter.context(), window);
+
+    if (priv->acceleratedBackingStore)
+        priv->acceleratedBackingStore->realize();
 }
 
 static void webkitWebViewBaseUnrealize(GtkWidget* widget)
 {
     WebKitWebViewBase* webView = WEBKIT_WEB_VIEW_BASE(widget);
     gtk_im_context_set_client_window(webView->priv->inputMethodFilter.context(), nullptr);
 
+    if (webView->priv->acceleratedBackingStore)
+        webView->priv->acceleratedBackingStore->unrealize();
+
     GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->unrealize(widget);
 }
 

diff --git a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h
@@ -47,6 +47,8 @@ class AcceleratedBackingStore {
 
     virtual void update(const LayerTreeContext&) { }
     virtual bool paint(cairo_t*, const WebCore::IntRect&) = 0;
+    virtual void realize() { };
+    virtual void unrealize() { };
     virtual bool makeContextCurrent() { return false; }
     virtual int renderHostFileDescriptor() { return -1; }
 

diff --git a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp
@@ -148,9 +148,37 @@ AcceleratedBackingStoreWayland::~AcceleratedBackingStoreWayland()
         gdk_gl_context_clear_current();
 }
 
+void AcceleratedBackingStoreWayland::realize()
+{
+#if !USE(WPE_RENDERER)
+    WaylandCompositor::singleton().bindWebPage(m_webPage);
+#endif
+}
+
+void AcceleratedBackingStoreWayland::unrealize()
+{
+    if (!m_glContextInitialized)
+        return;
+
+#if USE(WPE_RENDERER)
+    if (m_viewTexture) {
+        if (makeContextCurrent())
+            glDeleteTextures(1, &m_viewTexture);
+        m_viewTexture = 0;
+    }
+#else
+    WaylandCompositor::singleton().unbindWebPage(m_webPage);
+#endif
+
+    if (m_gdkGLContext && m_gdkGLContext.get() == gdk_gl_context_get_current())
+        gdk_gl_context_clear_current();
+
+    m_glContextInitialized = false;
+}
+
 void AcceleratedBackingStoreWayland::tryEnsureGLContext()
 {
-    if (m_glContextInitialized)
+    if (m_glContextInitialized || !gtk_widget_get_realized(m_webPage.viewWidget()))
         return;
 
     m_glContextInitialized = true;
@@ -208,18 +236,6 @@ void AcceleratedBackingStoreWayland::displayBuffer(struct wpe_fdo_egl_exported_i
         return;
     }
 
-    if (!m_viewTexture) {
-        if (!makeContextCurrent())
-            return;
-
-        glGenTextures(1, &m_viewTexture);
-        glBindTexture(GL_TEXTURE_2D, m_viewTexture);
-        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
-        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
-        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
-        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
-    }
-
     if (m_pendingImage)
         wpe_view_backend_exportable_fdo_egl_dispatch_release_exported_image(m_exportable, m_pendingImage);
     m_pendingImage = image;
@@ -235,7 +251,7 @@ bool AcceleratedBackingStoreWayland::paint(cairo_t* cr, const IntRect& clipRect)
 
 #if USE(WPE_RENDERER)
     if (!makeContextCurrent())
-        return false;
+        return true;
 
     if (m_pendingImage) {
         wpe_view_backend_exportable_fdo_dispatch_frame_complete(m_exportable);
@@ -249,6 +265,14 @@ bool AcceleratedBackingStoreWayland::paint(cairo_t* cr, const IntRect& clipRect)
     if (!m_committedImage)
         return true;
 
+    if (!m_viewTexture) {
+        glGenTextures(1, &m_viewTexture);
+        glBindTexture(GL_TEXTURE_2D, m_viewTexture);
+        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    }
     glBindTexture(GL_TEXTURE_2D, m_viewTexture);
     glImageTargetTexture2D(GL_TEXTURE_2D, wpe_fdo_egl_exported_image_get_egl_image(m_committedImage));
 

diff --git a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h
@@ -65,6 +65,8 @@ class AcceleratedBackingStoreWayland final : public AcceleratedBackingStore {
 #endif
 
     bool paint(cairo_t*, const WebCore::IntRect&) override;
+    void realize() override;
+    void unrealize() override;
     bool makeContextCurrent() override;
 #if USE(WPE_RENDERER)
     void update(const LayerTreeContext&) override;

diff --git a/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp b/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp
@@ -167,6 +167,9 @@ WaylandCompositor::Surface::~Surface()
 
 void WaylandCompositor::Surface::setWebPage(WebPageProxy* webPage)
 {
+    if (m_webPage == webPage)
+        return;
+
     if (m_webPage) {
         flushPendingFrameCallbacks();
         flushFrameCallbacks();
@@ -563,6 +566,18 @@ void WaylandCompositor::bindSurfaceToWebPage(WaylandCompositor::Surface* surface
     m_pageMap.set(webPage, makeWeakPtr(*surface));
 }
 
+void WaylandCompositor::bindWebPage(WebPageProxy& webPage)
+{
+    if (WeakPtr<Surface> surface = m_pageMap.get(&webPage))
+        surface->setWebPage(&webPage);
+}
+
+void WaylandCompositor::unbindWebPage(WebPageProxy& webPage)
+{
+    if (WeakPtr<Surface> surface = m_pageMap.get(&webPage))
+        surface->setWebPage(nullptr);
+}
+
 void WaylandCompositor::registerWebPage(WebPageProxy& webPage)
 {
     m_pageMap.add(&webPage, nullptr);

diff --git a/Source/WebKit/UIProcess/gtk/WaylandCompositor.h b/Source/WebKit/UIProcess/gtk/WaylandCompositor.h
@@ -104,6 +104,8 @@ class WaylandCompositor {
     String displayName() const { return m_displayName; }
 
     void bindSurfaceToWebPage(Surface*, WebCore::PageIdentifier);
+    void bindWebPage(WebPageProxy&);
+    void unbindWebPage(WebPageProxy&);
     void registerWebPage(WebPageProxy&);
     void unregisterWebPage(WebPageProxy&);
 

diff --git a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp
@@ -550,8 +550,7 @@ void DrawingAreaCoordinatedGraphics::enterAcceleratedCompositingMode(GraphicsLay
     if (m_previousLayerTreeHost) {
         m_layerTreeHost = WTFMove(m_previousLayerTreeHost);
         m_layerTreeHost->setIsDiscardable(false);
-        if (!m_isPaintingSuspended)
-            m_layerTreeHost->resumeRendering();
+        m_layerTreeHost->resumeRendering();
         if (!m_layerTreeStateIsFrozen)
             m_layerTreeHost->setLayerFlushSchedulingEnabled(true);
     } else {