Backed out changeset 18eec0849a1f (Bug 1195295) for leaking.

This changeset is causing shutdown leaks (bug 1234697 and bug 1220517).

browser/components/sessionstore/SessionStore.jsm

@@ -49,6 +49,10 @@ const WINDOW_HIDEABLE_FEATURES = [
 
 // Messages that will be received via the Frame Message Manager.
 const MESSAGES = [
+  // The content script gives us a reference to an object that performs
+  // synchronous collection of session data.
+  "SessionStore:setupSyncHandler",
+
   // The content script sends us data that has been invalidated and needs to
   // be saved to disk.
   "SessionStore:update",
@@ -79,6 +83,9 @@ const MESSAGES = [
 // ones that preload about:newtab pages, or from browsers where the window
 // has just been closed.
 const NOTAB_MESSAGES = new Set([
+  // For a description see above.
+  "SessionStore:setupSyncHandler",
+
   // For a description see above.
   "SessionStore:crashedTabRevived",
 
@@ -92,6 +99,9 @@ const NOTAB_MESSAGES = new Set([
 // The list of messages we accept without an "epoch" parameter.
 // See getCurrentEpoch() and friends to find out what an "epoch" is.
 const NOEPOCH_MESSAGES = new Set([
+  // For a description see above.
+  "SessionStore:setupSyncHandler",
+
   // For a description see above.
   "SessionStore:crashedTabRevived",
 
@@ -691,6 +701,9 @@ var SessionStoreInternal = {
     }
 
     switch (aMessage.name) {
+      case "SessionStore:setupSyncHandler":
+        TabState.setSyncHandler(browser, aMessage.objects.handler);
+        break;
       case "SessionStore:update":
         // |browser.frameLoader| might be empty if the browser was already
         // destroyed and its tab removed. In that case we still have the last

browser/components/sessionstore/TabState.jsm

@@ -27,10 +27,22 @@ XPCOMUtils.defineLazyModuleGetter(this, "Utils",
  * Module that contains tab state collection methods.
  */
 this.TabState = Object.freeze({
+  setSyncHandler: function (browser, handler) {
+    TabStateInternal.setSyncHandler(browser, handler);
+  },
+
   update: function (browser, data) {
     TabStateInternal.update(browser, data);
   },
 
+  flushAsync: function (browser) {
+    TabStateInternal.flushAsync(browser);
+  },
+
+  flushWindow: function (window) {
+    TabStateInternal.flushWindow(window);
+  },
+
   collect: function (tab) {
     return TabStateInternal.collect(tab);
   },
@@ -41,15 +53,62 @@ this.TabState = Object.freeze({
 
   copyFromCache(browser, tabData, options) {
     TabStateInternal.copyFromCache(browser, tabData, options);
-  },
+  }
 });
 
 var TabStateInternal = {
+  // A map (xul:browser -> handler) that maps a tab to the
+  // synchronous collection handler object for that tab.
+  // See SyncHandler in content-sessionStore.js.
+  _syncHandlers: new WeakMap(),
+
+  // A map (xul:browser -> int) that maps a browser to the
+  // last "SessionStore:update" message ID we received for it.
+  _latestMessageID: new WeakMap(),
+
+  /**
+   * Install the sync handler object from a given tab.
+   */
+  setSyncHandler: function (browser, handler) {
+    this._syncHandlers.set(browser.permanentKey, handler);
+    this._latestMessageID.set(browser.permanentKey, 0);
+  },
+
   /**
    * Processes a data update sent by the content script.
    */
-  update: function (browser, {data}) {
-    TabStateCache.update(browser, data);
+  update: function (browser, {id, data}) {
+    // Only ever process messages that have an ID higher than the last one we
+    // saw. This ensures we don't use stale data that has already been received
+    // synchronously.
+    if (id > this._latestMessageID.get(browser.permanentKey)) {
+      this._latestMessageID.set(browser.permanentKey, id);
+      TabStateCache.update(browser, data);
+    }
+  },
+
+  /**
+   * DO NOT USE - DEBUGGING / TESTING ONLY
+   *
+   * This function is used to simulate certain situations where race conditions
+   * can occur by sending data shortly before flushing synchronously.
+   */
+  flushAsync: function(browser) {
+    if (this._syncHandlers.has(browser.permanentKey)) {
+      this._syncHandlers.get(browser.permanentKey).flushAsync();
+    }
+  },
+
+  /**
+   * Flushes queued content script data for all browsers of a given window.
+   */
+  flushWindow: function (window) {
+    for (let browser of window.gBrowser.browsers) {
+      if (this._syncHandlers.has(browser.permanentKey)) {
+        let lastID = this._latestMessageID.get(browser.permanentKey);
+        this._syncHandlers.get(browser.permanentKey).flush(lastID);
+      }
+    }
   },
 
   /**

browser/components/sessionstore/content/content-sessionStore.js

@@ -191,6 +191,52 @@ var MessageListener = {
   }
 };
 
+/**
+ * On initialization, this handler gets sent to the parent process as a CPOW.
+ * The parent will use it only to flush pending data from the frame script
+ * when needed, i.e. when closing a tab, closing a window, shutting down, etc.
+ *
+ * This will hopefully not be needed in the future once we have async APIs for
+ * closing windows and tabs.
+ */
+var SyncHandler = {
+  init: function () {
+    // Send this object as a CPOW to chrome. In single-process mode,
+    // the synchronous send ensures that the handler object is
+    // available in SessionStore.jsm immediately upon loading
+    // content-sessionStore.js.
+    sendSyncMessage("SessionStore:setupSyncHandler", {}, {handler: this});
+  },
+
+  /**
+   * This function is used to make the tab process flush all data that
+   * hasn't been sent to the parent process, yet.
+   *
+   * @param id (int)
+   *        A unique id that represents the last message received by the chrome
+   *        process before flushing. We will use this to determine data that
+   *        would be lost when data has been sent asynchronously shortly
+   *        before flushing synchronously.
+   */
+  flush: function (id) {
+    MessageQueue.flush(id);
+  },
+
+  /**
+   * DO NOT USE - DEBUGGING / TESTING ONLY
+   *
+   * This function is used to simulate certain situations where race conditions
+   * can occur by sending data shortly before flushing synchronously.
+   */
+  flushAsync: function () {
+    if (!Services.prefs.getBoolPref("browser.sessionstore.debug")) {
+      throw new Error("flushAsync() must be used for testing, only.");
+    }
+
+    MessageQueue.send();
+  }
+};
+
 /**
  * Listens for changes to the session history. Whenever the user navigates
  * we will collect URLs and everything belonging to session history.
@@ -589,6 +635,13 @@ var PrivacyListener = {
  * flood.
  */
 var MessageQueue = {
+  /**
+   * A unique, monotonically increasing ID used for outgoing messages. This is
+   * important to make it possible to reuse tabs and allow sync flushes before
+   * data could be destroyed.
+   */
+  _id: 1,
+
   /**
    * A map (string -> lazy fn) holding lazy closures of all queued data
    * collection routines. These functions will return data collected from the
@@ -596,6 +649,13 @@ var MessageQueue = {
    */
   _data: new Map(),
 
+  /**
+   * A map holding the |this._id| value for every type of data back when it
+   * was pushed onto the queue. We will use those IDs to find the data to send
+   * and flush.
+   */
+  _lastUpdated: new Map(),
+
   /**
    * The delay (in ms) used to delay sending changes after data has been
    * invalidated.
@@ -669,6 +729,7 @@ var MessageQueue = {
    */
   push: function (key, fn) {
     this._data.set(key, createLazy(fn));
+    this._lastUpdated.set(key, this._id);
 
     if (!this._timeout && !this._timeoutDisabled) {
       // Wait a little before sending the message to batch multiple changes.
@@ -680,6 +741,8 @@ var MessageQueue = {
    * Sends queued data to the chrome process.
    *
    * @param options (object)
+   *        {id: 123} to override the update ID used to accumulate data to send.
+   *        {sync: true} to send data to the parent process synchronously.
    *        {flushID: 123} to specify that this is a flush
    *        {isFinal: true} to signal this is the final message sent on unload
    */
@@ -696,14 +759,36 @@ var MessageQueue = {
       this._timeout = null;
     }
 
+    let sync = options && options.sync;
+    let startID = (options && options.id) || this._id;
     let flushID = (options && options.flushID) || 0;
 
+    // We use sendRpcMessage in the sync case because we may have been called
+    // through a CPOW. RPC messages are the only synchronous messages that the
+    // child is allowed to send to the parent while it is handling a CPOW
+    // request.
+    let sendMessage = sync ? sendRpcMessage : sendAsyncMessage;
+
     let durationMs = Date.now();
 
     let data = {};
     let telemetry = {};
-    for (let [key, func] of this._data) {
-      let value = func();
+    for (let [key, id] of this._lastUpdated) {
+      // There is no data for the given key anymore because
+      // the parent process already marked it as received.
+      if (!this._data.has(key)) {
+        continue;
+      }
+
+      if (startID > id) {
+        // If the |id| passed by the parent process is higher than the one
+        // stored in |_lastUpdated| for the given key we know that the parent
+        // received all necessary data and we can remove it from the map.
+        this._data.delete(key);
+        continue;
+      }
+
+      let value = this._data.get(key)();
       if (key == "telemetry") {
         for (let histogramId of Object.keys(value)) {
           telemetry[histogramId] = value[histogramId];
@@ -718,8 +803,8 @@ var MessageQueue = {
 
     try {
       // Send all data to the parent process.
-      sendAsyncMessage("SessionStore:update", {
-        data, telemetry, flushID,
+      sendMessage("SessionStore:update", {
+        id: this._id, data, telemetry, flushID,
         isFinal: options.isFinal || false,
         epoch: gCurrentEpoch
       });
@@ -727,16 +812,39 @@ var MessageQueue = {
       let telemetry = {
         FX_SESSION_RESTORE_SEND_UPDATE_CAUSED_OOM: 1
       };
-      sendAsyncMessage("SessionStore:error", {
+      sendMessage("SessionStore:error", {
         telemetry
       });
     }
+
+    // Increase our unique message ID.
+    this._id++;
   },
+
+  /**
+   * This function is used to make the message queue flush all queue data that
+   * hasn't been sent to the parent process, yet.
+   *
+   * @param id (int)
+   *        A unique id that represents the latest message received by the
+   *        chrome process. We can use this to determine which messages have not
+   *        yet been received because they are still stuck in the event queue.
+   */
+  flush: function (id) {
+    // It's important to always send data, even if there is nothing to flush.
+    // The update message will be received by the parent process that can then
+    // update its last received update ID to ignore stale messages.
+    this.send({id: id + 1, sync: true});
+
+    this._data.clear();
+    this._lastUpdated.clear();
+  }
 };
 
 EventListener.init();
 MessageListener.init();
 FormDataListener.init();
+SyncHandler.init();
 PageStyleListener.init();
 SessionHistoryListener.init();
 SessionStorageListener.init();

browser/components/sessionstore/test/browser_broadcast.js

@@ -74,7 +74,7 @@ add_task(function flush_on_settabstate() {
 
   // Flush all data contained in the content script but send it using
   // asynchronous messages.
-  TabStateFlusher.flush(browser);
+  TabState.flushAsync(browser);
 
   yield promiseTabState(tab, state);
 
@@ -101,7 +101,7 @@ add_task(function flush_on_tabclose_racy() {
 
   // Flush all data contained in the content script but send it using
   // asynchronous messages.
-  TabStateFlusher.flush(browser);
+  TabState.flushAsync(browser);
   yield promiseRemoveTab(tab);
 
   let [{state: {storage}}] = JSON.parse(ss.getClosedTabData(window));