[ThreadedCompositor] Opening the inspector in a window causes a crash.
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 09:58:06 +0000 (09:58 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 09:58:06 +0000 (09:58 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202040 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp

index e54f4c1..4cbb833 100644 (file)
@@ -1,5 +1,32 @@
 2016-06-14  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        [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>
+
         [Threaded Compositor] Modernize and simplify threaded compositor code
         https://bugs.webkit.org/show_bug.cgi?id=158615
 
index 5df5f33..efc7a38 100644 (file)
@@ -71,6 +71,11 @@ void CompositingRunLoop::startUpdateTimer(UpdateTiming timing)
     m_updateTimer.startOneShot(nextUpdateTime);
 }
 
+void CompositingRunLoop::stopUpdateTimer()
+{
+    m_updateTimer.stop();
+}
+
 void CompositingRunLoop::updateTimerFired()
 {
     m_updateFunction();
index 61fe2e8..a56f894 100644 (file)
@@ -50,6 +50,7 @@ public:
     void performTaskSync(NoncopyableFunction<void ()>&&);
 
     void startUpdateTimer(UpdateTiming = Immediate);
+    void stopUpdateTimer();
 
     void run();
     void stop();
index 4ae3451..25caef7 100644 (file)
@@ -61,9 +61,15 @@ ThreadedCompositor::~ThreadedCompositor()
 
 void ThreadedCompositor::setNativeSurfaceHandleForCompositing(uint64_t handle)
 {
-    m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), handle] {
+    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);
         m_nativeSurfaceHandle = handle;
-        m_scene->setActive(true);
+        if (!m_nativeSurfaceHandle)
+            m_context = nullptr;
     });
 }
 
@@ -130,7 +136,7 @@ void ThreadedCompositor::commitScrollOffset(uint32_t layerID, const IntSize& off
     m_client->commitScrollOffset(layerID, offset);
 }
 
-bool ThreadedCompositor::ensureGLContext()
+bool ThreadedCompositor::tryEnsureGLContext()
 {
     if (!glContext())
         return false;
@@ -156,7 +162,7 @@ GLContext* ThreadedCompositor::glContext()
         return m_context.get();
 
     if (!m_nativeSurfaceHandle)
-        return 0;
+        return nullptr;
 
     m_context = GLContext::createContextForWindow(reinterpret_cast<GLNativeWindowType>(m_nativeSurfaceHandle), GLContext::sharingContext());
     return m_context.get();
@@ -181,10 +187,10 @@ void ThreadedCompositor::didChangeVisibleRect()
 void ThreadedCompositor::renderLayerTree()
 {
     ASSERT(&RunLoop::current() == &m_compositingRunLoop->runLoop());
-    if (!m_scene)
+    if (!m_scene || !m_scene->isActive())
         return;
 
-    if (!ensureGLContext())
+    if (!tryEnsureGLContext())
         return;
 
     FloatRect clipRect(0, 0, m_viewportSize.width(), m_viewportSize.height());
index 3700322..2a1236a 100644 (file)
@@ -88,7 +88,7 @@ private:
     void scheduleDisplayImmediately();
     void didChangeVisibleRect() override;
 
-    bool ensureGLContext();
+    bool tryEnsureGLContext();
     WebCore::GLContext* glContext();
 
     void createCompositingThread();
index 0d50df0..6317da3 100644 (file)
@@ -198,6 +198,7 @@ void ThreadedCoordinatedLayerTreeHost::setNativeSurfaceHandleForCompositing(uint
 {
     m_layerTreeContext.contextID = handle;
     m_compositor->setNativeSurfaceHandleForCompositing(handle);
+    scheduleLayerFlush();
 }
 #endif