Bug 1510569 - Remove onStatusChange handlers from browser_alltabslistener.js r=Gijs
authorBarret Rennie <barret@brennie.ca>
Wed, 03 Apr 2019 17:32:14 +0000
changeset 467880 d5106af44dabac2c09dc09654734fe098c5bf0a2
parent 467879 791382b2c9bf60371b68c0dd4d2ed26e4b846901
child 467881 1028814583232487b52b9c20d47e3b38dc1c288a
push id35810
push useraciure@mozilla.com
push dateThu, 04 Apr 2019 04:33:36 +0000
treeherdermozilla-central@b72c02e34261 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1510569
milestone68.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 1510569 - Remove onStatusChange handlers from browser_alltabslistener.js r=Gijs The `browser_talltabslistener.js` test verifies that (a) certain `nsIWebProgress` events occur in the correct order and (b) that the events come from the correct browser. However, the part of the test that listend for `onStatusChange` messages did not actually confirm that they were triggered or that the correct number of them were triggered. The ordering of the `onStatusChange` events with respect to the other `nsIWebProgress` events is non-deterministic because `onStatusChange` events are filtered through the `nsBrowserStatusFilter`, which may or may not put them on a delay timer (due to the message being repeated often). The presence of this timer makes for a lovely race condition. Now that the TabChild's onStatusChange event handler is registered earlier (in `TabChild::Init` instead of the `WebProgressChild.jsm`) we are getting more `onStatusChange` messages and the race more frequently results in a failing test. This is due to one of the test cases swapping browsers *but* not explicitly waiting for all `onStatusChange` events to come in, so the test case will run with swapped browsers and a `onStatusChange` event from a previous test case will leak through, causing the test to fail. The simplest way to fix these tests is to just remove the `onStatusChange` listeners, since they do not assert whether or not they are received -- the tests will pass even if no `onStatusChange` events are sent. Differential Revision: https://phabricator.services.mozilla.com/D25447
browser/base/content/test/general/browser_alltabslistener.js
--- a/browser/base/content/test/general/browser_alltabslistener.js
+++ b/browser/base/content/test/general/browser_alltabslistener.js
@@ -28,19 +28,16 @@ var gFrontProgressListener = {
     }
     var state = "onLocationChange";
     info("FrontProgress: " + state + " " + aLocationURI.spec);
     ok(gFrontNotificationsPos < gFrontNotifications.length, "Got an expected notification for the front notifications listener");
     is(state, gFrontNotifications[gFrontNotificationsPos], "Got a notification for the front notifications listener");
     gFrontNotificationsPos++;
   },
 
-  onStatusChange(aWebProgress, aRequest, aStatus, aMessage) {
-  },
-
   onSecurityChange(aWebProgress, aRequest, aState) {
     if (aRequest &&
         aRequest.QueryInterface(Ci.nsIChannel).originalURI.spec == "about:blank") {
       // ignore initial about blank
       return;
     }
     var state = "onSecurityChange";
     info("FrontProgress: " + state + " 0x" + aState.toString(16));
@@ -80,26 +77,16 @@ var gAllProgressListener = {
     var state = "onLocationChange";
     info("AllProgress: " + state + " " + aLocationURI.spec);
     ok(aBrowser == gTestBrowser, state + " notification came from the correct browser");
     ok(gAllNotificationsPos < gAllNotifications.length, "Got an expected notification for the all notifications listener");
     is(state, gAllNotifications[gAllNotificationsPos], "Got a notification for the all notifications listener");
     gAllNotificationsPos++;
   },
 
-  onStatusChange(aBrowser, aWebProgress, aRequest, aStatus, aMessage) {
-    if (aRequest &&
-        aRequest.QueryInterface(Ci.nsIChannel).originalURI.spec == "about:blank") {
-      // ignore initial about blank
-      return;
-    }
-    var state = "onStatusChange";
-    ok(aBrowser == gTestBrowser, state + " notification came from the correct browser");
-  },
-
   onSecurityChange(aBrowser, aWebProgress, aRequest, aState) {
     if (aRequest &&
         aRequest.QueryInterface(Ci.nsIChannel).originalURI.spec == "about:blank") {
       // ignore initial about blank
       return;
     }
     var state = "onSecurityChange";
     info("AllProgress: " + state + " 0x" + aState.toString(16));