Bug 1415217 - Try to replace reftest error 'load failed with unknown reason' with informative messages. r=dholbert
authorJonathan Watt <jwatt@jwatt.org>
Thu, 19 Oct 2017 16:59:32 +0100
changeset 443990 27a656fe5be8be38ffe629e088a9496c7b1c1501
parent 443989 80798ac3e0ded9e9f9ec90db65da9529b30093e5
child 443991 2f3606eba4bc442780ea32f4227b4633b74280ae
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1415217
milestone58.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 1415217 - Try to replace reftest error 'load failed with unknown reason' with informative messages. r=dholbert If the reftest harness times out the load of a URL before the 'load' event has even fired then the error message that we get is 'load failed with unknown reason'. This isn't very helpful for people unfamiliar with the harness. This change sets an initial error message that notes that we're waiting on the 'load' event, what the timeout delay is, and what URL we are/were waiting on loading. This will allow the timeout delay to be compared to log timestamps and will make it clear whether the timeout occurred for some other URL than the one we were expecting to load (which would be an error in the harness logic). It should now be impossible for the 'load failed with unknown reason' to occur, but if there is a logic error in the harness code (some race condition?) then it may still happen. MozReview-Commit-ID: JOb1kYBpLro
layout/tools/reftest/reftest-content.js
layout/tools/reftest/reftest.jsm
--- a/layout/tools/reftest/reftest-content.js
+++ b/layout/tools/reftest/reftest-content.js
@@ -119,31 +119,36 @@ function OnInitialLoad()
     addEventListener("load", OnDocumentLoad, true);
 
     addEventListener("MozPaintWait", PaintWaitListener, true);
     addEventListener("MozPaintWaitFinished", PaintWaitFinishedListener, true);
 
     LogInfo("Using browser remote="+ gBrowserIsRemote +"\n");
 }
 
-function SetFailureTimeout(cb, timeout)
+function SetFailureTimeout(cb, timeout, uri)
 {
   var targetTime = Date.now() + timeout;
 
   var wrapper = function() {
     // Timeouts can fire prematurely in some cases (e.g. in chaos mode). If this
     // happens, set another timeout for the remaining time.
     let remainingMs = targetTime - Date.now();
     if (remainingMs > 0) {
       SetFailureTimeout(cb, remainingMs);
     } else {
       cb();
     }
   }
 
+  // Once OnDocumentLoad is called to handle the 'load' event it will update
+  // this error message to reflect what stage of the processing it has reached
+  // as it advances to each stage in turn.
+  gFailureReason = "timed out after " + timeout +
+                   " ms waiting for 'load' event for " + uri;
   gFailureTimeout = setTimeout(wrapper, timeout);
 }
 
 function StartTestURI(type, uri, timeout)
 {
     // The GC is only able to clean up compartments after the CC runs. Since
     // the JS ref tests disable the normal browser chrome and do not otherwise
     // create substatial DOM garbage, the CC tends not to run enough normally.
@@ -159,17 +164,17 @@ function StartTestURI(type, uri, timeout
 
     gCurrentTestType = type;
     gCurrentURL = uri;
 
     gCurrentTestStartTime = Date.now();
     if (gFailureTimeout != null) {
         SendException("program error managing timeouts\n");
     }
-    SetFailureTimeout(LoadFailed, timeout);
+    SetFailureTimeout(LoadFailed, timeout, uri);
 
     LoadURI(gCurrentURL);
 }
 
 function setupFullZoom(contentRootElement) {
     if (!contentRootElement || !contentRootElement.hasAttribute('reftest-zoom'))
         return;
     markupDocumentViewer().fullZoom =
--- a/layout/tools/reftest/reftest.jsm
+++ b/layout/tools/reftest/reftest.jsm
@@ -1167,20 +1167,24 @@ function RecordResult(testRunTime, error
         default:
             throw "Unexpected state.";
     }
 }
 
 function LoadFailed(why)
 {
     ++g.testResults.FailedLoad;
-    // Once bug 896840 is fixed, this can go away, but for now it will give log
-    // output that is TBPL starable for bug 789751 and bug 720452.
     if (!why) {
-        logger.error("load failed with unknown reason");
+        // reftest-content.js sets an initial reason before it sets the
+        // timeout that will call us with the currently set reason, so we
+        // should never get here.  If we do then there's a logic error
+        // somewhere.  Perhaps tests are somehow running overlapped and the
+        // timeout for one test is not being cleared before the timeout for
+        // another is set?  Maybe there's some sort of race?
+        logger.error("load failed with unknown reason (we should always have a reason!)");
     }
     logger.testStatus(g.urls[0].identifier, "load failed: " + why, "FAIL", "PASS");
     FlushTestBuffer();
     FinishTestItem();
 }
 
 function RemoveExpectedCrashDumpFiles()
 {