From c0fa0a7a0e52136f14e94e3686214b982f2cad79 Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Mon, 18 Nov 2019 00:49:07 +0000 Subject: [PATCH] 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 --HG-- extra : moz-landing-system : lando --- layout/tools/reftest/reftest-content.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/layout/tools/reftest/reftest-content.js b/layout/tools/reftest/reftest-content.js index 44e345f3fb92..f2a4bd6c49c1 100644 --- a/layout/tools/reftest/reftest-content.js +++ b/layout/tools/reftest/reftest-content.js @@ -555,6 +555,12 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f // - MakeProgress // etc + function CheckForLivenessOfContentRootElement() { + if (contentRootElement && Cu.isDeadWrapper(contentRootElement)) { + contentRootElement = null; + } + } + var setTimeoutCallMakeProgressWhenComplete = false; var operationInProgress = false; @@ -631,6 +637,7 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f let rects = updateCanvasRects; updateCanvasRects = []; OperationInProgress(); + CheckForLivenessOfContentRootElement let promise = SendUpdateCanvasForEvent(rects, contentRootElement); promise.then(function () { OperationCompleted(); @@ -709,6 +716,7 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f function RemoveListeners() { // OK, we can end the test now. removeEventListener("MozAfterPaint", AfterPaintListener, false); + CheckForLivenessOfContentRootElement(); if (contentRootElement) { contentRootElement.removeEventListener("DOMAttrModified", AttrModifiedListener); } @@ -781,6 +789,7 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f } 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"); @@ -829,6 +838,7 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f 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"); @@ -866,6 +876,7 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f os.addObserver(flushWaiter, "apz-repaints-flushed"); var willSnapshot = IsSnapshottableTestType(); + CheckForLivenessOfContentRootElement(); var noFlush = !(contentRootElement && contentRootElement.classList.contains("reftest-no-flush")); @@ -903,6 +914,7 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f } return; } + CheckForLivenessOfContentRootElement(); if (contentRootElement) { var elements = getNoPaintElements(contentRootElement); for (var i = 0; i < elements.length; ++i) { @@ -944,6 +956,7 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f state = STATE_COMPLETED; gFailureReason = "timed out while taking snapshot (bug in harness?)"; RemoveListeners(); + CheckForLivenessOfContentRootElement(); CheckForProcessCrashExpectation(contentRootElement); setTimeout(RecordResult, 0, forURL); return; @@ -954,6 +967,7 @@ function WaitForTestEnd(contentRootElement, inPrintMode, spellCheckedElements, f 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); } @@ -1074,6 +1088,10 @@ function OnDocumentLoad(event) gExplicitPendingPaintsCompleteHook = null; + if (contentRootElement && Cu.isDeadWrapper(contentRootElement)) { + contentRootElement = null; + } + if (paintWaiterFinished || shouldWaitForExplicitPaintWaiters() || (!inPrintMode && doPrintMode(contentRootElement)) || // If we didn't force a paint above, in