Title: [203676] trunk/Source/WebKit2
Revision
203676
Author
carlo...@webkit.org
Date
2016-07-24 23:33:42 -0700 (Sun, 24 Jul 2016)

Log Message

[GTK][Threaded Compositor] ASSERTION FAILED: !!handle ^ !!m_nativeSurfaceHandle with several layout tests
https://bugs.webkit.org/show_bug.cgi?id=160143

Reviewed by Michael Catanzaro.

We have a message to set the native surface handle and another one for destroying it, the former is a normal
message while the latter is sync. This assertion happens if the web view is realized before the web process is
launched. This is the sequence:

1.- DrawingAreaProxyImpl sends SetNativeSurfaceHandleForCompositing message to the web process, since the
process hasn't been launched yet, the message is queued.
2.- Web process is launched and queued messages are now sent to the web process.
3.- The page is closed right after the web process is launched, and DrawingAreaProxyImpl sends
DestroyNativeSurfaceHandleForCompositing to the web process.
4.- The web process processes incoming messages, and DestroyNativeSurfaceHandleForCompositing is processed before
SetNativeSurfaceHandleForCompositing because it's sync.
5.- The web process processes SetNativeSurfaceHandleForCompositing message.

This is not only producing the assertion, it's also setting a handle for a X window already destroyed in the UI
process, so this could be producing the X errors we have seen in other tests. So, we need to make sure
SetNativeSurfaceHandleForCompositing and DestroyNativeSurfaceHandleForCompositing are handled in order by the
web process. We could make SetNativeSurfaceHandleForCompositing sync as well, but sync messages are just ignored
when sent before the web process has been launched (only normal messages are queued for obvious reasons). The
other option is sending the SetNativeSurfaceHandleForCompositing message with the
IPC::DispatchMessageEvenWhenWaitingForSyncReply flag. In this case the message is queued and dispatched on
process launch, but it's dispatched before other messages also queued without that flag, like
CreateWebPage. Since there's no WebPage the web process doesn't find a valid message receiver for it and
it's discarded. We need to ensure the DrawinArea object has been created before sending the
SetNativeSurfaceHandleForCompositing with the PC::DispatchMessageEvenWhenWaitingForSyncReply flag.

* UIProcess/DrawingAreaProxyImpl.cpp:
(WebKit::DrawingAreaProxyImpl::didUpdateBackingStoreState): If we have received the first update and there's a
SetNativeSurfaceHandleForCompositing message pending, send it.
(WebKit::DrawingAreaProxyImpl::setNativeSurfaceHandleForCompositing): Do not send the message before the first
update is received.
(WebKit::DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing): If there was a
SetNativeSurfaceHandleForCompositing message pending, just ignore this destroy since the web process never
received the handle.
* UIProcess/DrawingAreaProxyImpl.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (203675 => 203676)


