Bug 1171708 - Have SessionStore asynchronous collect window information on close. r?billm draft
authorMike Conley <mconley@mozilla.com>
Thu, 12 Nov 2015 14:21:21 -0500
changeset 308845 d674ea45bce3b413dabfabfc9cf62e92f29ee0c1
parent 308844 c84d9f53b335701b92816e3b24f4f7013dc63f42
child 308846 e3cd17601fc8f58d8161e61459012235cf520222
push id7541
push usermconley@mozilla.com
push dateMon, 16 Nov 2015 00:50:56 +0000
reviewersbillm
bugs1171708
milestone45.0a1
Bug 1171708 - Have SessionStore asynchronous collect window information on close. r?billm
browser/components/sessionstore/SessionStore.jsm
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -391,16 +391,18 @@ var SessionStoreInternal = {
   // associated frameLoader we heard about.
   _lastKnownFrameLoader: new WeakMap(),
 
   // A map (xul:browser -> object) that maps a browser associated with a
   // recently closed tab to all its necessary state information we need to
   // properly handle final update message.
   _closedTabs: new WeakMap(),
 
+  _closedWindowTabs: new WeakMap(),
+
   // whether a setBrowserState call is in progress
   _browserSetState: false,
 
   // time in milliseconds when the session was started (saved across sessions),
   // defaults to now if no session was restored or timestamp doesn't exist
   _sessionStartTime: Date.now(),
 
   // states for all currently opened windows
@@ -647,18 +649,18 @@ var SessionStoreInternal = {
    * This method handles incoming messages sent by the session store content
    * script via the Frame Message Manager or Parent Process Message Manager,
    * and thus enables communication with OOP tabs.
    */
   receiveMessage(aMessage) {
     // If we got here, that means we're dealing with a frame message
     // manager message, so the target will be a <xul:browser>.
     var browser = aMessage.target;
-    var win = browser.ownerDocument.defaultView;
-    let tab = win.gBrowser.getTabForBrowser(browser);
+    let win = browser.ownerDocument.defaultView;
+    let tab = win ? win.gBrowser.getTabForBrowser(browser) : null;
 
     // Ensure we receive only specific messages from <xul:browser>s that
     // have no tab assigned, e.g. the ones that preload about:newtab pages.
     if (!tab && !NOTAB_MESSAGES.has(aMessage.name)) {
       throw new Error(`received unexpected message '${aMessage.name}' ` +
                       `from a browser that has no tab`);
     }
 
@@ -746,16 +748,26 @@ var SessionStoreInternal = {
             } else if (!shouldSave && index > -1) {
               // Remove from the list of closed tabs. The update messages sent
               // after the tab was closed changed enough state so that we no
               // longer consider its data interesting enough to keep around.
               this.removeClosedTabData(closedTabs, index);
             }
           }
         }
+
+        if (this._closedWindowTabs.has(browser.permanentKey)) {
+          let tabData = this._closedWindowTabs.get(browser.permanentKey);
+          TabState.copyFromCache(browser, tabData);
+          // For closed window tabs, we really only care about the last flush
+          // we requested, since we will be detaching the message listeners for
+          // the window as soon as the last flush update comes in.
+          this._closedWindowTabs.delete(browser.permanentKey);
+        }
+
         break;
       case "SessionStore:restoreHistoryComplete":
         // Notify the tabbrowser that the tab chrome has been restored.
         let tabData = TabState.collect(tab);
 
         // wall-paper fix for bug 439675: make sure that the URL to be loaded
         // is always visible in the address bar
         let activePageData = tabData.entries[tabData.index - 1] || null;
@@ -1197,21 +1209,24 @@ var SessionStoreInternal = {
     }, this);
 
     aWindow.gBrowser.removeEventListener("XULFrameLoaderCreated", this);
 
     let winData = this._windows[aWindow.__SSi];
 
     // Collect window data only when *not* closed during shutdown.
     if (RunState.isRunning) {
-      // Flush all data queued in the content script before the window is gone.
-      TabState.flushWindow(aWindow);
-
-      // update all window data for a last time
-      this._collectWindowData(aWindow);
+      // Grab the most recent window data. The tab data will be updated
+      // once we finish flushing all of the messages from the tabs.
+      let tabMap = this._collectWindowData(aWindow);
+
+      for (let [tab, tabData] of tabMap) {
+        let permanentKey = tab.linkedBrowser.permanentKey;
+        this._closedWindowTabs.set(permanentKey, tabData);
+      }
 
       if (isFullyLoaded) {
         winData.title = tabbrowser.selectedBrowser.contentTitle || tabbrowser.selectedTab.label;
         winData.title = this._replaceLoadingTitle(winData.title, tabbrowser,
                                                   tabbrowser.selectedTab);
         SessionCookies.update([winData]);
       }
 
@@ -1221,65 +1236,133 @@ var SessionStoreInternal = {
         winData._shouldRestore = true;
       }
 
       // 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();
 
-      // Save non-private windows if they have at
-      // least one saveable tab or are the last window.
+      // we don't want to save the busy state
+      delete winData.busy;
+
+      // 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);
-
-        // 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);
-
-        if (hasSaveableTabs || isLastWindow) {
-          // we don't want to save the busy state
-          delete winData.busy;
-
-          this._closedWindows.unshift(winData);
-          this._capClosedWindows();
+        this._maybeSaveClosedWindow(winData);
+      }
+
+      TabStateFlusher.flushWindow(aWindow).then(() => {
+        // At this point, aWindow is closed! You should not try to access any
+        // DOM elements from aWindow within this callback.
+
+        // 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);
         }
-      }
-
-      // clear this window from the list
-      delete this._windows[aWindow.__SSi];
-
-      // save the state without this window to disk
-      this.saveStateDelayed();
+
+        // 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
+   *        The data for the window that we should hold in the
+   *        DyingWindowCache in case anybody is still holding a
+   *        reference to it.
+   */
+  _cleanUpWindow(aWindow, winData) {
     // Cache the window state until it is completely gone.
     DyingWindowCache.set(aWindow, winData);
 
     let mm = aWindow.getGroupMessageManager("browsers");
     MESSAGES.forEach(msg => mm.removeMessageListener(msg, this));
 
     delete aWindow.__SSi;
   },
 
   /**
+   * 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.
+   */
+  _maybeSaveClosedWindow(winData) {
+    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) {
+        this._closedWindows.unshift(winData);
+        this._capClosedWindows();
+      } else if (!shouldStore && alreadyStored) {
+        this._closedWindows.splice(winIndex, 1);
+      }
+    }
+  },
+
+  /**
    * On quit application requested
    */
   onQuitApplicationRequested: function ssi_onQuitApplicationRequested() {
     // get a current snapshot of all windows
     this._forEachBrowserWindow(function(aWindow) {
       // Flush all data queued in the content script to not lose it when
       // shutting down.
       TabState.flushWindow(aWindow);
@@ -1615,16 +1698,17 @@ var SessionStoreInternal = {
     let [closedTab] = closedTabs.splice(index, 1);
 
     // If the closed tab's state still has a .permanentKey property then we
     // haven't seen its final update message yet. Remove it from the map of
     // closed tabs so that we will simply discard its last messages and will
     // not add it back to the list of closed tabs again.
     if (closedTab.permanentKey) {
       this._closedTabs.delete(closedTab.permanentKey);
+      this._closedWindowTabs.delete(closedTab.permanentKey);
       delete closedTab.permanentKey;
     }
 
     return closedTab;
   },
 
   /**
    * When a tab is selected, save session data
@@ -2573,40 +2657,54 @@ var SessionStoreInternal = {
     }
 
     let windows = [this._windows[aWindow.__SSi]];
     SessionCookies.update(windows);
 
     return { windows: windows };
   },
 
+  /**
+   * Gathers data about a window and its tabs, and updates its
+   * entry in this._windows.
+   *
+   * @param aWindow
+   *        Window references.
+   * @returns a Map mapping the browser tabs from aWindow to the tab
+   *          entry that was put into the window data in this._windows.
+   */
   _collectWindowData: function ssi_collectWindowData(aWindow) {
+    let tabMap = new Map();
+
     if (!this._isWindowLoaded(aWindow))
-      return;
+      return tabMap;
 
     let tabbrowser = aWindow.gBrowser;
     let tabs = tabbrowser.tabs;
     let winData = this._windows[aWindow.__SSi];
     let tabsData = winData.tabs = [];
 
     // update the internal state data for this window
     for (let tab of tabs) {
-      tabsData.push(TabState.collect(tab));
+      let tabData = TabState.collect(tab);
+      tabMap.set(tab, tabData);
+      tabsData.push(tabData);
     }
     winData.selected = tabbrowser.mTabBox.selectedIndex + 1;
 
     this._updateWindowFeatures(aWindow);
 
     // Make sure we keep __SS_lastSessionWindowID around for cases like entering
     // or leaving PB mode.
     if (aWindow.__SS_lastSessionWindowID)
       this._windows[aWindow.__SSi].__lastSessionWindowID =
         aWindow.__SS_lastSessionWindowID;
 
     DirtyWindows.remove(aWindow);
+    return tabMap;
   },
 
   /* ........ Restoring Functionality .............. */
 
   /**
    * restore features to a single window
    * @param aWindow
    *        Window reference to the window to use for restoration