Bug 839445 - PopupNotifications need to know about location changes in background tabs. r=gavin

--HG--
extra : rebase_source : 3e12347e0e9eaa257343c941e37e3c417be94542

browser/base/content/browser.js

@@ -4341,18 +4341,6 @@ var XULBrowserWindow = {
         SocialShareButton.updateShareState();
       }
 
-      // Filter out anchor navigation, history.push/pop/replaceState and
-      // tab switches.
-      if (aRequest) {
-        // Only need to call locationChange if the PopupNotifications object
-        // for this window has already been initialized (i.e. its getter no
-        // longer exists)
-        // XXX bug 839445: We never tell PopupNotifications about location
-        // changes in background tabs.
-        if (!__lookupGetter__("PopupNotifications"))
-          PopupNotifications.locationChange();
-      }
-
       // Show or hide browser chrome based on the whitelist
       if (this.hideChromeForLocation(location)) {
         document.documentElement.setAttribute("disablechrome", "true");
@@ -4776,6 +4764,12 @@ var TabsProgressListener = {
         aBrowser._clickToPlayPluginsActivated = new Map();
         aBrowser._clickToPlayAllPluginsActivated = false;
         aBrowser._pluginScriptedState = gPluginHandler.PLUGIN_SCRIPTED_STATE_NONE;
+
+        // Only need to call locationChange if the PopupNotifications object
+        // for this window has already been initialized (i.e. its getter no
+        // longer exists)
+        if (!Object.getOwnPropertyDescriptor(window, "PopupNotifications").get)
+          PopupNotifications.locationChange(aBrowser);
       }
       FullZoom.onLocationChange(aLocationURI, false, aBrowser);
     }

browser/base/content/test/browser_popupNotification.js

@@ -27,16 +27,16 @@ var gActiveListeners = {};
 var gActiveObservers = {};
 var gShownState = {};
 
+function goNext() {
+  if (++gTestIndex == tests.length)
+    executeSoon(finish);
+  else
+    executeSoon(runNextTest);
+}
+
 function runNextTest() {
   let nextTest = tests[gTestIndex];
 
-  function goNext() {
-    if (++gTestIndex == tests.length)
-      executeSoon(finish);
-    else
-      executeSoon(runNextTest);
-  }
-
   function addObserver(topic) {
     function observer() {
       Services.obs.removeObserver(observer, "PopupNotifications-" + topic);
@@ -54,7 +54,7 @@ function runNextTest() {
     addObserver("backgroundShow");
   } else if (nextTest.updateNotShowing) {
     addObserver("updateNotShowing");
-  } else {
+  } else if (nextTest.onShown) {
     doOnPopupEvent("popupshowing", function () {
       info("[Test #" + gTestIndex + "] popup showing");
     });
@@ -724,6 +724,34 @@ var tests = [
       ok(!this.notifyObj.dismissalCallbackTriggered, "dismissal callback was not triggered");
       PopupNotifications.buttonDelay = PREF_SECURITY_DELAY_INITIAL;
     },
+  },
+  { // Test #25 - location change in background tab removes notification
+    run: function () {
+      this.oldSelectedTab = gBrowser.selectedTab;
+      this.newTab = gBrowser.addTab("about:blank");
+      gBrowser.selectedTab = this.newTab;
+
+      loadURI("http://example.com/", function() {
+        gBrowser.selectedTab = this.oldSelectedTab;
+        let browser = gBrowser.getBrowserForTab(this.newTab);
+
+        this.notifyObj = new basicNotification();
+        this.notifyObj.browser = browser;
+        this.notifyObj.options.eventCallback = function (eventName) {
+          if (eventName == "removed") {
+            ok(true, "Notification removed in background tab after reloading");
+            executeSoon(function () {
+              gBrowser.removeTab(this.newTab);
+              goNext();
+            }.bind(this));
+          }
+        }.bind(this);
+        this.notification = showNotification(this.notifyObj);
+        executeSoon(function () {
+          browser.reload();
+        });
+      }.bind(this));
+    }
   }
 ];
 

toolkit/content/PopupNotifications.jsm

@@ -277,11 +277,16 @@ PopupNotifications.prototype = {
   },
 
   /**
-   * Called by the consumer to indicate that the current browser's location has
-   * changed, so that we can update the active notifications accordingly.
+   * Called by the consumer to indicate that a browser's location has changed,
+   * so that we can update the active notifications accordingly.
    */
-  locationChange: function PopupNotifications_locationChange() {
-    this._currentNotifications = this._currentNotifications.filter(function(notification) {
+  locationChange: function PopupNotifications_locationChange(aBrowser) {
+    if (!aBrowser)
+      throw "PopupNotifications_locationChange: invalid browser";
+
+    let notifications = this._getNotificationsForBrowser(aBrowser);
+
+    notifications = notifications.filter(function (notification) {
       // The persistWhileVisible option allows an open notification to persist
       // across location changes
       if (notification.options.persistWhileVisible &&
@@ -310,7 +315,10 @@ PopupNotifications.prototype = {
       return false;
     }, this);
 
-    this._update();
+    this._setNotificationsForBrowser(aBrowser, notifications);
+
+    if (aBrowser == this.tabbrowser.selectedBrowser)
+      this._update();
   },
 
   /**
@@ -356,14 +364,11 @@ PopupNotifications.prototype = {
   _currentAnchorElement: null,
 
   /**
-   * Gets and sets notifications for the currently selected browser.
+   * Gets notifications for the currently selected browser.
    */
   get _currentNotifications() {
     return this._getNotificationsForBrowser(this.tabbrowser.selectedBrowser);
   },
-  set _currentNotifications(a) {
-    return this._setNotificationsForBrowser(this.tabbrowser.selectedBrowser, a);
-  },
 
   _remove: function PopupNotifications_removeHelper(notification) {
     // This notification may already be removed, in which case let's just fail