--- trunk/Source/WebKit2/ChangeLog	2016-07-25 06:28:35 UTC (rev 203675)
+++ trunk/Source/WebKit2/ChangeLog	2016-07-25 06:33:42 UTC (rev 203676)
@@ -1,3 +1,45 @@
+2016-07-24  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        [GTK][Threaded Compositor] ASSERTION FAILED: !!handle ^ !!m_nativeSurfaceHandle with several layout tests
+        https://bugs.webkit.org/show_bug.cgi?id=160143
+
+        Reviewed by Michael Catanzaro.
+
+        We have a message to set the native surface handle and another one for destroying it, the former is a normal
+        message while the latter is sync. This assertion happens if the web view is realized before the web process is
+        launched. This is the sequence:
+
+        1.- DrawingAreaProxyImpl sends SetNativeSurfaceHandleForCompositing message to the web process, since the
+        process hasn't been launched yet, the message is queued.
+        2.- Web process is launched and queued messages are now sent to the web process.
+        3.- The page is closed right after the web process is launched, and DrawingAreaProxyImpl sends
+        DestroyNativeSurfaceHandleForCompositing to the web process.
+        4.- The web process processes incoming messages, and DestroyNativeSurfaceHandleForCompositing is processed before
+        SetNativeSurfaceHandleForCompositing because it's sync.
+        5.- The web process processes SetNativeSurfaceHandleForCompositing message.
+
+        This is not only producing the assertion, it's also setting a handle for a X window already destroyed in the UI
+        process, so this could be producing the X errors we have seen in other tests. So, we need to make sure
+        SetNativeSurfaceHandleForCompositing and DestroyNativeSurfaceHandleForCompositing are handled in order by the
+        web process. We could make SetNativeSurfaceHandleForCompositing sync as well, but sync messages are just ignored
+        when sent before the web process has been launched (only normal messages are queued for obvious reasons). The
+        other option is sending the SetNativeSurfaceHandleForCompositing message with the
+        IPC::DispatchMessageEvenWhenWaitingForSyncReply flag. In this case the message is queued and dispatched on
+        process launch, but it's dispatched before other messages also queued without that flag, like
+        CreateWebPage. Since there's no WebPage the web process doesn't find a valid message receiver for it and
+        it's discarded. We need to ensure the DrawinArea object has been created before sending the
+        SetNativeSurfaceHandleForCompositing with the PC::DispatchMessageEvenWhenWaitingForSyncReply flag.
+
+        * UIProcess/DrawingAreaProxyImpl.cpp:
+        (WebKit::DrawingAreaProxyImpl::didUpdateBackingStoreState): If we have received the first update and there's a
+        SetNativeSurfaceHandleForCompositing message pending, send it.
+        (WebKit::DrawingAreaProxyImpl::setNativeSurfaceHandleForCompositing): Do not send the message before the first
+        update is received.
+        (WebKit::DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing): If there was a
+        SetNativeSurfaceHandleForCompositing message pending, just ignore this destroy since the web process never
+        received the handle.
+        * UIProcess/DrawingAreaProxyImpl.h:
+
 2016-07-25  Philippe Normand  <pnorm...@igalia.com>
 
         Improve GDB backtrace generation for GTK/EFL

Modified: trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp (203675 => 203676)


--- trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp	2016-07-25 06:28:35 UTC (rev 203675)
+++ trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp	2016-07-25 06:33:42 UTC (rev 203676)
@@ -170,9 +170,17 @@
 
     if (m_nextBackingStoreStateID != m_currentBackingStoreStateID)
         sendUpdateBackingStoreState(RespondImmediately);
-    else
+    else {
         m_hasReceivedFirstUpdate = true;
 
+#if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
+        if (m_pendingNativeSurfaceHandleForCompositing) {
+            setNativeSurfaceHandleForCompositing(m_pendingNativeSurfaceHandleForCompositing);
+            m_pendingNativeSurfaceHandleForCompositing = 0;
+        }
+#endif
+    }
+
     if (isInAcceleratedCompositingMode()) {
         ASSERT(!m_backingStore);
         return;
@@ -310,11 +318,19 @@
 #if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
 void DrawingAreaProxyImpl::setNativeSurfaceHandleForCompositing(uint64_t handle)
 {
-    m_webPageProxy.process().send(Messages::DrawingArea::SetNativeSurfaceHandleForCompositing(handle), m_webPageProxy.pageID());
+    if (!m_hasReceivedFirstUpdate) {
+        m_pendingNativeSurfaceHandleForCompositing = handle;
+        return;
+    }
+    m_webPageProxy.process().send(Messages::DrawingArea::SetNativeSurfaceHandleForCompositing(handle), m_webPageProxy.pageID(), IPC::DispatchMessageEvenWhenWaitingForSyncReply);
 }
 
 void DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing()
 {
+    if (m_pendingNativeSurfaceHandleForCompositing) {
+        m_pendingNativeSurfaceHandleForCompositing = 0;
+        return;
+    }
     bool handled;
     m_webPageProxy.process().sendSync(Messages::DrawingArea::DestroyNativeSurfaceHandleForCompositing(), Messages::DrawingArea::DestroyNativeSurfaceHandleForCompositing::Reply(handled), m_webPageProxy.pageID());
 }

Modified: trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h (203675 => 203676)


--- trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h	2016-07-25 06:28:35 UTC (rev 203675)
+++ trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h	2016-07-25 06:33:42 UTC (rev 203676)
@@ -109,6 +109,10 @@
     std::unique_ptr<BackingStore> m_backingStore;
 
     RunLoop::Timer<DrawingAreaProxyImpl> m_discardBackingStoreTimer;
+
+#if USE(TEXTURE_MAPPER) && PLATFORM(GTK)
+    uint64_t m_pendingNativeSurfaceHandleForCompositing { 0 };
+#endif
 };
 
 } // namespace WebKit
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to