Bug 1593170. Make the reftest harness deal with SynchronizeForSnapshot and FlushRendering returning promises. r=mattwoodrow

The code comment mostly explains the design. Basically, we force nothing to happen while we wait for the promises to finish and instead record what we need to do once the promise is finished, and do those pending tasks when it's finished.

Differential Revision: https://phabricator.services.mozilla.com/D51344

--HG--
extra : moz-landing-system : lando

layout/tools/reftest/reftest-content.js

@@ -530,10 +530,118 @@ function FlushRendering(aFlushMode) {
 }
 
 function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, forURL) {
+    // WaitForTestEnd works via the MakeProgress function below. It is responsible for
+    // moving through the states listed above and calling FlushRendering. We also listen
+    // for a number of events, the most important of which is the AfterPaintListener,
+    // which is responsible for updating the canvas after paints. In a fission world
+    // FlushRendering and updating the canvas must necessarily be async operations.
+    // During these async operations we want to wait for them to finish and we don't
+    // want to try to do anything else (what would we even want to do while only some of
+    // the processes involved have flushed layout or updated their layer trees?). So
+    // we call OperationInProgress whenever we are about to go back to the event loop
+    // during one of these calls, and OperationCompleted when it finishes. This prevents
+    // anything else from running while we wait and getting us into a confused state. We
+    // then record anything that happens while we are waiting to make sure that the
+    // right actions are triggered. The possible actions are basically calling
+    // MakeProgress from a setTimeout, and updating the canvas for an after paint event.
+    // The after paint listener just stashes the rects and we update them after a
+    // completed MakeProgress call. This is handled by
+    // HandlePendingTasksAfterMakeProgress, which also waits for any pending after paint
+    // events. The general sequence of events is:
+    //   - MakeProgress
+    //   - HandlePendingTasksAfterMakeProgress
+    //     - wait for after paint event if one is pending
+    //     - update canvas for after paint events we have received
+    //   - MakeProgress
+    //   etc
+
+    var setTimeoutCallMakeProgressWhenComplete = false;
+
+    var operationInProgress = false;
+    function OperationInProgress() {
+        if (operationInProgress != false) {
+            LogWarning("Nesting atomic operations?");
+        }
+        operationInProgress = true;
+    }
+    function OperationCompleted() {
+        if (operationInProgress != true) {
+            LogWarning("Mismatched OperationInProgress/OperationCompleted calls?");
+        }
+        operationInProgress = false;
+        if (setTimeoutCallMakeProgressWhenComplete) {
+            setTimeoutCallMakeProgressWhenComplete = false;
+            setTimeout(CallMakeProgress, 0);
+        }
+    }
+    function AssertNoOperationInProgress() {
+        if (operationInProgress) {
+            LogWarning("AssertNoOperationInProgress but operationInProgress");
+        }
+    }
+
+    var updateCanvasPending = false;
+    var updateCanvasRects = []
+
     var stopAfterPaintReceived = false;
     var currentDoc = content.document;
     var state = STATE_WAITING_TO_FIRE_INVALIDATE_EVENT;
 
+    var setTimeoutMakeProgressPending = false;
+
+    function CallSetTimeoutMakeProgress() {
+        if (setTimeoutMakeProgressPending) {
+            return;
+        }
+        setTimeoutMakeProgressPending = true;
+        setTimeout(CallMakeProgress, 0);
+    }
+
+    // This should only ever be called from a timeout.
+    function CallMakeProgress() {
+        if (operationInProgress) {
+            setTimeoutCallMakeProgressWhenComplete = true;
+            return;
+        }
+        setTimeoutMakeProgressPending = false;
+        MakeProgress();
+    }
+
+    var waitingForAnAfterPaint = false;
+
+    // Updates the canvas if there are pending updates for it. Checks if we
+    // need to call MakeProgress.
+    function HandlePendingTasksAfterMakeProgress() {
+        AssertNoOperationInProgress();
+
+        if ((state == STATE_WAITING_TO_FIRE_INVALIDATE_EVENT || state == STATE_WAITING_TO_FINISH) &&
+            shouldWaitForPendingPaints()) {
+            LogInfo("HandlePendingTasksAfterMakeProgress waiting for a MozAfterPaint");
+            // We are in a state where we wait for MozAfterPaint to clear and a
+            // MozAfterPaint event is pending, give it a chance to fire, but don't
+            // let anything else run.
+            waitingForAnAfterPaint = true;
+            OperationInProgress();
+            return;
+        }
+
+        if (updateCanvasPending) {
+            LogInfo("HandlePendingTasksAfterMakeProgress updating canvas");
+            updateCanvasPending = false;
+            let rects = updateCanvasRects;
+            updateCanvasRects = [];
+            OperationInProgress();
+            let promise = SendUpdateCanvasForEvent(forURL, rects, contentRootElement);
+            promise.then(function () {
+                OperationCompleted();
+                // After paint events are fired immediately after a paint (one
+                // of the things that can call us). Don't confuse ourselves by
+                // firing synchronously if we triggered the paint ourselves.
+                CallSetTimeoutMakeProgress();
+            });
+        }
+    }
+
     function AfterPaintListener(event) {
         LogInfo("AfterPaintListener in " + event.target.document.location.href);
         if (event.target.document != currentDoc) {
@@ -542,11 +650,27 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
             return;
         }
 
-        SendUpdateCanvasForEvent(forURL, event, contentRootElement);
-        // These events are fired immediately after a paint. Don't
-        // confuse ourselves by firing synchronously if we triggered the
-        // paint ourselves.
-        setTimeout(MakeProgress, 0);
+        updateCanvasPending = true;
+        for (let i = 0; i < event.clientRects.length; ++i) {
+            let r = event.clientRects[i];
+
+            // Copy the rect; it's content and we are chrome, which means if the
+            // document goes away (and it can in some crashtests) our reference
+            // to it will be turned into a dead wrapper that we can't acccess.
+            updateCanvasRects.push({ left: r.left, top: r.top, right: r.right, bottom: r.bottom });
+        }
+
+        if (waitingForAnAfterPaint) {
+            waitingForAnAfterPaint = false;
+            OperationCompleted();
+        }
+
+        if (!operationInProgress) {
+            HandlePendingTasksAfterMakeProgress();
+        }
+        // Otherwise we know that eventually after the operation finishes we
+        // will get a MakeProgress and/or HandlePendingTasksAfterMakeProgress
+        // call, so we don't need to do anything.
     }
 
     function AttrModifiedListener() {
@@ -555,14 +679,14 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
         // example, the attribute may have been modified in an subdocument's
         // load event handler, in which case we need load event processing
         // to complete and unsuppress painting before we check isMozAfterPaintPending.
-        setTimeout(MakeProgress, 0);
+        CallSetTimeoutMakeProgress();
     }
 
     function ExplicitPaintsCompleteListener() {
         LogInfo("ExplicitPaintsCompleteListener fired");
         // Since this can fire while painting, don't confuse ourselves by
         // firing synchronously. It's fine to do this asynchronously.
-        setTimeout(MakeProgress, 0);
+        CallSetTimeoutMakeProgress();
     }
 
     function RemoveListeners() {
@@ -587,8 +711,12 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
             return;
         }
 
+        LogInfo("MakeProgress");
+
         // We don't need to flush styles any more when we are in the state
         // after reftest-wait has removed.
+        OperationInProgress();
+        let promise = Promise.resolve(undefined);
         if (state != STATE_WAITING_TO_FINISH) {
           // If we are waiting for the MozReftestInvalidate event we don't want
           // to flush throttled animations. Flushing throttled animations can
@@ -600,13 +728,25 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
           flushMode = (state === STATE_WAITING_TO_FIRE_INVALIDATE_EVENT)
                     ? FlushMode.IGNORE_THROTTLED_ANIMATIONS
                     : FlushMode.ALL;
-          FlushRendering(flushMode);
+          promise = FlushRendering(flushMode);
         }
+        promise.then(function () {
+            OperationCompleted();
+            MakeProgress2();
+            // If there is an operation in progress then we know there will be
+            // a MakeProgress call is will happen after it finishes.
+            if (!operationInProgress) {
+                HandlePendingTasksAfterMakeProgress();
+            }
+        });
+    }
 
