Bug 1177310 - Don't flush windows synchronously on application shutdown. r=billm,feedback=ttaubert

Instead of synchronously flushing the windows, we use AsyncShutdown to
wait for all window flushes to finish.

--HG--
extra : commitid : 1dDdTLBcZrt
extra : rebase_source : ea10bed3dc4920e769e73a6469510d4f303b88dc

browser/components/sessionstore/RunState.jsm

@@ -19,14 +19,6 @@ const STATE_CLOSED = 4;
 // We're initially stopped.
 var state = STATE_STOPPED;
 
-function observer(subj, topic) {
-  Services.obs.removeObserver(observer, topic);
-  state = STATE_QUITTING;
-}
-
-// Listen for when the application is quitting.
-Services.obs.addObserver(observer, "quit-application-granted", false);
-
 /**
  * This module keeps track of SessionStore's current run state. We will
  * always start out at STATE_STOPPED. After the sessionw as read from disk and
@@ -95,5 +87,14 @@ this.RunState = Object.freeze({
     if (this.isClosing) {
       state = STATE_CLOSED;
     }
-  }
+  },
+
+  // Switch the run state to STATE_QUITTING. This should be called once we're
+  // certain that the browser is going away and before we start collecting the
+  // final window states to save in the session file.
+  setQuitting() {
+    if (this.isRunning) {
+      state = STATE_QUITTING;
+    }
+  },
 });

browser/components/sessionstore/SessionStore.jsm

@@ -31,7 +31,7 @@ const MAX_CONCURRENT_TAB_RESTORES = 3;
 // global notifications observed
 const OBSERVING = [
   "browser-window-before-show", "domwindowclosed",
-  "quit-application-requested", "browser-lastwindow-close-granted",
+  "quit-application-granted", "browser-lastwindow-close-granted",
   "quit-application", "browser:purge-session-history",
   "browser:purge-domain-data",
   "idle-daily",
@@ -180,6 +180,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "Utils",
   "resource:///modules/sessionstore/Utils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ViewSourceBrowser",
   "resource://gre/modules/ViewSourceBrowser.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
+  "resource://gre/modules/AsyncShutdown.jsm");
 
 /**
  * |true| if we are in debug mode, |false| otherwise.
@@ -627,8 +629,8 @@ var SessionStoreInternal = {
       case "domwindowclosed": // catch closed windows
         this.onClose(aSubject);
         break;
-      case "quit-application-requested":
-        this.onQuitApplicationRequested();
+      case "quit-application-granted":
+        this.onQuitApplicationGranted();
         break;
       case "browser-lastwindow-close-granted":
         this.onLastWindowCloseGranted();
@@ -1402,23 +1404,76 @@ var SessionStoreInternal = {
   },
 
   /**
-   * On quit application requested
+   * On quit application granted
    */
-  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);
-      this._collectWindowData(aWindow);
+  onQuitApplicationGranted: function ssi_onQuitApplicationGranted() {
+    // Collect an initial snapshot of window data before we do the flush
+    this._forEachBrowserWindow((win) => {
+      this._collectWindowData(win);
     });
-    // we must cache this because _getMostRecentBrowserWindow will always
-    // return null by the time quit-application occurs
+
+    // Now add an AsyncShutdown blocker that'll spin the event loop
+    // until the windows have all been flushed.
+
+    // This progress object will track the state of async window flushing
+    // and will help us debug things that go wrong with our AsyncShutdown
+    // blocker.
+    let progress = { total: -1, current: -1 };
+
+    // We're going down! Switch state so that we treat closing windows and
+    // tabs correctly.
+    RunState.setQuitting();
+
+    AsyncShutdown.quitApplicationGranted.addBlocker(
+      "SessionStore: flushing all windows",
+      this.flushAllWindowsAsync(progress),
+      () => progress);
+  },
+
+  /**
+   * An async Task that iterates all open browser windows and flushes
+   * any outstanding messages from their tabs. This will also close
+   * all of the currently open windows while we wait for the flushes
+   * to complete.
+   *
+   * @param progress (Object)
+   *        Optional progress object that will be updated as async
+   *        window flushing progresses. flushAllWindowsSync will
+   *        write to the following properties:
+   *
+   *        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 = [];
+    // 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();
+    });
+
+    progress.total = windowPromises.length;
+
+    // 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];
+    };
+
+    // 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();
-  },
+  }),
 
   /**
    * On last browser window close

browser/components/sessionstore/test/browser_broadcast.js

@@ -21,29 +21,6 @@ add_task(function flush_on_tabclose() {
     "sessionStorage data has been flushed on TabClose");
 });
 
-/**
- * This test ensures we won't lose tab data queued in the content script when
- * the application tries to quit.
- */
-add_task(function flush_on_quit_requested() {
-  let tab = yield createTabWithStorageData(["http://example.com"]);
-  let browser = tab.linkedBrowser;
-
-  yield modifySessionStorage(browser, {test: "on-quit-requested"});
-
-  // Note that sending quit-application-requested should not interfere with
-  // other tests and code. We're just notifying about a shutdown request but
-  // we will not send quit-application-granted. Observers will thus assume
-  // that some other observer has canceled the request.
-  sendQuitApplicationRequested();
-
-  let {storage} = JSON.parse(ss.getTabState(tab));
-  is(storage["http://example.com"].test, "on-quit-requested",
-    "sessionStorage data has been flushed when a quit is requested");
-
-  gBrowser.removeTab(tab);
-});
-
 /**
  * This test ensures we won't lose tab data queued in the content script when
  * duplicating a tab.
@@ -152,9 +129,3 @@ function createTabWithStorageData(urls, win = window) {
     throw new Task.Result(tab);
   });
 }
-
-function sendQuitApplicationRequested() {
-  let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"]
-                     .createInstance(Ci.nsISupportsPRBool);
-  Services.obs.notifyObservers(cancelQuit, "quit-application-requested", null);
-}