Bug 1315969 - Only open captive portal tab if we also immediately focus it. r=MattN a=jcristau
authorNihanth Subramanya <nhnt11@gmail.com>
Tue, 08 Nov 2016 20:14:34 +0530
changeset 352846 bf7877a40fe66b43c031b19aff4920418c1dbd01
parent 352845 2ac543baa4920166fc214dd70df9058486e7cdf7
child 352847 392d293b5169b5cda8b84d8a6955702664a969d0
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN, jcristau
bugs1315969
milestone52.0a2
Bug 1315969 - Only open captive portal tab if we also immediately focus it. r=MattN a=jcristau MozReview-Commit-ID: 4nKE1WkzV2n
browser/modules/CaptivePortalWatcher.jsm
--- a/browser/modules/CaptivePortalWatcher.jsm
+++ b/browser/modules/CaptivePortalWatcher.jsm
@@ -98,34 +98,32 @@ this.CaptivePortalWatcher = {
     // 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 != Services.ww.activeWindow) {
       this._waitingToAddTab = true;
       Services.obs.addObserver(this, "xul-window-visible", false);
       return;
     }
 
-    // The browser is in use - show a notification and add the tab without
-    // selecting it, unless the caller specifically requested selection.
-    this._ensureCaptivePortalTab(win);
     this._showNotification(win);
   },
 
   _ensureCaptivePortalTab(win) {
     let tab;
     if (this._captivePortalTab) {
       tab = this._captivePortalTab.get();
     }
 
     // If the tab is gone or going, we need to open a new one.
     if (!tab || tab.closing || !tab.parentNode) {
       tab = win.gBrowser.addTab(this.canonicalURL);
     }
 
     this._captivePortalTab = Cu.getWeakReference(tab);
+    win.gBrowser.selectedTab = tab;
     return tab;
   },
 
   /**
    * Called after we regain focus if we detect a portal while a browser window
    * doesn't have focus. Triggers a portal recheck to reaffirm state, and adds
    * the tab if needed after a short delay to allow the recheck to complete.
    */
@@ -138,40 +136,36 @@ this.CaptivePortalWatcher = {
     if (win != Services.ww.activeWindow) {
       // 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();
+    let requestTime = Date.now();
 
-    // We wait for PORTAL_RECHECK_DELAY_MS after the trigger.
-    // - If the portal is no longer locked, we don't need to add a tab.
-    // - If it is, the delay is chosen to not be extremely noticeable.
-    setTimeout(() => {
-      this._waitingToAddTab = false;
+    let self = this;
+    Services.obs.addObserver(function observer() {
+      Services.obs.removeObserver(observer, "captive-portal-check-complete");
+      self._waitingToAddTab = false;
       if (cps.state != cps.LOCKED_PORTAL) {
         // We're free of the portal!
         return;
       }
 
-      this._showNotification(win);
-      let tab = this._ensureCaptivePortalTab(win);
-
-      // Focus the tab only if the recheck has completed, i.e. we're sure
-      // that the portal is still locked. This way, if the recheck completes
-      // after we add the tab and we're free of the portal, the tab contents
-      // won't flicker.
-      if (cps.lastChecked != lastChecked) {
-        win.gBrowser.selectedTab = tab;
+      self._showNotification(win);
+      if (Date.now() - requestTime <= PORTAL_RECHECK_DELAY_MS) {
+        // The amount of time elapsed since we requested a recheck (i.e. since
+        // the browser window was focused) was small enough that we can add and
+        // focus a tab with the login page with no noticeable delay.
+        self._ensureCaptivePortalTab(win);
       }
-    }, PORTAL_RECHECK_DELAY_MS);
+    }, "captive-portal-check-complete", false);
   },
 
   _captivePortalGone() {
     if (this._waitingToAddTab) {
       Services.obs.removeObserver(this, "xul-window-visible");
       this._waitingToAddTab = false;
     }
 
@@ -200,23 +194,16 @@ this.CaptivePortalWatcher = {
         tabbrowser.selectedTab == tab) {
       return;
     }
 
     // Remove the tab.
     tabbrowser.removeTab(tab);
   },
 
-  get _productName() {
-    delete this._productName;
-    return this._productName =
-      Services.strings.createBundle("chrome://branding/locale/brand.properties")
-                      .GetStringFromName("brandShortName");
-  },
-
   get _browserBundle() {
     delete this._browserBundle;
     return this._browserBundle =
       Services.strings.createBundle("chrome://browser/locale/browser.properties");
   },
 
   handleEvent(aEvent) {
     if (aEvent.type != "TabSelect" || !this._captivePortalTab || !this._captivePortalNotification) {
@@ -238,27 +225,26 @@ this.CaptivePortalWatcher = {
     }
   },
 
   _showNotification(win) {
     let buttons = [
       {
         label: this._browserBundle.GetStringFromName("captivePortal.showLoginPage"),
         callback: () => {
-          win.gBrowser.selectedTab = this._ensureCaptivePortalTab(win);
+          this._ensureCaptivePortalTab(win);
 
           // Returning true prevents the notification from closing.
           return true;
         },
         isDefault: true,
       },
     ];
 
-    let message = this._browserBundle.formatStringFromName("captivePortal.infoMessage",
-                                                           [this._productName], 1);
+    let message = this._browserBundle.GetStringFromName("captivePortal.infoMessage2");
 
     let closeHandler = (aEventName) => {
       if (aEventName != "removed") {
         return;
       }
       win.gBrowser.tabContainer.removeEventListener("TabSelect", this);
     };