Bug 1216250 - Limit amount of DOM Storage data stored by Session Restore. r=ttaubert, a=ritu
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Tue, 20 Oct 2015 14:15:17 +0200
changeset 305664 17f3d2ac13a4125e89cf2c73ccab2f3c78986b00
parent 305663 b204be8f7416ba66a20a0769799383a86e2d1eb3
child 305665 089460329f5f567b56bedd612e6b2d9c3ece0404
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert, ritu
bugs1216250, 1106264
milestone44.0a2
Bug 1216250 - Limit amount of DOM Storage data stored by Session Restore. r=ttaubert, a=ritu DOM Storage is a pretty inefficient and memory-hungry storage mechanism. Session Store attempts to record DOM Storage for each tab, which leads to (possibly very large) objects being serialized once to be sent from frame/content to parent and once to be sent from the main thread to the I/O thread. This is a suspect behind a number of crashes (see bug 1106264 for a discussion on the topic). This patch limits the amount of DOM Storage that Session Restore attempts to store. We perform a quick estimate on the amount of memory needed to serialize DOM Storage and prevent storage larger than ~10M chars being sent from frame/content to the parent. Once this patch has landed, we will need to watch FX_SESSION_RESTORE_DOM_STORAGE_SIZE_ESTIMATE_CHARS to find out whether our threshold is meaningful.
browser/components/sessionstore/SessionStorage.jsm
browser/components/sessionstore/content/content-sessionStore.js
browser/components/sessionstore/test/browser_sessionStorage.html
browser/components/sessionstore/test/browser_sessionStorage.js
toolkit/components/telemetry/Histograms.json
--- a/browser/components/sessionstore/SessionStorage.jsm
+++ b/browser/components/sessionstore/SessionStorage.jsm
@@ -25,47 +25,47 @@ function getPrincipalForFrame(docShell, 
 this.SessionStorage = Object.freeze({
   /**
    * Updates all sessionStorage "super cookies"
    * @param docShell
    *        That tab's docshell (containing the sessionStorage)
    * @param frameTree
    *        The docShell's FrameTree instance.
    * @return Returns a nested object that will have hosts as keys and per-host
-   *         session storage data as values. For example:
-   *         {"example.com": {"key": "value", "my_number": 123}}
+   *         session storage data as strings. For example:
+   *         {"example.com": {"key": "value", "my_number": "123"}}
    */
   collect: function (docShell, frameTree) {
     return SessionStorageInternal.collect(docShell, frameTree);
   },
 
   /**
    * Restores all sessionStorage "super cookies".
    * @param aDocShell
    *        A tab's docshell (containing the sessionStorage)
    * @param aStorageData
    *        A nested object with storage data to be restored that has hosts as
-   *        keys and per-host session storage data as values. For example:
-   *        {"example.com": {"key": "value", "my_number": 123}}
+   *        keys and per-host session storage data as strings. For example:
+   *        {"example.com": {"key": "value", "my_number": "123"}}
    */
   restore: function (aDocShell, aStorageData) {
     SessionStorageInternal.restore(aDocShell, aStorageData);
-  }
+  },
 });
 
 var SessionStorageInternal = {
   /**
    * Reads all session storage data from the given docShell.
    * @param docShell
    *        A tab's docshell (containing the sessionStorage)
    * @param frameTree
    *        The docShell's FrameTree instance.
    * @return Returns a nested object that will have hosts as keys and per-host
-   *         session storage data as values. For example:
-   *         {"example.com": {"key": "value", "my_number": 123}}
+   *         session storage data as strings. For example:
+   *         {"example.com": {"key": "value", "my_number": "123"}}
    */
   collect: function (docShell, frameTree) {
     let data = {};
     let visitedOrigins = new Set();
 
     frameTree.forEach(frame => {
       let principal = getPrincipalForFrame(docShell, frame);
       if (!principal) {
@@ -93,18 +93,18 @@ var SessionStorageInternal = {
   },
 
   /**
    * Writes session storage data to the given tab.
    * @param aDocShell
    *        A tab's docshell (containing the sessionStorage)
    * @param aStorageData
    *        A nested object with storage data to be restored that has hosts as
-   *        keys and per-host session storage data as values. For example:
-   *        {"example.com": {"key": "value", "my_number": 123}}
+   *        keys and per-host session storage data as strings. For example:
+   *        {"example.com": {"key": "value", "my_number": "123"}}
    */
   restore: function (aDocShell, aStorageData) {
     for (let origin of Object.keys(aStorageData)) {
       let data = aStorageData[origin];
       let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(origin);
       let storageManager = aDocShell.QueryInterface(Ci.nsIDOMStorageManager);
       let window = aDocShell.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindow);
 
--- a/browser/components/sessionstore/content/content-sessionStore.js
+++ b/browser/components/sessionstore/content/content-sessionStore.js
@@ -11,20 +11,23 @@ function debug(msg) {
 var Cu = Components.utils;
 var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cr = Components.results;
 
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/Timer.jsm", this);
 
+XPCOMUtils.defineLazyModuleGetter(this, "FormData",
+  "resource://gre/modules/FormData.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
+  "resource://gre/modules/Preferences.jsm");
+
 XPCOMUtils.defineLazyModuleGetter(this, "DocShellCapabilities",
   "resource:///modules/sessionstore/DocShellCapabilities.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "FormData",
-  "resource://gre/modules/FormData.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "PageStyle",
   "resource:///modules/sessionstore/PageStyle.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ScrollPosition",
   "resource://gre/modules/ScrollPosition.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionHistory",
   "resource:///modules/sessionstore/SessionHistory.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "SessionStorage",
   "resource:///modules/sessionstore/SessionStorage.jsm");
@@ -38,16 +41,19 @@ var gFrameTree = new FrameTree(this);
 
 Cu.import("resource:///modules/sessionstore/ContentRestore.jsm", this);
 XPCOMUtils.defineLazyGetter(this, 'gContentRestore',
                             () => { return new ContentRestore(this) });
 
 // The current epoch.
 var gCurrentEpoch = 0;
 
+// A bound to the size of data to store for DOM Storage.
+const DOM_STORAGE_MAX_CHARS = 10000000; // 10M characters
+
 /**
  * Returns a lazy function that will evaluate the given
  * function |fn| only once and cache its return value.
  */
 function createLazy(fn) {
   let cached = false;
   let cachedValue = null;
 
@@ -522,19 +528,68 @@ var SessionStorageListener = {
   },
 
   observe: function () {
     // Collect data on the next tick so that any other observer
     // that needs to purge data can do its work first.
     setTimeout(() => this.collect(), 0);
   },
 
+  // Before DOM Storage can be written to disk, it needs to be serialized
+  // for sending across frames/processes, then again to be sent across
+  // threads, then again to be put in a buffer for the disk. Each of these
+  // serializations is an opportunity to OOM and (depending on the site of
+  // the OOM), either crash, lose all data for the frame or lose all data
+  // for the application.
+  //
+  // In order to avoid this, compute an estimate of the size of the
+  // object, and block SessionStorage items that are too large. As
+  // we also don't want to cause an OOM here, we use a quick and memory-
+  // efficient approximation: we compute the total sum of string lengths
+  // involved in this object.
+  estimateStorageSize: function(collected) {
+    if (!collected) {
+      return 0;
+    }
+
+    let size = 0;
+    for (let host of Object.keys(collected)) {
+      size += host.length;
+      let perHost = collected[host];
+      for (let key of Object.keys(perHost)) {
+        size += key.length;
+        let perKey = perHost[key];
+        size += perKey.length;
+      }
+    }
+
+    return size;
+  },
+
   collect: function () {
     if (docShell) {
-      MessageQueue.push("storage", () => SessionStorage.collect(docShell, gFrameTree));
+      MessageQueue.push("storage", () => {
+        let collected = SessionStorage.collect(docShell, gFrameTree);
+
+        if (collected == null) {
+          return collected;
+        }
+
+        let size = this.estimateStorageSize(collected);
+
+        MessageQueue.push("telemetry", () => ({ FX_SESSION_RESTORE_DOM_STORAGE_SIZE_ESTIMATE_CHARS: size }));
+        if (size > Preferences.get("browser.sessionstore.dom_storage_limit", DOM_STORAGE_MAX_CHARS)) {
+          // Rather than keeping the old storage, which wouldn't match the rest
+          // of the state of the page, empty the storage. DOM storage will be
+          // recollected the next time and stored if it is now small enough.
+          return {};
+        }
+
+        return collected;
+      });
     }
   },
 
   onFrameTreeCollected: function () {
     this.collect();
   },
 
   onFrameTreeReset: function () {
@@ -664,38 +719,44 @@ var MessageQueue = {
     // 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, 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;
       }
 
-      data[key] = this._data.get(key)();
+      let value = this._data.get(key)();
+      if (key == "telemetry") {
+        for (let histogramId of Object.keys(value)) {
+          telemetry[histogramId] = value[histogramId];
+        }
+      } else {
+        data[key] = value;
+      }
     }
 
     durationMs = Date.now() - durationMs;
-    let telemetry = {
-      FX_SESSION_RESTORE_CONTENT_COLLECT_DATA_LONGEST_OP_MS: durationMs
-    }
+    telemetry.FX_SESSION_RESTORE_CONTENT_COLLECT_DATA_LONGEST_OP_MS = durationMs;
 
     // Send all data to the parent process.
     sendMessage("SessionStore:update", {
       id: this._id, data, telemetry, flushID,
       isFinal: options.isFinal || false,
       epoch: gCurrentEpoch
     });
 
--- a/browser/components/sessionstore/test/browser_sessionStorage.html
+++ b/browser/components/sessionstore/test/browser_sessionStorage.html
@@ -15,12 +15,13 @@
         let isSecure = args.indexOf("secure") > -1;
         let scheme = isSecure ? "https" : "http";
         iframe.setAttribute("src", scheme + "://example.com" + location.pathname + "?" + rand);
         document.body.appendChild(iframe);
       }
 
       if (sessionStorage.length === 0) {
         sessionStorage.test = (isOuter ? "outer" : "inner") + "-value-" + rand;
+        document.title = sessionStorage.test;
       }
     </script>
   </body>
 </html>
--- a/browser/components/sessionstore/test/browser_sessionStorage.js
+++ b/browser/components/sessionstore/test/browser_sessionStorage.js
@@ -178,11 +178,51 @@ add_task(function respect_privacy_level(
   // With privacy_level=0 the duplicated |tab2| should persist all data.
   [{state: {storage}}] = JSON.parse(ss.getClosedTabData(window));
   is(storage["http://mochi.test:8888"].test, OUTER_VALUE,
     "http sessionStorage data has been saved");
   is(storage["https://example.com"].test, INNER_VALUE,
     "https sessionStorage data has been saved");
 });
 
+// Test that we record the size of messages.
+add_task(function* test_telemetry() {
+  Services.telemetry.canRecordExtended = true;
+  let histogram = Services.telemetry.getHistogramById("FX_SESSION_RESTORE_DOM_STORAGE_SIZE_ESTIMATE_CHARS");
+  let snap1 = histogram.snapshot();
+
+  let tab = gBrowser.addTab(URL);
+  let browser = tab.linkedBrowser;
+  yield promiseBrowserLoaded(browser);
+
+  // Flush to make sure chrome received all data.
+  yield TabStateFlusher.flush(browser);
+  let snap2 = histogram.snapshot();
+
+  Assert.ok(snap2.counts[5] > snap1.counts[5]);
+  yield promiseRemoveTab(tab);
+  Services.telemetry.canRecordExtended = false;
+});
+
+// Lower the size limit for DOM Storage content. Check that DOM Storage
+// is not updated, but that other things remain updated.
+add_task(function* test_large_content() {
+  Services.prefs.setIntPref("browser.sessionstore.dom_storage_limit", 5);
+
+  let tab = gBrowser.addTab(URL);
+  let browser = tab.linkedBrowser;
+  yield promiseBrowserLoaded(browser);
+
+  // Flush to make sure chrome received all data.
+  yield TabStateFlusher.flush(browser);
+
+  let state = JSON.parse(ss.getTabState(tab));
+  info(JSON.stringify(state, null, "\t"));
+  Assert.equal(state.storage, null, "We have no storage for the tab");
+  Assert.equal(state.entries[0].title, OUTER_VALUE);
+  yield promiseRemoveTab(tab);
+
+  Services.prefs.clearUserPref("browser.sessionstore.dom_storage_limit");
+});
+
 function purgeDomainData(browser, domain) {
   return sendMessage(browser, "ss-test:purgeDomainData", domain);
 }
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -4354,16 +4354,24 @@
     "alert_emails": ["session-restore-telemetry-alerts@mozilla.com"],
     "expires_in_version": "default",
     "kind": "exponential",
     "high": "3000",
     "n_buckets": 10,
     "extended_statistics_ok": true,
     "description": "Session restore: Time spent blocking the main thread while restoring a window state (ms)"
   },
+  "FX_SESSION_RESTORE_DOM_STORAGE_SIZE_ESTIMATE_CHARS": {
+    "expires_in_version": "default",
+    "kind": "exponential",
+    "high": "30000000",
+    "n_buckets": 20,
+    "extended_statistics_ok": true,
+    "description": "Session restore: Number of characters in DOM Storage for a tab. Pages without DOM Storage or with an empty DOM Storage are ignored."
+  },
   "FX_TABLETMODE_PAGE_LOAD": {
     "expires_in_version": "47",
     "kind": "exponential",
     "high": 100000,
     "n_buckets": 30,
     "keyed": true,
     "description": "Number of toplevel location changes in tablet and desktop mode (only used on win10 where tablet mode is available)"
   },