Bug 1177310 - Don't flush windows synchronously on application shutdown. r?Yoric draft
authorMike Conley <mconley@mozilla.com>
Fri, 13 Nov 2015 14:46:37 -0500
changeset 308822 6fa5635fdb12407c239697125556ef5d01acc29d
parent 308821 466999ae27f8a052f305fea29881c2f54844fba4
child 511217 ef611e6197a88489544496385d94fdc2e953ce18
push id7531
push usermconley@mozilla.com
push dateFri, 13 Nov 2015 22:48:42 +0000
reviewersYoric
bugs1177310
milestone45.0a1
Bug 1177310 - Don't flush windows synchronously on application shutdown. r?Yoric
browser/components/sessionstore/SessionStore.jsm
browser/components/sessionstore/test/browser_broadcast.js
--- a/browser/components/sessionstore/SessionStore.jsm
+++ b/browser/components/sessionstore/SessionStore.jsm
@@ -25,17 +25,17 @@ const NOTIFY_TAB_RESTORED = "sessionstor
 
 // Maximum number of tabs to restore simultaneously. Previously controlled by
 // the browser.sessionstore.max_concurrent_tabs pref.
 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",
 ];
 
 // XUL Window properties to (re)store
 // Restored in restoreDimensions()
 const WINDOW_ATTRIBUTES = ["width", "height", "screenX", "screenY", "sizemode"];
@@ -170,16 +170,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "TabStateCache",
   "resource:///modules/sessionstore/TabStateCache.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TabStateFlusher",
   "resource:///modules/sessionstore/TabStateFlusher.jsm");
 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.
  * Debug mode is controlled by preference browser.sessionstore.debug
  */
 var gDebuggingEnabled = false;
 function debug(aMsg) {
   if (gDebuggingEnabled) {
@@ -616,18 +618,18 @@ var SessionStoreInternal = {
   observe: function ssi_observe(aSubject, aTopic, aData) {
     switch (aTopic) {
       case "browser-window-before-show": // catch new windows
         this.onBeforeBrowserWindowShown(aSubject);
         break;
       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();
         break;
       case "quit-application":
         this.onQuitApplication(aData);
         break;
       case "browser:purge-session-history": // catch sanitization
@@ -1353,33 +1355,42 @@ var SessionStoreInternal = {
         this._capClosedWindows();
       } else if (!shouldStore && alreadyStored) {
         this._closedWindows.splice(winIndex, 1);
       }
     }
   },
 
   /**
-   * On quit application requested
+   * On quit application granted
    */
-  onQuitApplicationRequested: function ssi_onQuitApplicationRequested() {
-    // get a current snapshot of all windows
-    this._forEachBrowserWindow(function(aWindow) {
+  onQuitApplicationGranted: function ssi_onQuitApplicationGranted() {
+    // Create a Promise to get a current snapshot of all windows
+    AsyncShutdown.quitApplicationGranted.addBlocker(
+      "SessionStore: flushing all windows", this._flushAllWindowsAsync());
+  },
+
+  _flushAllWindowsAsync: Task.async(function*() {
+    let windows = [];
+    this._forEachBrowserWindow((win) => windows.push(win));
+
+    for (let win of windows) {
       // Flush all data queued in the content script to not lose it when
       // shutting down.
-      TabState.flushWindow(aWindow);
-      this._collectWindowData(aWindow);
-    });
-    // 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();
-  },
+      yield TabStateFlusher.flushWindow(win);
+      this._collectWindowData(win);
+
+      // 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
    */
   onLastWindowCloseGranted: function ssi_onLastWindowCloseGranted() {
     // last browser window is quitting.
     // remember to restore the last window when another browser window is opened
     // do not account for pref(resume_session_once) at this point, as it might be
--- a/browser/components/sessionstore/test/browser_broadcast.js
+++ b/browser/components/sessionstore/test/browser_broadcast.js
@@ -18,39 +18,16 @@ add_task(function flush_on_tabclose() {
 
   let [{state: {storage}}] = JSON.parse(ss.getClosedTabData(window));
   is(storage["http://example.com"].test, "on-tab-close",
     "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.
  */
 add_task(function flush_on_duplicate() {
   let tab = yield createTabWithStorageData(["http://example.com"]);
   let browser = tab.linkedBrowser;
 
   yield modifySessionStorage(browser, {test: "on-duplicate"});
   let tab2 = ss.duplicateTab(window, tab);
@@ -148,14 +125,8 @@ function createTabWithStorageData(urls, 
       browser.loadURI(url);
       yield promiseBrowserLoaded(browser);
       yield modifySessionStorage(browser, {test: INITIAL_VALUE});
     }
 
     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);
-}