<!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>[202040] 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/202040">202040</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-06-14 02:58:06 -0700 (Tue, 14 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[ThreadedCompositor] Opening the inspector in a window causes a crash.
https://bugs.webkit.org/show_bug.cgi?id=154444
Reviewed by Žan Doberšek.
The threaded compositor doesn't handle the case of changing or removing the native surface handle. When the web
view is reparented, the current native surface handle is destroyed when the view is removed from the parent, and
a new one is created when added to the new parent. We need to handle this case in the threaded compositor.
* Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:
(WebKit::CompositingRunLoop::stopUpdateTimer): Allow users to stop the update timer.
* Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
* Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
(WebKit::ThreadedCompositor::setNativeSurfaceHandleForCompositing): Use performTaskSync because this is called
from a synchronous IPC message and right after it returns, the current native surface is destroyed by the UI
process. So we need to ensure we finish all pending operations for the current native surface in the compositing
thread before it's destroyed. Then we enable or disable the scene depending on whether the native surface has
been created or destroyed and destroy the current context in case the new handle is 0.
(WebKit::ThreadedCompositor::tryEnsureGLContext): Just renamed to make it clear that it can fail.
(WebKit::ThreadedCompositor::glContext):
(WebKit::ThreadedCompositor::renderLayerTree): Return early if scene is not active.
* WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp:
(WebKit::ThreadedCoordinatedLayerTreeHost::setNativeSurfaceHandleForCompositing): Schedule a new layer flush
after the native surface handle changed.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2SharedCoordinatedGraphicsthreadedcompositorCompositingRunLoopcpp">trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp</a></li>
<li><a href="#trunkSourceWebKit2SharedCoordinatedGraphicsthreadedcompositorCompositingRunLooph">trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h</a></li>
<li><a href="#trunkSourceWebKit2SharedCoordinatedGraphicsthreadedcompositorThreadedCompositorcpp">trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp</a></li>
<li><a href="#trunkSourceWebKit2SharedCoordinatedGraphicsthreadedcompositorThreadedCompositorh">trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebPageCoordinatedGraphicsThreadedCoordinatedLayerTreeHostcpp">trunk/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (202039 => 202040)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2016-06-14 08:08:43 UTC (rev 202039)
+++ trunk/Source/WebKit2/ChangeLog 2016-06-14 09:58:06 UTC (rev 202040)
</span><span class="lines">@@ -1,5 +1,32 @@
</span><span class="cx"> 2016-06-14 Carlos Garcia Campos <cgarcia@igalia.com>
</span><span class="cx">
</span><ins>+ [ThreadedCompositor] Opening the inspector in a window causes a crash.
+ https://bugs.webkit.org/show_bug.cgi?id=154444
+
+ Reviewed by Žan Doberšek.
+
+ The threaded compositor doesn't handle the case of changing or removing the native surface handle. When the web
+ view is reparented, the current native surface handle is destroyed when the view is removed from the parent, and
+ a new one is created when added to the new parent. We need to handle this case in the threaded compositor.
+
+ * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:
+ (WebKit::CompositingRunLoop::stopUpdateTimer): Allow users to stop the update timer.
+ * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
+ * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
+ (WebKit::ThreadedCompositor::setNativeSurfaceHandleForCompositing): Use performTaskSync because this is called
+ from a synchronous IPC message and right after it returns, the current native surface is destroyed by the UI
+ process. So we need to ensure we finish all pending operations for the current native surface in the compositing
+ thread before it's destroyed. Then we enable or disable the scene depending on whether the native surface has
+ been created or destroyed and destroy the current context in case the new handle is 0.
+ (WebKit::ThreadedCompositor::tryEnsureGLContext): Just renamed to make it clear that it can fail.
+ (WebKit::ThreadedCompositor::glContext):
+ (WebKit::ThreadedCompositor::renderLayerTree): Return early if scene is not active.
+ * WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp:
+ (WebKit::ThreadedCoordinatedLayerTreeHost::setNativeSurfaceHandleForCompositing): Schedule a new layer flush
+ after the native surface handle changed.
+
+2016-06-14 Carlos Garcia Campos <cgarcia@igalia.com>
+
</ins><span class="cx"> [Threaded Compositor] Modernize and simplify threaded compositor code
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=158615
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedCoordinatedGraphicsthreadedcompositorCompositingRunLoopcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp (202039 => 202040)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp 2016-06-14 08:08:43 UTC (rev 202039)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp 2016-06-14 09:58:06 UTC (rev 202040)
</span><span class="lines">@@ -71,6 +71,11 @@
</span><span class="cx"> m_updateTimer.startOneShot(nextUpdateTime);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+void CompositingRunLoop::stopUpdateTimer()
+{
+ m_updateTimer.stop();
+}
+
</ins><span class="cx"> void CompositingRunLoop::updateTimerFired()
</span><span class="cx"> {
</span><span class="cx"> m_updateFunction();
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedCoordinatedGraphicsthreadedcompositorCompositingRunLooph"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h (202039 => 202040)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h 2016-06-14 08:08:43 UTC (rev 202039)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h 2016-06-14 09:58:06 UTC (rev 202040)
</span><span class="lines">@@ -50,6 +50,7 @@
</span><span class="cx"> void performTaskSync(NoncopyableFunction<void ()>&&);
</span><span class="cx">
</span><span class="cx"> void startUpdateTimer(UpdateTiming = Immediate);
</span><ins>+ void stopUpdateTimer();
</ins><span class="cx">
</span><span class="cx"> void run();
</span><span class="cx"> void stop();
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedCoordinatedGraphicsthreadedcompositorThreadedCompositorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (202039 => 202040)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2016-06-14 08:08:43 UTC (rev 202039)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2016-06-14 09:58:06 UTC (rev 202040)
</span><span class="lines">@@ -61,9 +61,15 @@
</span><span class="cx">
</span><span class="cx"> void ThreadedCompositor::setNativeSurfaceHandleForCompositing(uint64_t handle)
</span><span class="cx"> {
</span><del>- m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), handle] {
</del><ins>+ m_compositingRunLoop->stopUpdateTimer();
+ m_compositingRunLoop->performTaskSync([this, protectedThis = Ref<ThreadedCompositor>(*this), handle] {
+ m_scene->setActive(!!handle);
+
+ // A new native handle can't be set without destroying the previous one first if any.
+ ASSERT(!!handle ^ !!m_nativeSurfaceHandle);
</ins><span class="cx"> m_nativeSurfaceHandle = handle;
</span><del>- m_scene->setActive(true);
</del><ins>+ if (!m_nativeSurfaceHandle)
+ m_context = nullptr;
</ins><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -130,7 +136,7 @@
</span><span class="cx"> m_client->commitScrollOffset(layerID, offset);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool ThreadedCompositor::ensureGLContext()
</del><ins>+bool ThreadedCompositor::tryEnsureGLContext()
</ins><span class="cx"> {
</span><span class="cx"> if (!glContext())
</span><span class="cx"> return false;
</span><span class="lines">@@ -156,7 +162,7 @@
</span><span class="cx"> return m_context.get();
</span><span class="cx">
</span><span class="cx"> if (!m_nativeSurfaceHandle)
</span><del>- return 0;
</del><ins>+ return nullptr;
</ins><span class="cx">
</span><span class="cx"> m_context = GLContext::createContextForWindow(reinterpret_cast<GLNativeWindowType>(m_nativeSurfaceHandle), GLContext::sharingContext());
</span><span class="cx"> return m_context.get();
</span><span class="lines">@@ -181,10 +187,10 @@
</span><span class="cx"> void ThreadedCompositor::renderLayerTree()
</span><span class="cx"> {
</span><span class="cx"> ASSERT(&RunLoop::current() == &m_compositingRunLoop->runLoop());
</span><del>- if (!m_scene)
</del><ins>+ if (!m_scene || !m_scene->isActive())
</ins><span class="cx"> return;
</span><span class="cx">
</span><del>- if (!ensureGLContext())
</del><ins>+ if (!tryEnsureGLContext())
</ins><span class="cx"> return;
</span><span class="cx">
</span><span class="cx"> FloatRect clipRect(0, 0, m_viewportSize.width(), m_viewportSize.height());
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedCoordinatedGraphicsthreadedcompositorThreadedCompositorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h (202039 => 202040)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h 2016-06-14 08:08:43 UTC (rev 202039)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h 2016-06-14 09:58:06 UTC (rev 202040)
</span><span class="lines">@@ -88,7 +88,7 @@
</span><span class="cx"> void scheduleDisplayImmediately();
</span><span class="cx"> void didChangeVisibleRect() override;
</span><span class="cx">
</span><del>- bool ensureGLContext();
</del><ins>+ bool tryEnsureGLContext();
</ins><span class="cx"> WebCore::GLContext* glContext();
</span><span class="cx">
</span><span class="cx"> void createCompositingThread();
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebPageCoordinatedGraphicsThreadedCoordinatedLayerTreeHostcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp (202039 => 202040)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp 2016-06-14 08:08:43 UTC (rev 202039)
+++ trunk/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp 2016-06-14 09:58:06 UTC (rev 202040)
</span><span class="lines">@@ -198,6 +198,7 @@
</span><span class="cx"> {
</span><span class="cx"> m_layerTreeContext.contextID = handle;
</span><span class="cx"> m_compositor->setNativeSurfaceHandleForCompositing(handle);
</span><ins>+ scheduleLayerFlush();
</ins><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>