Changeset 202040 in webkit

Timestamp:

Jun 14, 2016 2:58:06 AM (8 years ago)

Author:

Carlos Garcia Campos

Message:

[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.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp (modified) (1 diff)
• Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h (modified) (1 diff)
• Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (modified) (4 diffs)
• Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h (modified) (1 diff)
• WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp (modified) (1 diff)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp

@@ -72 +72 @@
 }
 
+void CompositingRunLoop::stopUpdateTimer()
+{
+    m_updateTimer.stop();
+}
+
 void CompositingRunLoop::updateTimerFired()
 {

trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h

@@ -51 +51 @@
 
     void startUpdateTimer(UpdateTiming = Immediate);
+    void stopUpdateTimer();
 
     void run();

trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp

@@ -62 +62 @@
 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;
     });
 }
@@ -131 +137 @@
 }
 
-bool ThreadedCompositor::ensureGLContext()
+bool ThreadedCompositor::tryEnsureGLContext()
 {
     if (!glContext())
@@ -157 +163 @@
 
     if (!m_nativeSurfaceHandle)
-        return 0;
+        return nullptr;
 
     m_context = GLContext::createContextForWindow(reinterpret_cast<GLNativeWindowType>(m_nativeSurfaceHandle), GLContext::sharingContext());
@@ -182 +188 @@
 {
     ASSERT(&RunLoop::current() == &m_compositingRunLoop->runLoop());
-    if (!m_scene)
+    if (!m_scene || !m_scene->isActive())
         return;
 
-    if (!ensureGLContext())
+    if (!tryEnsureGLContext())
         return;
 

trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h

@@ -89 +89 @@
     void didChangeVisibleRect() override;
 
-    bool ensureGLContext();
+    bool tryEnsureGLContext();
     WebCore::GLContext* glContext();
 

trunk/Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp

@@ -199 +199 @@
     m_layerTreeContext.contextID = handle;
     m_compositor->setNativeSurfaceHandleForCompositing(handle);
+    scheduleLayerFlush();
 }
 #endif