Bug 528776 - discard stale windows before messing with the browser state. r=zeniko

--HG--
extra : rebase_source : 29e4fdfe833a20a975c71efd7a112001636a3f1a

browser/components/sessionstore/src/nsSessionStore.js

@@ -186,9 +186,6 @@ SessionStoreService.prototype = {
   // whether we clearing history on shutdown
   _clearingOnShutdown: false,
 
-  // List of windows that are being closed during setBrowserState.
-  _closingWindows: [],
-
 #ifndef XP_MACOSX
   // whether the last window was closed and should be restored
   _restoreLastWindow: false,
@@ -340,17 +337,9 @@ SessionStoreService.prototype = {
       aSubject.addEventListener("load", function(aEvent) {
         aEvent.currentTarget.removeEventListener("load", arguments.callee, false);
         _this.onLoad(aEvent.currentTarget);
-        }, false);
+      }, false);
       break;
     case "domwindowclosed": // catch closed windows
-      if (this._closingWindows.length > 0) {
-        let index = this._closingWindows.indexOf(aSubject);
-        if (index != -1) {
-          this._closingWindows.splice(index, 1);
-          if (this._closingWindows.length == 0)
-            this._sendRestoreCompletedNotifications(true);
-        }
-      }
       this.onClose(aSubject);
       break;
     case "quit-application-requested":
@@ -901,6 +890,8 @@ SessionStoreService.prototype = {
   },
 
   setBrowserState: function sss_setBrowserState(aState) {
+    this._handleClosedWindows();
+
     try {
       var state = this._safeEval("(" + aState + ")");
     }
@@ -917,21 +908,20 @@ SessionStoreService.prototype = {
       return;
     }
 
+    // close all other browser windows
+    this._forEachBrowserWindow(function(aWindow) {
+      if (aWindow != window) {
+        aWindow.close();
+        this.onClose(aWindow);
+      }
+    });
+
     // make sure closed window data isn't kept
     this._closedWindows = [];
 
     // determine how many windows are meant to be restored
     this._restoreCount = state.windows ? state.windows.length : 0;
 
-    var self = this;
-    // close all other browser windows
-    this._forEachBrowserWindow(function(aWindow) {
-      if (aWindow != window) {
-        self._closingWindows.push(aWindow);
-        aWindow.close();
-      }
-    });
-
     // restore to the given state
     this.restoreWindow(window, state, true);
   },
@@ -1722,6 +1712,8 @@ SessionStoreService.prototype = {
    * @returns string
    */
   _getCurrentState: function sss_getCurrentState(aUpdateAll) {
+    this._handleClosedWindows();
+
     var activeWindow = this._getMostRecentBrowserWindow();
     
     if (this._loadState == STATE_RUNNING) {
@@ -1735,7 +1727,7 @@ SessionStoreService.prototype = {
         else { // always update the window features (whose change alone never triggers a save operation)
           this._updateWindowFeatures(aWindow);
         }
-      }, this);
+      });
       this._dirtyWindows = [];
     }
     
@@ -2670,6 +2662,24 @@ SessionStoreService.prototype = {
 #endif
   },
 
+  /**
+   * Calls onClose for windows that are determined to be closed but aren't
+   * destroyed yet, which would otherwise cause getBrowserState and
+   * setBrowserState to treat them as open windows.
+   */
+  _handleClosedWindows: function sss_handleClosedWindows() {
+    var windowMediator = Cc["@mozilla.org/appshell/window-mediator;1"].
+                         getService(Ci.nsIWindowMediator);
+    var windowsEnum = windowMediator.getEnumerator("navigator:browser");
+
+    while (windowsEnum.hasMoreElements()) {
+      var window = windowsEnum.getNext();
+      if (window.closed) {
+        this.onClose(window);
+      }
+    }
+  },
+
   /**
    * open a new browser window for a given session state
    * called when restoring a multi-window session
@@ -2881,17 +2891,16 @@ SessionStoreService.prototype = {
     return jsonString;
   },
 
-  _sendRestoreCompletedNotifications:
-  function sss_sendRestoreCompletedNotifications(aOnWindowClose) {
-    if (this._restoreCount && !aOnWindowClose)
+  _sendRestoreCompletedNotifications: function sss_sendRestoreCompletedNotifications() {
+    if (this._restoreCount) {
       this._restoreCount--;
-
-    if (this._restoreCount == 0 && this._closingWindows.length == 0) {
-      // This was the last window restored at startup, notify observers.
-      this._observerService.notifyObservers(null,
-        this._browserSetState ? NOTIFY_BROWSER_STATE_RESTORED : NOTIFY_WINDOWS_RESTORED,
-        "");
-      this._browserSetState = false;
+      if (this._restoreCount == 0) {
+        // This was the last window restored at startup, notify observers.
+        this._observerService.notifyObservers(null,
+          this._browserSetState ? NOTIFY_BROWSER_STATE_RESTORED : NOTIFY_WINDOWS_RESTORED,
+          "");
+        this._browserSetState = false;
+      }
     }
   },
 

browser/components/sessionstore/test/browser/Makefile.in

@@ -109,6 +109,7 @@ _BROWSER_TEST_FILES = \
 	browser_514751.js \
 	browser_522545.js \
 	browser_526613.js \
+	browser_528776.js \
 	$(NULL)
 
 libs:: $(_BROWSER_TEST_FILES)

browser/components/sessionstore/test/browser/browser_528776.js

@@ -0,0 +1,29 @@
+var ss = Cc["@mozilla.org/browser/sessionstore;1"].getService(Ci.nsISessionStore);
+var wm = Cc["@mozilla.org/appshell/window-mediator;1"].getService(Ci.nsIWindowMediator);
+
+function browserWindowsCount(expected) {
+  var count = 0;
+  var e = wm.getEnumerator("navigator:browser");
+  while (e.hasMoreElements()) {
+    if (!e.getNext().closed)
+      ++count;
+  }
+  is(count, expected,
+     "number of open browser windows according to nsIWindowMediator");
+  is(JSON.parse(ss.getBrowserState()).windows.length, expected,
+     "number of open browser windows according to getBrowserState");
+}
+
+function test() {
+  waitForExplicitFinish();
+
+  browserWindowsCount(1);
+
+  var win = openDialog(location, "", "chrome,all,dialog=no");
+  win.addEventListener("load", function () {
+    browserWindowsCount(2);
+    win.close();
+    browserWindowsCount(1);
+    finish();
+  }, false);
+}