[webkit-changes] [204070] branches/safari-602-branch/Source/WebKit2

Tue, 02 Aug 2016 23:16:39 -0700

Title: [204070] branches/safari-602-branch/Source/WebKit2

Diff

Modified: branches/safari-602-branch/Source/WebKit2/ChangeLog (204069 => 204070)


--- branches/safari-602-branch/Source/WebKit2/ChangeLog	2016-08-03 06:15:56 UTC (rev 204069)
+++ branches/safari-602-branch/Source/WebKit2/ChangeLog	2016-08-03 06:15:59 UTC (rev 204070)
@@ -1,3 +1,48 @@
+2016-08-02  Babak Shafiei  <[email protected]>
+
+        Merge r204057. rdar://problem/27534205
+
+    2016-08-02  Tim Horton  <[email protected]>
+
+            REGRESSION (r203385): Frequent RELEASE_ASSERT in WebKit::RemoteLayerTreeDrawingArea::flushLayers()
+            https://bugs.webkit.org/show_bug.cgi?id=160481
+            <rdar://problem/27534205>
+
+            Reviewed by Simon Fraser.
+
+            * UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:
+            * UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:
+            (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
+            (WebKit::RemoteLayerTreeDrawingAreaProxy::didRefreshDisplay):
+            (WebKit::RemoteLayerTreeDrawingAreaProxy::waitForDidUpdateViewState):
+            If the UI process sends a didUpdate message while the Web process is in
+            the middle of flushing on a background thread, the drawing area will
+            allow another commit to start on the main thread, which then (rightfully)
+            causes the RELEASE_ASSERT.
+
+            This is normally not a problem, because didRefreshDisplay (which sends the didUpdate)
+            bails if m_didUpdateMessageState is anything other than NotSent, and m_didUpdateMessageState
+            is only NotSent if the Web process has sent a commit (and thus will not commit again until
+            it gets a didUpdate). This is the fundamental mechanism that avoids multiple commits being
+            in flight at once.
+
+            In r203385, I added a path where didRefreshDisplay could be called
+            before the first commit arrived (by way of
+            _applicationWillEnterForeground -> viewStateDidChange -> waitForDidUpdateViewState).
+
+            This caused trouble because m_didUpdateMessageState is initialized to NotSent,
+            which means that we could end up sending a didUpdate immediately, before the first
+            commit arrives - even worse, while the first commit is being flushed on a background thread,
+            leading the aforementioned RELEASE_ASSERT to fire.
+
+            Instead, initialize it to Sent (which I've renamed to DoesNotNeedDidUpdate), so that
+            we won't send a didUpdate until after the first commit arrives (at which point
+            the two processes are in agreement about the order of things).
+
+            It's not currently possible to API test this for multiple reasons, though it is fairly
+            easy to write a test app that reproduces reliably (by simulating suspend/resume notifications
+            inside the didFinishNavigation: callback).
+
 2016-07-31  Babak Shafiei  <[email protected]>
 
         Merge r203842. rdar://problem/27553464

Modified: branches/safari-602-branch/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h (204069 => 204070)


--- branches/safari-602-branch/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h	2016-08-03 06:15:56 UTC (rev 204069)
+++ branches/safari-602-branch/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h	2016-08-03 06:15:59 UTC (rev 204070)
@@ -91,8 +91,8 @@
 
     RemoteLayerTreeHost m_remoteLayerTreeHost;
     bool m_isWaitingForDidUpdateGeometry { false };
-    enum DidUpdateMessageState { NotSent, Sent, MissedCommit };
-    DidUpdateMessageState m_didUpdateMessageState { NotSent };
+    enum DidUpdateMessageState { DoesNotNeedDidUpdate, NeedsDidUpdate, MissedCommit };
+    DidUpdateMessageState m_didUpdateMessageState { DoesNotNeedDidUpdate };
 
     WebCore::IntSize m_lastSentSize;

Modified: branches/safari-602-branch/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm (204069 => 204070)


--- branches/safari-602-branch/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm	2016-08-03 06:15:56 UTC (rev 204069)
+++ branches/safari-602-branch/Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm	2016-08-03 06:15:59 UTC (rev 204070)
@@ -227,11 +227,11 @@
     m_webPageProxy.layerTreeCommitComplete();
 
 #if PLATFORM(IOS)
-    if (std::exchange(m_didUpdateMessageState, NotSent) == MissedCommit)
+    if (std::exchange(m_didUpdateMessageState, NeedsDidUpdate) == MissedCommit)
         didRefreshDisplay(monotonicallyIncreasingTime());
     [m_displayLinkHandler schedule];
 #else
-    m_didUpdateMessageState = NotSent;
+    m_didUpdateMessageState = NeedsDidUpdate;
     didRefreshDisplay(monotonicallyIncreasingTime());
 #endif
 
@@ -399,7 +399,7 @@
     if (!m_webPageProxy.isValid())
         return;
 
-    if (m_didUpdateMessageState != NotSent) {
+    if (m_didUpdateMessageState != NeedsDidUpdate) {
         m_didUpdateMessageState = MissedCommit;
 #if PLATFORM(IOS)
         [m_displayLinkHandler pause];
@@ -407,7 +407,7 @@
         return;
     }
     
-    m_didUpdateMessageState = Sent;
+    m_didUpdateMessageState = DoesNotNeedDidUpdate;
 
     TraceScope tracingScope(RAFDidRefreshDisplayStart, RAFDidRefreshDisplayEnd);
 
@@ -425,7 +425,7 @@
 {
     // We must send the didUpdate message before blocking on the next commit, otherwise
     // we can be guaranteed that the next commit won't come until after the waitForAndDispatchImmediately times out.
-    if (m_didUpdateMessageState != Sent)
+    if (m_didUpdateMessageState != DoesNotNeedDidUpdate)
         didRefreshDisplay(monotonicallyIncreasingTime());
 
     static std::chrono::milliseconds viewStateUpdateTimeout = [] {

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to