Blob Blame History Raw
45aabb992c69026ec12c857c023fec13920a753f
From 45aabb992c69026ec12c857c023fec13920a753f Mon Sep 17 00:00:00 2001
From: "carlosgc@webkit.org"
 <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue, 17 Sep 2019 08:05:30 +0000
Subject: [PATCH] [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().

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@249947 268f45cc-cd09-0410-ab3c-d52691b4dbfc
---
 .../UIProcess/API/gtk/WebKitWebViewBase.cpp   |  6 +++
 .../UIProcess/gtk/AcceleratedBackingStore.h   |  2 +
 .../gtk/AcceleratedBackingStoreWayland.cpp    | 52 ++++++++++++++-----
 .../gtk/AcceleratedBackingStoreWayland.h      |  2 +
 .../UIProcess/gtk/WaylandCompositor.cpp       | 15 ++++++
 .../WebKit/UIProcess/gtk/WaylandCompositor.h  |  2 +
 .../DrawingAreaCoordinatedGraphics.cpp        |  3 +-
 7 files changed, 66 insertions(+), 16 deletions(-)

diff --git a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
index 88b3cf43bfe..68ca3ad5b5b 100644
--- a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
+++ b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
@@ -422,6 +422,9 @@ 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)
@@ -429,6 +432,9 @@ 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
index 182264c6952..c6afb20b046 100644
--- a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h
+++ b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h
@@ -47,6 +47,8 @@ public:
 
     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
index 893279fbcf5..973b31681fb 100644
--- 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
index 5826bcce9bd..dffe23d8f3b 100644
--- a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h
+++ b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h
@@ -65,6 +65,8 @@ private:
 #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
index ce895293fee..8a14c552c87 100644
--- 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
index d40c2303257..e9a42189b73 100644
--- a/Source/WebKit/UIProcess/gtk/WaylandCompositor.h
+++ b/Source/WebKit/UIProcess/gtk/WaylandCompositor.h
@@ -104,6 +104,8 @@ public:
     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
index 010ace6213b..fa974c21650 100644
--- a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp
+++ b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/DrawingAreaCoordinatedGraphics.cpp
@@ -551,8 +551,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 {
-- 
2.21.0