Changeset 242597 in webkit

Timestamp:

Mar 7, 2019 9:17:16 AM (5 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION(r242364): [WPE] Do not stop the compositing run loop update timer on suspend
​https://bugs.webkit.org/show_bug.cgi?id=195410

Patch by Carlos Garcia Campos <​cgarcia@igalia.com> on 2019-03-07
Reviewed by Žan Doberšek.

Calling CompositingRunLoop::stopUpdates() on suspend is leaving the threaded compositor in an inconsistent
state, failing to resume and stopping the updates forever. This is causing timeouts in WPE layout tests. Instead
of calling stopUpdates(), a new suspend() is called, that stops the update timer, without changing the current
updae tha compositing state. A new method resume() is also added to schedule an update if needed.

• Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:

(WebKit::CompositingRunLoop::suspend): Set state as suspended and stop the update timer.
(WebKit::CompositingRunLoop::resume): Set state as not suspended and start the update timer if it was scheduled
while suspended.
(WebKit::CompositingRunLoop::scheduleUpdate): Do not start the update timer when suspended.
(WebKit::CompositingRunLoop::compositionCompleted): Ditto.
(WebKit::CompositingRunLoop::updateCompleted): Ditto.
(WebKit::CompositingRunLoop::updateTimerFired): Add an assert to ensure the update timer is not fired while suspended.

• Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
• Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:

(WebKit::ThreadedCompositor::suspend): Call CompositingRunLoop::suspend() instead of stopUpdates().
(WebKit::ThreadedCompositor::resume): Call CompositingRunLoop::resume().

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp (modified) (5 diffs)
• Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h (modified) (2 diffs)
• Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-03-07  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        REGRESSION(r242364): [WPE] Do not stop the compositing run loop update timer on suspend
+        https://bugs.webkit.org/show_bug.cgi?id=195410
+
+        Reviewed by Žan Doberšek.
+
+        Calling CompositingRunLoop::stopUpdates() on suspend is leaving the threaded compositor in an inconsistent
+        state, failing to resume and stopping the updates forever. This is causing timeouts in WPE layout tests. Instead
+        of calling stopUpdates(), a new suspend() is called, that stops the update timer, without changing the current
+        updae tha compositing state. A new method resume() is also added to schedule an update if needed.
+
+        * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:
+        (WebKit::CompositingRunLoop::suspend): Set state as suspended and stop the update timer.
+        (WebKit::CompositingRunLoop::resume): Set state as not suspended and start the update timer if it was scheduled
+        while suspended.
+        (WebKit::CompositingRunLoop::scheduleUpdate): Do not start the update timer when suspended.
+        (WebKit::CompositingRunLoop::compositionCompleted): Ditto.
+        (WebKit::CompositingRunLoop::updateCompleted): Ditto.
+        (WebKit::CompositingRunLoop::updateTimerFired): Add an assert to ensure the update timer is not fired while suspended.
+        * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
+        * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
+        (WebKit::ThreadedCompositor::suspend): Call CompositingRunLoop::suspend() instead of stopUpdates().
+        (WebKit::ThreadedCompositor::resume): Call CompositingRunLoop::resume().
+
 2019-03-06  Mark Lam  <mark.lam@apple.com>
 

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

@@ -95 +95 @@
 }
 
+void CompositingRunLoop::suspend()
+{
+    LockHolder stateLocker(m_state.lock);
+    m_state.isSuspended = true;
+    m_updateTimer.stop();
+}
+
+void CompositingRunLoop::resume()
+{
+    LockHolder stateLocker(m_state.lock);
+    m_state.isSuspended = false;
+    if (m_state.update == UpdateState::Scheduled)
+        m_updateTimer.startOneShot(0_s);
+}
+
 void CompositingRunLoop::scheduleUpdate()
 {
@@ -114 +129 @@
     case UpdateState::Idle:
         m_state.update = UpdateState::Scheduled;
-        m_updateTimer.startOneShot(0_s);
+        if (!m_state.isSuspended)
+            m_updateTimer.startOneShot(0_s);
         return;
     case UpdateState::Scheduled:
@@ -157 +173 @@
             m_state.pendingUpdate = false;
             m_state.update = UpdateState::Scheduled;
-            m_updateTimer.startOneShot(0_s);
+            if (!m_state.isSuspended)
+                m_updateTimer.startOneShot(0_s);
             return;
         }
@@ -189 +206 @@
             m_state.pendingUpdate = false;
             m_state.update = UpdateState::Scheduled;
-            m_updateTimer.startOneShot(0_s);
+            if (!m_state.isSuspended)
+                m_updateTimer.startOneShot(0_s);
             return;
         }
@@ -205 +223 @@
         // Both composition and scene update are now in progress.
         LockHolder locker(m_state.lock);
+        ASSERT(!m_state.isSuspended);
         m_state.composition = CompositionState::InProgress;
         m_state.update = UpdateState::InProgress;

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

@@ -48 +48 @@
     void performTaskSync(Function<void ()>&&);
 
+    void suspend();
+    void resume();
+
     Lock& stateLock() { return m_state.lock; }
 
@@ -82 +85 @@
         UpdateState update { UpdateState::Idle };
         bool pendingUpdate { false };
+        bool isSuspended { false };
     } m_state;
 };

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

@@ -122 +122 @@
         return;
 
-    m_compositingRunLoop->stopUpdates();
+    m_compositingRunLoop->suspend();
     m_compositingRunLoop->performTaskSync([this, protectedThis = makeRef(*this)] {
         m_scene->setActive(false);
@@ -137 +137 @@
         m_scene->setActive(true);
     });
+    m_compositingRunLoop->resume();
 }