Bug 951675 - Make sure to always copy data from the persistent cache r=yoric

From bbe5effc34b6e81e57750ec05164e7e10de75d7a Mon Sep 17 00:00:00 2001

browser/components/sessionstore/src/TabState.jsm

@@ -182,9 +182,6 @@ let TabStateInternal = {
         tabData.index = history.index;
       }
 
-      // Copy data from the persistent cache.
-      this._copyFromPersistentCache(tab, tabData);
-
       // If we're still the latest async collection for the given tab and
       // the cache hasn't been filled by collect() in the meantime, let's
       // fill the cache with the data we received.
@@ -193,6 +190,16 @@ let TabStateInternal = {
         this._pendingCollections.delete(browser);
       }
 
+      // Copy data from the persistent cache. We need to create an explicit
+      // copy of the |tabData| object so that the properties injected by
+      // |_copyFromPersistentCache| don't end up in the non-persistent cache.
+      // The persistent cache does not store "null" values, so any values that
+      // have been cleared by the frame script would not be overriden by
+      // |_copyFromPersistentCache|. These two caches are only an interim
+      // solution and the non-persistent one will go away soon.
+      tabData = Utils.copy(tabData);
+      this._copyFromPersistentCache(tab, tabData);
+
       throw new Task.Result(tabData);
     }.bind(this));
 
@@ -219,7 +226,16 @@ let TabStateInternal = {
       throw new TypeError("Expecting a tab");
     }
     if (TabStateCache.has(tab)) {
-      return TabStateCache.get(tab);
+      // Copy data from the persistent cache. We need to create an explicit
+      // copy of the |tabData| object so that the properties injected by
+      // |_copyFromPersistentCache| don't end up in the non-persistent cache.
+      // The persistent cache does not store "null" values, so any values that
+      // have been cleared by the frame script would not be overriden by
+      // |_copyFromPersistentCache|. These two caches are only an interim
+      // solution and the non-persistent one will go away soon.
+      let tabData = Utils.copy(TabStateCache.get(tab));
+      this._copyFromPersistentCache(tab, tabData);
+      return tabData;
     }
 
     let tabData = this._collectSyncUncached(tab);
@@ -228,6 +244,16 @@ let TabStateInternal = {
       TabStateCache.set(tab, tabData);
     }
 
+    // Copy data from the persistent cache. We need to create an explicit
+    // copy of the |tabData| object so that the properties injected by
+    // |_copyFromPersistentCache| don't end up in the non-persistent cache.
+    // The persistent cache does not store "null" values, so any values that
+    // have been cleared by the frame script would not be overriden by
+    // |_copyFromPersistentCache|. These two caches are only an interim
+    // solution and the non-persistent one will go away soon.
+    tabData = Utils.copy(tabData);
+    this._copyFromPersistentCache(tab, tabData);
+
     // Prevent all running asynchronous collections from filling the cache.
     // Every asynchronous data collection started before a collectSync() call
     // can't expect to retrieve different data than the sync call. That's why
@@ -262,7 +288,13 @@ let TabStateInternal = {
    *                   up-to-date.
    */
   clone: function (tab) {
-    return this._collectSyncUncached(tab, {includePrivateData: true});
+    let options = {includePrivateData: true};
+    let tabData = this._collectSyncUncached(tab, options);
+
+    // Copy data from the persistent cache.
+    this._copyFromPersistentCache(tab, tabData, options);
+
+    return tabData;
   },
 
   /**
@@ -305,9 +337,6 @@ let TabStateInternal = {
       tabData.index = history.index;
     }
 
-    // Copy data from the persistent cache.
-    this._copyFromPersistentCache(tab, tabData, options);
-
     return tabData;
   },
 

browser/components/sessionstore/src/Utils.jsm

@@ -64,5 +64,16 @@ this.Utils = Object.freeze({
       map.set(otherKey, value);
       map.delete(key);
     }
+  },
+
+  // Copies all properties of a given object to a new one and returns it.
+  copy: function (from) {
+    let to = {};
+
+    for (let key of Object.keys(from)) {
+      to[key] = from[key];
+    }
+
+    return to;
   }
 });