Bug 1216250 - Limit amount of DOM Storage data stored by Session Restore. r=ttaubert
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Tue, 20 Oct 2015 14:15:17 +0200
changeset 305423 bb09cf018056889554a4b2cfa7a629a043e4c49c
parent 305277 ff8ebe5251ec227be26994089871cb40c1da3011
child 305424 1dd1ef5e115039a54fcf30c2571768711bc3e6d2
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttaubert
bugs1216250, 1106264
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1216250 - Limit amount of DOM Storage data stored by Session Restore. r=ttaubert 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");
@@ -34,16 +37,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;
 
@@ -518,19 +524,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 () {
@@ -660,38 +715,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;
 
     try {
       // 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
@@ -4353,16 +4353,24 @@
     "description": "Session restore: Time spent blocking the main thread while restoring a window state (ms)"
   },
   "FX_SESSION_RESTORE_SEND_UPDATE_CAUSED_OOM": {
     "alert_emails": ["session-restore-telemetry-alerts@mozilla.com"],
     "expires_in_version": "default",
     "kind": "count",
     "description": "Count of messages sent by SessionRestore from child frames to the parent and that cannot be transmitted as they eat up too much memory."
   },
+  "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)"
   },