Bug 1227444 - Remove a closed window from _windows right away without waiting for a flush to complete. r=billm
authorMike Conley <mconley@mozilla.com>
Tue, 24 Nov 2015 18:11:33 -0500
changeset 274187 2cbf3af88ad3b08cbd211b71fe83bc45868b3719
parent 274186 3e7edd0d28a6ab21191fac9946ea0a3ac7eecc98
child 274188 1213752bdbed8228ca34dbe44cc42ae751141f10
push id29724
push usercbook@mozilla.com
push dateThu, 26 Nov 2015 10:57:21 +0000
treeherdermozilla-central@c321d8403851 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1227444
milestone45.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 1227444 - Remove a closed window from _windows right away without waiting for a flush to complete. r=billm
browser/components/sessionstore/SessionStore.jsm
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -1245,32 +1245,45 @@ var SessionStoreInternal = {
       // Store the window's close date to figure out when each individual tab
       // was closed. This timestamp should allow re-arranging data based on how
       // recently something was closed.
       winData.closedAt = Date.now();
 
       // we don't want to save the busy state
       delete winData.busy;
 
+      // When closing windows one after the other until Firefox quits, we
+      // will move those closed in series back to the "open windows" bucket
+      // before writing to disk. If however there is only a single window
+      // with tabs we deem not worth saving then we might end up with a
+      // random closed or even a pop-up window re-opened. To prevent that
+      // we explicitly allow saving an "empty" window state.
+      let isLastWindow =
+        Object.keys(this._windows).length == 1 &&
+        !this._closedWindows.some(win => win._shouldRestore || false);
+
+      // clear this window from the list, since it has definitely been closed.
+      delete this._windows[aWindow.__SSi];
+
       // Now we have to figure out if this window is worth saving in the _closedWindows
       // Object.
       //
       // We're about to flush the tabs from this window, but it's possible that we
       // might never hear back from the content process(es) in time before the user
       // chooses to restore the closed window. So we do the following:
       //
       // 1) Use the tab state cache to determine synchronously if the window is
       //    worth stashing in _closedWindows.
       // 2) Flush the window.
       // 3) When the flush is complete, revisit our decision to store the window
       //    in _closedWindows, and add/remove as necessary.
       if (!winData.isPrivate) {
         // Remove any open private tabs the window may contain.
         PrivacyFilter.filterPrivateTabs(winData);
-        this.maybeSaveClosedWindow(winData);
+        this.maybeSaveClosedWindow(winData, isLastWindow);
       }
 
       // The tabbrowser binding will go away once the window is closed,
       // so we'll hold a reference to the browsers in the closure here.
       let browsers = tabbrowser.browsers;
 
       TabStateFlusher.flushWindow(aWindow).then(() => {
         // At this point, aWindow is closed! You should probably not try to
@@ -1287,38 +1300,35 @@ var SessionStoreInternal = {
         }
 
         // Save non-private windows if they have at
         // least one saveable tab or are the last window.
         if (!winData.isPrivate) {
           // It's possible that a tab switched its privacy state at some point
           // before our flush, so we need to filter again.
           PrivacyFilter.filterPrivateTabs(winData);
-          this.maybeSaveClosedWindow(winData);
+          this.maybeSaveClosedWindow(winData, isLastWindow);
         }
 
-        // clear this window from the list
-        delete this._windows[aWindow.__SSi];
         // Update the tabs data now that we've got the most
         // recent information.
         this.cleanUpWindow(aWindow, winData);
 
         // save the state without this window to disk
         this.saveStateDelayed();
       });
     } else {
       this.cleanUpWindow(aWindow, winData);
     }
 
     for (let i = 0; i < tabbrowser.tabs.length; i++) {
       this.onTabRemove(aWindow, tabbrowser.tabs[i], true);
     }
   },
 
-
   /**
    * Clean up the message listeners on a window that has finally
    * gone away. Call this once you're sure you don't want to hear
    * from any of this windows tabs from here forward.
    *
    * @param aWindow
    *        The browser window we're cleaning up.
    * @param winData
@@ -1340,32 +1350,29 @@ var SessionStoreInternal = {
    * Decides whether or not a closed window should be put into the
    * _closedWindows Object. This might be called multiple times per
    * window, and will do the right thing of moving the window data
    * in or out of _closedWindows if the winData indicates that our
    * need for saving it has changed.
    *
    * @param winData
    *        The data for the closed window that we might save.
+   * @param isLastWindow
+   *        Whether or not the window being closed is the last
+   *        browser window. Callers of this function should pass
+   *        in the value of SessionStoreInternal.atLastWindow for
+   *        this argument, and pass in the same value if they happen
+   *        to call this method again asynchronously (for example, after
+   *        a window flush).
    */
-  maybeSaveClosedWindow(winData) {
+  maybeSaveClosedWindow(winData, isLastWindow) {
     if (RunState.isRunning) {
       // Determine whether the window has any tabs worth saving.
       let hasSaveableTabs = winData.tabs.some(this._shouldSaveTabState);
 
-      // When closing windows one after the other until Firefox quits, we
-      // will move those closed in series back to the "open windows" bucket
-      // before writing to disk. If however there is only a single window
-      // with tabs we deem not worth saving then we might end up with a
-      // random closed or even a pop-up window re-opened. To prevent that
-      // we explicitly allow saving an "empty" window state.
-      let isLastWindow =
-        Object.keys(this._windows).length == 1 &&
-        !this._closedWindows.some(win => win._shouldRestore || false);
-
       // Note that we might already have this window stored in
       // _closedWindows from a previous call to this function.
       let winIndex = this._closedWindows.indexOf(winData);
       let alreadyStored = (winIndex != -1);
       let shouldStore = (hasSaveableTabs || isLastWindow);
 
       if (shouldStore && !alreadyStored) {
         let index = this._closedWindows.findIndex(win => {