Bug 1677697 - Fix storage actors being instanced twice r=ochameau
authorBelén Albeza <balbeza@mozilla.com>
Tue, 17 Nov 2020 15:53:28 +0000
changeset 557790 2f08ec7e57c3b7968ce11a729bb563257173b70e
parent 557789 a0ff2113d5b1ebfd026ff7bfc393191928f7eebe
child 557791 6cde833264983df1d8bb92f4051cf8ece18ec221
push id37961
push userccoroiu@mozilla.com
push dateWed, 18 Nov 2020 16:05:35 +0000
treeherdermozilla-central@2f08ec7e57c3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1677697
milestone85.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 1677697 - Fix storage actors being instanced twice r=ochameau This avoids instancing twice the different storage type actors (legacy and resources). In order to keep current server tests working, a pref to force instancing legacy actors has been introduced. Differential Revision: https://phabricator.services.mozilla.com/D97287
devtools/server/actors/storage.js
devtools/server/tests/browser/browser_storage_updates.js
devtools/server/tests/browser/storage-helpers.js
modules/libpref/init/all.js
--- a/devtools/server/actors/storage.js
+++ b/devtools/server/actors/storage.js
@@ -3398,23 +3398,41 @@ const StorageActor = protocol.ActorClass
     this.parentActor = targetActor;
 
     this.childActorPool = new Map();
     this.childWindowPool = new Set();
 
     // Fetch all the inner iframe windows in this tab.
     this.fetchChildWindows(this.parentActor.docShell);
 
+    // Skip initializing storage actors already instanced as Resources
+    // by the watcher. This happens when the target is a tab.
+    const isAddonTarget = !!this.parentActor.addonId;
+    const isWatcherEnabled = !isAddonTarget && !this.parentActor.isRootActor;
+    const shallUseLegacyActors = Services.prefs.getBoolPref(
+      "devtools.storage.test.forceLegacyActors",
+      false
+    );
+    const resourcesInWatcher = {
+      localStorage: isWatcherEnabled,
+      sessionStorage: isWatcherEnabled,
+    };
+
     // Initialize the registered store types
     for (const [store, ActorConstructor] of storageTypePool) {
       // Only create the extensionStorage actor when the debugging target
       // is an extension.
-      if (store === "extensionStorage" && !this.parentActor.addonId) {
+      if (store === "extensionStorage" && !isAddonTarget) {
         continue;
       }
+      // Skip resource actors
+      if (resourcesInWatcher[store] && !shallUseLegacyActors) {
+        continue;
+      }
+
       this.childActorPool.set(store, new ActorConstructor(this));
     }
 
     // Notifications that help us keep track of newly added windows and windows
     // that got removed
     Services.obs.addObserver(this, "content-document-global-created");
     Services.obs.addObserver(this, "inner-window-destroyed");
     this.onPageChange = this.onPageChange.bind(this);
--- a/devtools/server/tests/browser/browser_storage_updates.js
+++ b/devtools/server/tests/browser/browser_storage_updates.js
@@ -1,16 +1,22 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // Ensure that storage updates are detected and that the correct information is
 // contained inside the storage actors.
 
 "use strict";
 
+/* import-globals-from storage-helpers.js */
+Services.scriptloader.loadSubScript(
+  "chrome://mochitests/content/browser/devtools/server/tests/browser/storage-helpers.js",
+  this
+);
+
 const TESTS = [
   // index 0
   {
     action: async function(win) {
       await addCookie("c1", "foobar1");
       await addCookie("c2", "foobar2");
       await localStorageSetItem("l1", "foobar1");
     },
@@ -185,18 +191,19 @@ const TESTS = [
       cookies: [],
       localStorage: [],
       sessionStorage: [],
     },
   },
 ];
 
 add_task(async function() {
-  const target = await addTabTarget(MAIN_DOMAIN + "storage-updates.html");
-  const front = await target.getFront("storage");
+  const { target, front } = await openTabAndSetupStorage(
+    MAIN_DOMAIN + "storage-updates.html"
+  );
 
   for (let i = 0; i < TESTS.length; i++) {
     const test = TESTS[i];
     await runTest(test, front, i);
   }
 
   await finishTests(target);
 });
--- a/devtools/server/tests/browser/storage-helpers.js
+++ b/devtools/server/tests/browser/storage-helpers.js
@@ -3,25 +3,32 @@
 
 // This file assumes head.js is loaded in the global scope.
 /* import-globals-from head.js */
 
 /* exported openTabAndSetupStorage, clearStorage */
 
 "use strict";
 
+const LEGACY_ACTORS_PREF = "devtools.storage.test.forceLegacyActors";
+
 /**
  * This generator function opens the given url in a new tab, then sets up the
  * page by waiting for all cookies, indexedDB items etc. to be created.
  *
  * @param url {String} The url to be opened in the new tab
  *
  * @return {Promise} A promise that resolves after storage inspector is ready
  */
 async function openTabAndSetupStorage(url) {
+  // Enable testing prefs
+  SpecialPowers.pushPrefEnv({
+    set: [[LEGACY_ACTORS_PREF, true]],
+  });
+
   const content = await addTab(url);
 
   // Setup the async storages in main window and for all its iframes
   await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() {
     /**
      * Get all windows including frames recursively.
      *
      * @param {Window} [baseWindow]
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -885,16 +885,20 @@ pref("devtools.performance.recording.obj
 pref("devtools.performance.recording.objdirs.remote", "[]");
 // The popup will display some introductory text the first time it is displayed.
 pref("devtools.performance.popup.intro-displayed", false);
 
 // Compatibility preferences
 // Stringified array of target browsers that users investigate.
 pref("devtools.inspector.compatibility.target-browsers", "");
 
+// Storage preferencex
+// Force instancing legacy storage actors
+pref("devtools.storage.test.forceLegacyActors", false);
+
 // view source
 pref("view_source.editor.path", "");
 // allows to add further arguments to the editor; use the %LINE% placeholder
 // for jumping to a specific line (e.g. "/line:%LINE%" or "--goto %LINE%")
 pref("view_source.editor.args", "");
 
 // whether or not to draw images while dragging
 pref("nglayout.enable_drag_images", true);