Bug 1284687 - Hide windows on shutdown while persisting session instead of closing them. r=billm a=lizzard
authorMike Conley <mconley@mozilla.com>
Thu, 07 Jul 2016 15:04:52 -0400
changeset 333664 0b4087792d5fa736a0dc18df7aeefecf0a34d99f
parent 333663 36faf180f49d6e6af84f55231052b1bac85b3bcd
child 333665 8139a58cc0c18b47b6945f2a26fc347ef6f3a0b5
push id6215
push userkwierso@gmail.com
push dateThu, 14 Jul 2016 17:55:05 +0000
treeherdermozilla-beta@0b4087792d5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm, lizzard
bugs1284687
milestone48.0
Bug 1284687 - Hide windows on shutdown while persisting session instead of closing them. r=billm a=lizzard We were closing the windows before to improve perceived shutdown performance, but we end up in a state where we're likely to miss out on the last ~2 seconds of session activity for most tabs per window. This is because we were removing the session update message listeners and resolving the flush Promises once the domwindowclosed notification fired for the window. Hiding the window allows us to wait for the messages properly. What's more, we weren't even collecting the window state after we had flushed, so we have _always_ been missing (in the worst case) about 2 seconds of session state per window. This addresses that. MozReview-Commit-ID: BEOIHV4EErf
browser/components/sessionstore/SessionStore.jsm
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -1478,32 +1478,43 @@ var SessionStoreInternal = {
    *        total (int):
    *          The total number of windows to be flushed.
    *        current (int):
    *          The current window that we're waiting for a flush on.
    *
    * @return Promise
    */
   flushAllWindowsAsync: Task.async(function*(progress={}) {
-    let windowPromises = [];
+    let windowPromises = new Map();
     // We collect flush promises and close each window immediately so that
     // the user can't start changing any window state while we're waiting
     // for the flushes to finish.
     this._forEachBrowserWindow((win) => {
-      windowPromises.push(TabStateFlusher.flushWindow(win));
-      win.close();
+      windowPromises.set(win, TabStateFlusher.flushWindow(win));
+
+      // We have to wait for these messages to come up from
+      // each window and each browser. In the meantime, hide
+      // the windows to improve perceived shutdown speed.
+      let baseWin = win.QueryInterface(Ci.nsIInterfaceRequestor)
+                       .getInterface(Ci.nsIDocShell)
+                       .QueryInterface(Ci.nsIDocShellTreeItem)
+                       .treeOwner
+                       .QueryInterface(Ci.nsIBaseWindow);
+      baseWin.visibility = false;
     });
 
-    progress.total = windowPromises.length;
+    progress.total = windowPromises.size;
+    progress.current = 0;
 
     // We'll iterate through the Promise array, yielding each one, so as to
     // provide useful progress information to AsyncShutdown.
-    for (let i = 0; i < windowPromises.length; ++i) {
-      progress.current = i;
-      yield windowPromises[i];
+    for (let [win, promise] of windowPromises) {
+      yield promise;
+      this._collectWindowData(win);
+      progress.current++;
     };
 
     // We must cache this because _getMostRecentBrowserWindow will always
     // return null by the time quit-application occurs.
     var activeWindow = this._getMostRecentBrowserWindow();
     if (activeWindow)
       this.activeWindowSSiCache = activeWindow.__SSi || "";
     DirtyWindows.clear();