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 325432 2299c291b4398bbb8caa79bfc6064102e053e3ff
parent 325431 a2992d672f629f1e56eb35c220779b77eaf53ba8
child 325433 6893a8d54639abb5d331896858af8a305584c1b6
push id9789
push userkwierso@gmail.com
push dateThu, 14 Jul 2016 17:54:19 +0000
treeherdermozilla-aurora@2299c291b439 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm, lizzard
bugs1284687
milestone49.0a2
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
@@ -1483,32 +1483,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();