Bug 1593170. Check if contentRootElement is a dead wrapper before using it. r=mattwoodrow,dbaron
authorTimothy Nikkel <tnikkel@gmail.com>
Mon, 18 Nov 2019 11:27:00 +0000
changeset 502491 0dc7546c4a8a75dda043edc312305a6ceba0ad53
parent 502490 285e9108032ffb6cd62fe7d587ef97c1a86d8d41
child 502492 249e26bfefd68d3efe8c06eee1ffa1341a43c59e
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow, dbaron
bugs1593170
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1593170. Check if contentRootElement is a dead wrapper before using it. r=mattwoodrow,dbaron With the fission changes everything is more async, meaning some tests can run longer. Some crashtests navigate to a new location meaning the original root element we have a variable for is no longer around, and chrome isn't allowed to keep content nodes alive, so we are left holding a dead wrapper that throws if we try to access anything on it. So we check if contentRootElement has become a dead wrapper (and null it out) any time we try to access it and it could have become dead. Generally the existing code already handled a null contentRootElement. Differential Revision: https://phabricator.services.mozilla.com/D52829
layout/tools/reftest/reftest-content.js
--- a/layout/tools/reftest/reftest-content.js
+++ b/layout/tools/reftest/reftest-content.js
@@ -550,16 +550,22 @@ function WaitForTestEnd(contentRootEleme
     // 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
 
+    function CheckForLivenessOfContentRootElement() {
+        if (contentRootElement && Cu.isDeadWrapper(contentRootElement)) {
+            contentRootElement = null;
+        }
+    }
+
     var setTimeoutCallMakeProgressWhenComplete = false;
 
     var operationInProgress = false;
     function OperationInProgress() {
         if (operationInProgress != false) {
             LogWarning("Nesting atomic operations?");
         }
         operationInProgress = true;
@@ -626,16 +632,17 @@ function WaitForTestEnd(contentRootEleme
         }
 
         if (updateCanvasPending) {
             LogInfo("HandlePendingTasksAfterMakeProgress updating canvas");
             updateCanvasPending = false;
             let rects = updateCanvasRects;
             updateCanvasRects = [];
             OperationInProgress();
+            CheckForLivenessOfContentRootElement();
             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();
             });
@@ -704,16 +711,17 @@ function WaitForTestEnd(contentRootEleme
         // Since this can fire while painting, don't confuse ourselves by
         // firing synchronously. It's fine to do this asynchronously.
         CallSetTimeoutMakeProgress();
     }
 
     function RemoveListeners() {
         // OK, we can end the test now.
         removeEventListener("MozAfterPaint", AfterPaintListener, false);
+        CheckForLivenessOfContentRootElement();
         if (contentRootElement) {
             contentRootElement.removeEventListener("DOMAttrModified", AttrModifiedListener);
         }
         gExplicitPendingPaintsCompleteHook = null;
         gTimeoutHook = null;
         // Make sure we're in the COMPLETED state just in case
         // (this may be called via the test-timeout hook)
         state = STATE_COMPLETED;
@@ -776,16 +784,17 @@ function WaitForTestEnd(contentRootEleme
                 if (updateCanvasPending) {
                     gFailureReason += " (waiting for updateCanvasPending)";
                     LogInfo("MakeProgress: waiting for updateCanvasPending");
                 }
                 return;
             }
 
             state = STATE_WAITING_FOR_REFTEST_WAIT_REMOVAL;
+            CheckForLivenessOfContentRootElement();
             var hasReftestWait = shouldWaitForReftestWaitRemoval(contentRootElement);
             // Notify the test document that now is a good time to test some invalidation
             LogInfo("MakeProgress: dispatching MozReftestInvalidate");
             if (contentRootElement) {
                 var elements = getNoPaintElements(contentRootElement);
                 for (var i = 0; i < elements.length; ++i) {
                   windowUtils().checkAndClearPaintedState(elements[i]);
                 }
@@ -824,16 +833,17 @@ function WaitForTestEnd(contentRootEleme
             }
             // Try next state
             MakeProgress();
             return;
         }
 
         case STATE_WAITING_FOR_REFTEST_WAIT_REMOVAL:
             LogInfo("MakeProgress: STATE_WAITING_FOR_REFTEST_WAIT_REMOVAL");
+            CheckForLivenessOfContentRootElement();
             if (shouldWaitForReftestWaitRemoval(contentRootElement)) {
                 gFailureReason = "timed out waiting for reftest-wait to be removed";
                 LogInfo("MakeProgress: waiting for reftest-wait to be removed");
                 return;
             }
 
             // Try next state
             state = STATE_WAITING_FOR_SPELL_CHECKS;
@@ -861,16 +871,17 @@ function WaitForTestEnd(contentRootEleme
                     CallSetTimeoutMakeProgress();
                 } else {
                     MakeProgress();
                 }
             };
             os.addObserver(flushWaiter, "apz-repaints-flushed");
 
             var willSnapshot = IsSnapshottableTestType();
+            CheckForLivenessOfContentRootElement();
             var noFlush =
                 !(contentRootElement &&
                   contentRootElement.classList.contains("reftest-no-flush"));
             if (noFlush && willSnapshot && windowUtils().flushApzRepaints()) {
                 LogInfo("MakeProgress: done requesting APZ flush");
             } else {
                 LogInfo("MakeProgress: APZ flush not required");
                 flushWaiter(null, null, null);
@@ -898,16 +909,17 @@ function WaitForTestEnd(contentRootEleme
                     LogInfo("MakeProgress: waiting for MozAfterPaint");
                 }
                 if (updateCanvasPending) {
                     gFailureReason += " (waiting for updateCanvasPending)";
                     LogInfo("MakeProgress: waiting for updateCanvasPending");
                 }
                 return;
             }
+            CheckForLivenessOfContentRootElement();
             if (contentRootElement) {
               var elements = getNoPaintElements(contentRootElement);
               for (var i = 0; i < elements.length; ++i) {
                   if (windowUtils().checkAndClearPaintedState(elements[i])) {
                       SendFailedNoPaint();
                   }
               }
               // We only support retained display lists in the content process
@@ -939,26 +951,28 @@ function WaitForTestEnd(contentRootEleme
               gFailureReason = "timed out while waiting for sync compositor flush"
               windowUtils().syncFlushCompositor();
             }
 
             LogInfo("MakeProgress: Completed");
             state = STATE_COMPLETED;
             gFailureReason = "timed out while taking snapshot (bug in harness?)";
             RemoveListeners();
+            CheckForLivenessOfContentRootElement();
             CheckForProcessCrashExpectation(contentRootElement);
             setTimeout(RecordResult, 0, forURL);
             return;
         }
     }
 
     LogInfo("WaitForTestEnd: Adding listeners");
     addEventListener("MozAfterPaint", AfterPaintListener, false);
     // If contentRootElement is null then shouldWaitForReftestWaitRemoval will
     // always return false so we don't need a listener anyway
+    CheckForLivenessOfContentRootElement();
     if (contentRootElement) {
       contentRootElement.addEventListener("DOMAttrModified", AttrModifiedListener);
     }
     gExplicitPendingPaintsCompleteHook = ExplicitPaintsCompleteListener;
     gTimeoutHook = RemoveListeners;
 
     // Listen for spell checks on spell-checked elements.
     var numPendingSpellChecks = spellCheckedElements.length;
@@ -1069,16 +1083,20 @@ function OnDocumentLoad(event)
         // 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.
         let painted = await SendInitCanvasWithSnapshot(ourURL);
 
         gExplicitPendingPaintsCompleteHook = null;
 
+        if (contentRootElement && Cu.isDeadWrapper(contentRootElement)) {
+            contentRootElement = null;
+        }
+
         if (paintWaiterFinished || shouldWaitForExplicitPaintWaiters() ||
             (!inPrintMode && doPrintMode(contentRootElement)) ||
             // If we didn't force a paint above, in
             // InitCurrentCanvasWithSnapshot, so we should wait for a
             // paint before we consider them done.
             !painted) {
             LogInfo("AfterOnLoadScripts belatedly entering WaitForTestEnd");
             // Go into reftest-wait mode belatedly.