Changeset 192510 in webkit

Timestamp:

Nov 17, 2015 12:37:21 AM (8 years ago)

Author:

Carlos Garcia Campos

Message:

[GTK] Web Process crashes on reparenting a WebView with AC mode on
​https://bugs.webkit.org/show_bug.cgi?id=151139

Reviewed by Mario Sanchez Prada.

When the web view is reparented, the widget is first unrealized,
and then realized again when added to the new parent. In the
second realize, the old redirected XComposite window is destroyed
and a new one is created, but the web process is still using the
old redirected window ID. As soon as the redirected window is
destroyed and the web process tries to use the window ID, it
crashes due to a BadDrawable X error. We have to notify the web
process as soon as the web view is unrealized to stop using the
current window ID and exit accelerated compositing mode until a
new window ID is given. This notification needs to be synchronous,
because the window can be destroyed in the UI process before the
message is received in the web process.

• UIProcess/API/gtk/WebKitWebViewBase.cpp:

(webkitWebViewBaseRealize): Add an assert to ensure we never have
a redirected window when the view is realized. Also check drawing
area is not nullptr, since it can be destroyed at any time if the
web process crashes.
(webkitWebViewBaseUnrealize): Call
DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing()
and destroy the redirected XComposite window.

• UIProcess/DrawingAreaProxyImpl.cpp:

(WebKit::DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing):
Send DestroyNativeSurfaceHandleForCompositing synchronous messsage
to the web process.

• UIProcess/DrawingAreaProxyImpl.h:
• WebProcess/WebPage/DrawingArea.h:
• WebProcess/WebPage/DrawingArea.messages.in: Add

DestroyNativeSurfaceHandleForCompositing message.

• WebProcess/WebPage/DrawingAreaImpl.cpp:

(WebKit::DrawingAreaImpl::destroyNativeSurfaceHandleForCompositing):
Set the native surface handler for compositing to 0 to reset it.

• WebProcess/WebPage/DrawingAreaImpl.h:
• WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:

(WebKit::LayerTreeHostGtk::makeContextCurrent): Return false
early always when layer tree context ID is 0, even if we already
have a context.
(WebKit::LayerTreeHostGtk::setNativeSurfaceHandleForCompositing):
Cancel any pending layer flush when setting a new handler.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/gtk/WebKitWebViewBase.cpp (modified) (4 diffs)
• UIProcess/DrawingAreaProxyImpl.cpp (modified) (1 diff)
• UIProcess/DrawingAreaProxyImpl.h (modified) (1 diff)
• WebProcess/WebPage/DrawingArea.h (modified) (2 diffs)
• WebProcess/WebPage/DrawingArea.messages.in (modified) (1 diff)
• WebProcess/WebPage/DrawingAreaImpl.cpp (modified) (1 diff)
• WebProcess/WebPage/DrawingAreaImpl.h (modified) (1 diff)
• WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp (modified) (2 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-11-17  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Web Process crashes on reparenting a WebView with AC mode on
+        https://bugs.webkit.org/show_bug.cgi?id=151139
+
+        Reviewed by Mario Sanchez Prada.
+
+        When the web view is reparented, the widget is first unrealized,
+        and then realized again when added to the new parent. In the
+        second realize, the old redirected XComposite window is destroyed
+        and a new one is created, but the web process is still using the
+        old redirected window ID. As soon as the redirected window is
+        destroyed and the web process tries to use the window ID, it
+        crashes due to a BadDrawable X error. We have to notify the web
+        process as soon as the web view is unrealized to stop using the
+        current window ID and exit accelerated compositing mode until a
+        new window ID is given. This notification needs to be synchronous,
+        because the window can be destroyed in the UI process before the
+        message is received in the web process.
+
+        * UIProcess/API/gtk/WebKitWebViewBase.cpp:
+        (webkitWebViewBaseRealize): Add an assert to ensure we never have
+        a redirected window when the view is realized. Also check drawing
+        area is not nullptr, since it can be destroyed at any time if the
+        web process crashes.
+        (webkitWebViewBaseUnrealize): Call
+        DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing()
+        and destroy the redirected XComposite window.
+        * UIProcess/DrawingAreaProxyImpl.cpp:
+        (WebKit::DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing):
+        Send DestroyNativeSurfaceHandleForCompositing synchronous messsage
+        to the web process.
+        * UIProcess/DrawingAreaProxyImpl.h:
+        * WebProcess/WebPage/DrawingArea.h:
+        * WebProcess/WebPage/DrawingArea.messages.in: Add
+        DestroyNativeSurfaceHandleForCompositing message.
+        * WebProcess/WebPage/DrawingAreaImpl.cpp:
+        (WebKit::DrawingAreaImpl::destroyNativeSurfaceHandleForCompositing):
+        Set the native surface handler for compositing to 0 to reset it.
+        * WebProcess/WebPage/DrawingAreaImpl.h:
+        * WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp:
+        (WebKit::LayerTreeHostGtk::makeContextCurrent): Return false
+        early always when layer tree context ID is 0, even if we already
+        have a context.
+        (WebKit::LayerTreeHostGtk::setNativeSurfaceHandleForCompositing):
+        Cancel any pending layer flush when setting a new handler.
+
 2015-11-16  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp

@@ -281 +281 @@
 #if USE(REDIRECTED_XCOMPOSITE_WINDOW)
     if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::X11) {
+        ASSERT(!priv->redirectedWindow);
         priv->redirectedWindow = RedirectedXCompositeWindow::create(
             gtk_widget_get_parent_window(widget),
@@ -289 +290 @@
             });
         if (priv->redirectedWindow) {
-            DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea());
-            drawingArea->setNativeSurfaceHandleForCompositing(priv->redirectedWindow->windowID());
+            if (DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea()))
+                drawingArea->setNativeSurfaceHandleForCompositing(priv->redirectedWindow->windowID());
         }
     }