+    function MakeProgress2() {
         switch (state) {
         case STATE_WAITING_TO_FIRE_INVALIDATE_EVENT: {
             LogInfo("MakeProgress: STATE_WAITING_TO_FIRE_INVALIDATE_EVENT");
-            if (shouldWaitForExplicitPaintWaiters() || shouldWaitForPendingPaints()) {
+            if (shouldWaitForExplicitPaintWaiters() || shouldWaitForPendingPaints() ||
+                updateCanvasPending) {
                 gFailureReason = "timed out waiting for pending paint count to reach zero";
                 if (shouldWaitForExplicitPaintWaiters()) {
                     gFailureReason += " (waiting for MozPaintWaitFinished)";
@@ -616,6 +756,10 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
                     gFailureReason += " (waiting for MozAfterPaint)";
                     LogInfo("MakeProgress: waiting for MozAfterPaint");
                 }
+                if (updateCanvasPending) {
+                    gFailureReason += " (waiting for updateCanvasPending)";
+                    LogInfo("MakeProgress: waiting for updateCanvasPending");
+                }
                 return;
             }
 
@@ -649,10 +793,17 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
             if (hasReftestWait && !shouldWaitForReftestWaitRemoval(contentRootElement)) {
                 // MozReftestInvalidate handler removed reftest-wait.
                 // We expect something to have been invalidated...
-                FlushRendering(FlushMode.ALL);
-                if (!shouldWaitForPendingPaints() && !shouldWaitForExplicitPaintWaiters()) {
-                    LogWarning("MozInvalidateEvent didn't invalidate");
-                }
+                OperationInProgress();
+                let promise = FlushRendering(FlushMode.ALL);
+                promise.then(function () {
+                    OperationCompleted();
+                    if (!updateCanvasPending && !shouldWaitForPendingPaints() &&
+                        !shouldWaitForExplicitPaintWaiters()) {
+                        LogWarning("MozInvalidateEvent didn't invalidate");
+                    }
+                    MakeProgress();
+                });
+                return;
             }
             // Try next state
             MakeProgress();
@@ -689,7 +840,11 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
                 if (aTopic) LogInfo("MakeProgress: apz-repaints-flushed fired");
                 os.removeObserver(flushWaiter, "apz-repaints-flushed");
                 state = STATE_WAITING_TO_FINISH;
-                MakeProgress();
+                if (operationInProgress) {
+                    CallSetTimeoutMakeProgress();
+                } else {
+                    MakeProgress();
+                }
             };
             os.addObserver(flushWaiter, "apz-repaints-flushed");
 
@@ -713,7 +868,8 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
 
         case STATE_WAITING_TO_FINISH:
             LogInfo("MakeProgress: STATE_WAITING_TO_FINISH");
-            if (shouldWaitForExplicitPaintWaiters() || shouldWaitForPendingPaints()) {
+            if (shouldWaitForExplicitPaintWaiters() || shouldWaitForPendingPaints() ||
+                updateCanvasPending) {
                 gFailureReason = "timed out waiting for pending paint count to " +
                     "reach zero (after reftest-wait removed and switch to print mode)";
                 if (shouldWaitForExplicitPaintWaiters()) {
@@ -724,6 +880,10 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
                     gFailureReason += " (waiting for MozAfterPaint)";
                     LogInfo("MakeProgress: waiting for MozAfterPaint");
                 }
+                if (updateCanvasPending) {
+                    gFailureReason += " (waiting for updateCanvasPending)";
+                    LogInfo("MakeProgress: waiting for updateCanvasPending");
+                }
                 return;
             }
             if (contentRootElement) {
@@ -787,7 +947,11 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
     var numPendingSpellChecks = spellCheckedElements.length;
     function decNumPendingSpellChecks() {
         --numPendingSpellChecks;
-        MakeProgress();
+        if (operationInProgress) {
+            CallSetTimeoutMakeProgress();
+        } else {
+            MakeProgress();
+        }
     }
     for (let editable of spellCheckedElements) {
         try {
@@ -801,8 +965,12 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f
     // Take a full snapshot now that all our listeners are set up. This
     // ensures it's impossible for us to miss updates between taking the snapshot
     // and adding our listeners.
-    SendInitCanvasWithSnapshot(forURL);
-    MakeProgress();
+    OperationInProgress();
+    let promise = SendInitCanvasWithSnapshot(forURL);
+    promise.then(function () {
+        OperationCompleted();
+        MakeProgress();
+    });
 }
 
 function OnDocumentLoad(event)
@@ -855,19 +1023,19 @@ function OnDocumentLoad(event)
     setupDisplayport(contentRootElement);
     var inPrintMode = false;
 
-    function AfterOnLoadScripts() {
+    async function AfterOnLoadScripts() {
         // Regrab the root element, because the document may have changed.
         var contentRootElement =
           content.document ? content.document.documentElement : null;
 
         // Flush the document in case it got modified in a load event handler.
-        FlushRendering(FlushMode.ALL);
+        await FlushRendering(FlushMode.ALL);
 
         // Take a snapshot now. We need to do this before we check whether
         // we should wait, since this might trigger dispatching of
         // MozPaintWait events and make shouldWaitForExplicitPaintWaiters() true
         // below.
-        var painted = SendInitCanvasWithSnapshot(ourURL);
+        let painted = await SendInitCanvasWithSnapshot(ourURL);
 
         if (shouldWaitForExplicitPaintWaiters() ||
             (!inPrintMode && doPrintMode(contentRootElement)) ||
@@ -1138,10 +1306,11 @@ function IsSnapshottableTestType()
 
 const SYNC_DEFAULT = 0x0;
 const SYNC_ALLOW_DISABLE = 0x1;
+// Returns a promise that resolve when the snapshot is done.
 function SynchronizeForSnapshot(flags)
 {
     if (!IsSnapshottableTestType()) {
-        return;
+        return Promise.resolve(undefined);
     }
 
     if (flags & SYNC_ALLOW_DISABLE) {
@@ -1150,7 +1319,7 @@ function SynchronizeForSnapshot(flags)
             (docElt.hasAttribute("reftest-no-sync-layers") ||
              docElt.classList.contains("reftest-no-flush"))) {
             LogInfo("Test file chose to skip SynchronizeForSnapshot");
-            return;
+            return Promise.resolve(undefined);
         }
     }
 
@@ -1294,7 +1463,7 @@ function SendFailedAssignedLayer(why)
     sendAsyncMessage("reftest:FailedAssignedLayer", { why: why });
 }
 
-// Return true if a snapshot was taken.
+// Returns a promise that resolves to a bool that indicates if a snapshot was taken.
 function SendInitCanvasWithSnapshot(forURL)
 {
     if (forURL != gCurrentURL) {
@@ -1302,7 +1471,7 @@ function SendInitCanvasWithSnapshot(forURL)
         // Lie and say we painted because it doesn't matter, this is a test we
         // are already done with that is clearing out. Then AfterOnLoadScripts
         // should finish quicker if that is who is calling us.
-        return true;
+        return Promise.resolve(true);
     }
 
     // If we're in the same process as the top-level XUL window, then
@@ -1312,7 +1481,13 @@ function SendInitCanvasWithSnapshot(forURL)
     // NB: this is a test-harness optimization only, it must not
     // affect the validity of the tests.
     if (gBrowserIsRemote) {
-        SynchronizeForSnapshot(SYNC_DEFAULT);
+        let promise = SynchronizeForSnapshot(SYNC_DEFAULT);
+        return promise.then(function () {
+            let ret = sendSyncMessage("reftest:InitCanvasWithSnapshot")[0];
+
+            gHaveCanvasSnapshot = ret.painted;
+            return ret.painted;
+        });
     }
 
     // For in-process browser, we have to make a synchronous request
@@ -1321,10 +1496,10 @@ function SendInitCanvasWithSnapshot(forURL)
     // before we check the paint-wait counter.  For out-of-process
     // browser though, it doesn't wrt correctness whether this request
     // is sync or async.
-    var ret = sendSyncMessage("reftest:InitCanvasWithSnapshot")[0];
+    let ret = sendSyncMessage("reftest:InitCanvasWithSnapshot")[0];
 
     gHaveCanvasSnapshot = ret.painted;
-    return ret.painted;
+    return Promise.resolve(ret.painted);
 }
 
 function SendScriptResults(runtimeMs, error, results)
@@ -1362,13 +1537,13 @@ function elementDescription(element)
         '>';
 }
 
-function SendUpdateCanvasForEvent(forURL, event, contentRootElement)
+function SendUpdateCanvasForEvent(forURL, rectList, contentRootElement)
 {
     if (forURL != gCurrentURL) {
         LogInfo("SendUpdateCanvasForEvent called for previous document");
         // This is a test we are already done with that is clearing out.
         // Don't do anything.
-        return;
+        return Promise.resolve(undefined);
     }
 
     var win = content;
@@ -1381,10 +1556,12 @@ function SendUpdateCanvasForEvent(forURL, event, contentRootElement)
       if (!gBrowserIsRemote) {
           sendSyncMessage("reftest:UpdateWholeCanvasForInvalidation");
       } else {
-          SynchronizeForSnapshot(SYNC_ALLOW_DISABLE);
-          sendAsyncMessage("reftest:UpdateWholeCanvasForInvalidation");
+          let promise = SynchronizeForSnapshot(SYNC_ALLOW_DISABLE);
+          return promise.then(function () {
+            sendAsyncMessage("reftest:UpdateWholeCanvasForInvalidation");
+          });
       }
-      return;
+      return Promise.resolve(undefined);
     }
 
     var message;
@@ -1396,7 +1573,6 @@ function SendUpdateCanvasForEvent(forURL, event, contentRootElement)
         LogInfo("Webrender enabled, sending update whole canvas for invalidation");
         message = "reftest:UpdateWholeCanvasForInvalidation";
     } else {
-        var rectList = event.clientRects;
         LogInfo("SendUpdateCanvasForEvent with " + rectList.length + " rects");
         for (var i = 0; i < rectList.length; ++i) {
             var r = rectList[i];
@@ -1418,10 +1594,15 @@ function SendUpdateCanvasForEvent(forURL, event, contentRootElement)
     if (!gBrowserIsRemote) {
         sendSyncMessage(message, { rects: rects });
     } else {
-        SynchronizeForSnapshot(SYNC_ALLOW_DISABLE);
-        sendAsyncMessage(message, { rects: rects });
+        let promise = SynchronizeForSnapshot(SYNC_ALLOW_DISABLE);
+        return promise.then(function () {
+            sendAsyncMessage(message, { rects: rects });
+        });
     }
+
+    return Promise.resolve(undefined);
 }
+
 if (content.document.readyState == "complete") {
   // load event has already fired for content, get started
   OnInitialLoad();