<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[203676] trunk/Source/WebKit2</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/203676">203676</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-07-24 23:33:42 -0700 (Sun, 24 Jul 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>[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:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2UIProcessDrawingAreaProxyImplcpp">trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp</a></li>
<li><a href="#trunkSourceWebKit2UIProcessDrawingAreaProxyImplh">trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (203675 => 203676)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,45 @@
</span><ins>+2016-07-24  Carlos Garcia Campos  &lt;cgarcia@igalia.com&gt;
+
+        [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:
+
</ins><span class="cx"> 2016-07-25  Philippe Normand  &lt;pnormand@igalia.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Improve GDB backtrace generation for GTK/EFL
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessDrawingAreaProxyImplcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp (203675 => 203676)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -170,9 +170,17 @@
</span><span class="cx"> 
</span><span class="cx">     if (m_nextBackingStoreStateID != m_currentBackingStoreStateID)
</span><span class="cx">         sendUpdateBackingStoreState(RespondImmediately);
</span><del>-    else
</del><ins>+    else {
</ins><span class="cx">         m_hasReceivedFirstUpdate = true;
</span><span class="cx"> 
</span><ins>+#if USE(TEXTURE_MAPPER) &amp;&amp; PLATFORM(GTK)
+        if (m_pendingNativeSurfaceHandleForCompositing) {
+            setNativeSurfaceHandleForCompositing(m_pendingNativeSurfaceHandleForCompositing);
+            m_pendingNativeSurfaceHandleForCompositing = 0;
+        }
+#endif
+    }
+
</ins><span class="cx">     if (isInAcceleratedCompositingMode()) {
</span><span class="cx">         ASSERT(!m_backingStore);
</span><span class="cx">         return;
</span><span class="lines">@@ -310,11 +318,19 @@
</span><span class="cx"> #if USE(TEXTURE_MAPPER) &amp;&amp; PLATFORM(GTK)
</span><span class="cx"> void DrawingAreaProxyImpl::setNativeSurfaceHandleForCompositing(uint64_t handle)
</span><span class="cx"> {
</span><del>-    m_webPageProxy.process().send(Messages::DrawingArea::SetNativeSurfaceHandleForCompositing(handle), m_webPageProxy.pageID());
</del><ins>+    if (!m_hasReceivedFirstUpdate) {
+        m_pendingNativeSurfaceHandleForCompositing = handle;
+        return;
+    }
+    m_webPageProxy.process().send(Messages::DrawingArea::SetNativeSurfaceHandleForCompositing(handle), m_webPageProxy.pageID(), IPC::DispatchMessageEvenWhenWaitingForSyncReply);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void DrawingAreaProxyImpl::destroyNativeSurfaceHandleForCompositing()
</span><span class="cx"> {
</span><ins>+    if (m_pendingNativeSurfaceHandleForCompositing) {
+        m_pendingNativeSurfaceHandleForCompositing = 0;
+        return;
+    }
</ins><span class="cx">     bool handled;
</span><span class="cx">     m_webPageProxy.process().sendSync(Messages::DrawingArea::DestroyNativeSurfaceHandleForCompositing(), Messages::DrawingArea::DestroyNativeSurfaceHandleForCompositing::Reply(handled), m_webPageProxy.pageID());
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessDrawingAreaProxyImplh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/DrawingAreaProxyImpl.h (203675 => 203676)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -109,6 +109,10 @@
</span><span class="cx">     std::unique_ptr&lt;BackingStore&gt; m_backingStore;
</span><span class="cx"> 
</span><span class="cx">     RunLoop::Timer&lt;DrawingAreaProxyImpl&gt; m_discardBackingStoreTimer;
</span><ins>+
+#if USE(TEXTURE_MAPPER) &amp;&amp; PLATFORM(GTK)
+    uint64_t m_pendingNativeSurfaceHandleForCompositing { 0 };
+#endif
</ins><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> } // namespace WebKit
</span></span></pre>
</div>
</div>

</body>
</html>