@@ -330 +331 @@
 
 #if USE(TEXTURE_MAPPER) && PLATFORM(X11) && !USE(REDIRECTED_XCOMPOSITE_WINDOW)
-    DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea());
-    drawingArea->setNativeSurfaceHandleForCompositing(GDK_WINDOW_XID(window));
+    if (DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(priv->pageProxy->drawingArea()))
+        drawingArea->setNativeSurfaceHandleForCompositing(GDK_WINDOW_XID(window));
 #endif
 
@@ -346 +347 @@
 {
     WebKitWebViewBase* webView = WEBKIT_WEB_VIEW_BASE(widget);
+#if USE(TEXTURE_MAPPER) && PLATFORM(X11)
+    if (DrawingAreaProxyImpl* drawingArea = static_cast<DrawingAreaProxyImpl*>(webView->priv->pageProxy->drawingArea()))
+        drawingArea->destroyNativeSurfaceHandleForCompositing();
+#endif
+#if USE(REDIRECTED_XCOMPOSITE_WINDOW)
+    webView->priv->redirectedWindow = nullptr;
+#endif
     gtk_im_context_set_client_window(webView->priv->inputMethodFilter.context(), nullptr);
 

trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp

@@ -315 +315 @@
     m_webPageProxy.process().send(Messages::DrawingArea::SetNativeSurfaceHandleForCompositing(handle), m_webPageProxy.pageID());
 }
+
+void DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing()
+{
+    bool handled;
+    m_webPageProxy.process().sendSync(Messages::DrawingArea::DestroyNativeSurfaceHandleForCompositing(), Messages::DrawingArea::DestroyNativeSurfaceHandleForCompositing::Reply(handled), m_webPageProxy.pageID());
+}
 #endif
 

trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h

@@ -51 +51 @@
 #if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
     void setNativeSurfaceHandleForCompositing(uint64_t);
+    void destroyNativeSurfaceHandleForCompositing();
 #endif
 

trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.h

@@ -151 +151 @@
     // IPC::MessageReceiver.
     virtual void didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) override;
+    virtual void didReceiveSyncMessage(IPC::Connection&, IPC::MessageDecoder&, std::unique_ptr<IPC::MessageEncoder>&) override;
 
     // Message handlers.
@@ -171 +172 @@
 #if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
     virtual void setNativeSurfaceHandleForCompositing(uint64_t) = 0;
+    virtual void destroyNativeSurfaceHandleForCompositing(bool&) = 0;
 #endif
 };

trunk/Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in

@@ -43 +43 @@
 #if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
     SetNativeSurfaceHandleForCompositing(uint64_t handle)
+    DestroyNativeSurfaceHandleForCompositing() -> (bool handled)
 #endif
 }

trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp

@@ -700 +700 @@
     }
 }
+
+void DrawingAreaImpl::destroyNativeSurfaceHandleForCompositing(bool& handled)
+{
+    handled = true;
+    setNativeSurfaceHandleForCompositing(0);
+}
 #endif
 

trunk/Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.h

@@ -73 +73 @@
 #if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
     virtual void setNativeSurfaceHandleForCompositing(uint64_t) override;
+    virtual void destroyNativeSurfaceHandleForCompositing(bool&) override;
 #endif
 

trunk/Source/WebKit2/WebProcess/WebPage/gtk/LayerTreeHostGtk.cpp

@@ -146 +146 @@
 bool LayerTreeHostGtk::makeContextCurrent()
 {
+    if (!m_layerTreeContext.contextID) {
+        m_context = nullptr;
+        return false;
+    }
+
     if (!m_context) {
-        if (!m_layerTreeContext.contextID)
-            return false;
-
         m_context = GLContext::createContextForWindow(reinterpret_cast<GLNativeWindowType>(m_layerTreeContext.contextID), GLContext::sharingContext());
         if (!m_context)
@@ -417 +419 @@
 void LayerTreeHostGtk::setNativeSurfaceHandleForCompositing(uint64_t handle)
 {
+    cancelPendingLayerFlush();
     m_layerTreeContext.contextID = handle;