Bug 1279491 - CaptivePortalWatcher: use Services.ww.activeWindow to check window focus instead of document.hasFocus(). r=MattN
☠☠ backed out by f10815c69250 ☠ ☠
authorNihanth Subramanya <nhnt11@gmail.com>
Thu, 30 Jun 2016 23:25:13 +0000
changeset 306848 8136281a8b86aa22509eba7d5adaa508c2bd1a02
parent 306847 739a8707a3e290ea0f995ffcb6709d30f9027028
child 306849 b0ff95a9eb9d7dcb56e89f94cc83fabaa89b5b47
push id20132
push userpastithas@mozilla.com
push dateWed, 27 Jul 2016 16:14:24 +0000
treeherderfx-team@b0ff95a9eb9d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1279491
milestone50.0a1
Bug 1279491 - CaptivePortalWatcher: use Services.ww.activeWindow to check window focus instead of document.hasFocus(). r=MattN MozReview-Commit-ID: ERzJP8WhGsM
browser/modules/CaptivePortalWatcher.jsm
browser/modules/test/browser_CaptivePortalWatcher.js
--- a/browser/modules/CaptivePortalWatcher.jsm
+++ b/browser/modules/CaptivePortalWatcher.jsm
@@ -82,17 +82,17 @@ this.CaptivePortalWatcher = {
       return;
     }
 
     let win = RecentWindow.getMostRecentBrowserWindow();
     // If there's no browser window or none have focus, open and show the
     // tab when we regain focus. This is so that if a different application was
     // focused, when the user (re-)focuses a browser window, we open the tab
     // immediately in that window so they can login before continuing to browse.
-    if (!win || !win.document.hasFocus()) {
+    if (!win || Services.ww.activeWindow != win) {
       this._waitingToAddTab = true;
       Services.obs.addObserver(this, "xul-window-visible", false);
       return;
     }
 
     // The browser is in use - add the tab without selecting it.
     let tab = win.gBrowser.addTab(this.canonicalURL);
     this._captivePortalTab = Cu.getWeakReference(tab);
@@ -105,18 +105,18 @@ this.CaptivePortalWatcher = {
    * the tab if needed after a short delay to allow the recheck to complete.
    */
   _delayedAddCaptivePortalTab() {
     if (!this._waitingToAddTab) {
       return;
     }
 
     let win = RecentWindow.getMostRecentBrowserWindow();
-    if (!win.document.hasFocus()) {
-      // The document that got focused was not in a browser window.
+    if (!win || Services.ww.activeWindow != win) {
+      // The window that got focused was not a browser window.
       return;
     }
     Services.obs.removeObserver(this, "xul-window-visible");
 
     // Trigger a portal recheck. The user may have logged into the portal via
     // another client, or changed networks.
     let lastChecked = cps.lastChecked;
     cps.recheckCaptivePortal();
--- a/browser/modules/test/browser_CaptivePortalWatcher.js
+++ b/browser/modules/test/browser_CaptivePortalWatcher.js
@@ -33,32 +33,57 @@ function* openWindowAndWaitForPortalTab(
   return win;
 }
 
 function freePortal(aSuccess) {
   Services.obs.notifyObservers(null,
     "captive-portal-login-" + (aSuccess ? "success" : "abort"), null);
 }
 
+/**
+ * Some tests open a new window and close it later. When the window is closed,
+ * the original window opened by mochitest gains focus, generating a
+ * xul-window-visible notification. If the next test also opens a new window
+ * before this notification has a chance to fire, CaptivePortalWatcher picks
+ * up the first one instead of the one from the new window. To avoid this
+ * unfortunate intermittent timing issue, we wait for the notification from
+ * the original window every time we close a window that we opened.
+ */
+
+function waitForXulWindowVisible() {
+  return new Promise(resolve => {
+    Services.obs.addObserver(function observe() {
+      Services.obs.removeObserver(observe, "xul-window-visible");
+      resolve();
+    }, "xul-window-visible", false);
+  });
+}
+
+function* closeWindowAndWaitForXulWindowVisible(win) {
+  let p = waitForXulWindowVisible();
+  yield BrowserTestUtils.closeWindow(win);
+  yield p;
+}
+
 // Each of the test cases below is run twice: once for login-success and once
 // for login-abort (aSuccess set to true and false respectively).
 let testCasesForBothSuccessAndAbort = [
   /**
    * A portal is detected when there's no browser window,
    * then a browser window is opened, then the portal is freed.
    * The portal tab should be added and focused when the window is
    * opened, and closed automatically when the success event is fired.
    */
   function* test_detectedWithNoBrowserWindow_Open(aSuccess) {
     yield portalDetectedNoBrowserWindow();
     let win = yield openWindowAndWaitForPortalTab();
     freePortal(aSuccess);
     is(win.gBrowser.tabs.length, 1,
       "The captive portal tab should have been closed.");
-    yield BrowserTestUtils.closeWindow(win);
+    yield closeWindowAndWaitForXulWindowVisible(win);
   },
 
   /**
    * A portal is detected when there's no browser window, and the
    * portal is freed before a browser window is opened. No portal
    * tab should be added when a browser window is opened.
    */
   function* test_detectedWithNoBrowserWindow_GoneBeforeOpen(aSuccess) {
@@ -66,17 +91,17 @@ let testCasesForBothSuccessAndAbort = [
     freePortal(aSuccess);
     let win = yield BrowserTestUtils.openNewBrowserWindow();
     // Wait for a while to make sure no tab is opened.
     yield new Promise(resolve => {
       setTimeout(resolve, 1000);
     });
     is(win.gBrowser.tabs.length, 1,
       "No captive portal tab should have been opened.");
-    yield BrowserTestUtils.closeWindow(win);
+    yield closeWindowAndWaitForXulWindowVisible(win);
   },
 
   /**
    * A portal is detected when a browser window has focus. A portal tab should be
    * opened in the background in the focused browser window. If the portal is
    * freed when the tab isn't focused, the tab should be closed automatically.
    */
   function* test_detectedWithFocus(aSuccess) {
@@ -124,17 +149,17 @@ let singleRunTestCases = [
     let browser = win.gBrowser.selectedTab.linkedBrowser;
     let loadPromise =
       BrowserTestUtils.browserLoaded(browser, false, CANONICAL_URL_REDIRECTED);
     BrowserTestUtils.loadURI(browser, CANONICAL_URL_REDIRECTED);
     yield loadPromise;
     freePortal(true);
     is(win.gBrowser.tabs.length, 2,
       "The captive portal tab should not have been closed.");
-    yield BrowserTestUtils.closeWindow(win);
+    yield closeWindowAndWaitForXulWindowVisible(win);
   },
 
   /**
    * A portal is detected when a browser window has focus. A portal tab should be
    * opened in the background in the focused browser window. If the portal is
    * freed when the tab isn't focused, the tab should be closed automatically,
    * even if the portal has redirected to a URL other than CANONICAL_URL.
    */