Bug 1667839 - Pass watchedData to already existing targets. r=nchevobbe

The previous code was all wrong and we weren't correctly listening to resource
for already existing additional targets. Top level target was still working fine
thanks to WatcherActor.watchResources calling watchTargetResource directly.

Differential Revision: https://phabricator.services.mozilla.com/D91969

devtools/server/actors/resources/index.js

@@ -188,7 +188,7 @@ exports.hasResourceTypesForTargets = hasResourceTypesForTargets;
  * Stop watching for a list of resource types.
  *
  * @param Actor watcherOrTargetActor
- *        The related actor, already passed to watchTargetResources
+ *        The related actor, already passed to watchResources.
  * @param Array<String> resourceTypes
  *        List of all type of resource to stop listening to.
  */

devtools/server/actors/targets/browsing-context.js

@@ -308,13 +308,13 @@ const browsingContextTargetPrototype = {
 
   addWatcherDataEntry(type, entries) {
     if (type == "resources") {
-      this.watchTargetResources(entries);
+      this._watchTargetResources(entries);
     }
   },
 
   removeWatcherDataEntry(type, entries) {
     if (type == "resources") {
-      this.unwatchTargetResources(entries);
+      this._unwatchTargetResources(entries);
     }
   },
 
@@ -326,11 +326,11 @@ const browsingContextTargetPrototype = {
    * We have these shortcut methods in this module, because this is called from DevToolsFrameChild
    * which is a JSM and doesn't have a reference to a DevTools Loader.
    */
-  watchTargetResources(resourceTypes) {
+  _watchTargetResources(resourceTypes) {
     return Resources.watchResources(this, resourceTypes);
   },
 
-  unwatchTargetResources(resourceTypes) {
+  _unwatchTargetResources(resourceTypes) {
     return Resources.unwatchResources(this, resourceTypes);
   },
 

devtools/server/actors/watcher.js

@@ -77,8 +77,8 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
    * @return Array<String>
    *         Returns the list of currently watched resource types.
    */
-  get watchedResources() {
-    return WatcherRegistry.getWatchedResources(this);
+  get watchedData() {
+    return WatcherRegistry.getWatchedData(this);
   },
 
   form() {
@@ -140,10 +140,9 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
   async watchTargets(targetType) {
     WatcherRegistry.watchTargets(this, targetType);
 
-    const watchedResources = WatcherRegistry.getWatchedResources(this);
     const targetHelperModule = TARGET_HELPERS[targetType];
     // Await the registration in order to ensure receiving the already existing targets
-    await targetHelperModule.createTargets(this, watchedResources);
+    await targetHelperModule.createTargets(this);
   },
 
   /**
@@ -311,7 +310,7 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
       ? TargetActorRegistry.getTargetActor(this.browserId)
       : TargetActorRegistry.getParentProcessTargetActor();
     if (targetActor) {
-      await targetActor.watchTargetResources(frameResourceTypes);
+      await targetActor.addWatcherDataEntry("resources", frameResourceTypes);
     }
   },
 
@@ -378,7 +377,7 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
       ? TargetActorRegistry.getTargetActor(this.browserId)
       : TargetActorRegistry.getParentProcessTargetActor();
     if (targetActor) {
-      targetActor.unwatchTargetResources(frameResourceTypes);
+      targetActor.removeWatcherDataEntry("resources", frameResourceTypes);
     }
 
     // Unregister the JS Window Actor if there is no more DevTools code observing any target/resource

devtools/server/actors/watcher/WatcherRegistry.jsm

@@ -62,45 +62,6 @@ const SUPPORTED_DATA = {
   RESOURCES: "resources",
 };
 
-/**
- * Retrieve the data saved into `sharedData` that is used to know
- * about which type of targets and resources we care listening about.
- * `watchedDataByWatcherActor` is saved into `sharedData` after each mutation,
- * but `watchedDataByWatcherActor` is the source of truth.
- *
- * @param WatcherActor watcher
- *               The related WatcherActor which starts/stops observing.
- * @param object options (optional)
- *               A dictionary object with `createData` boolean attribute.
- *               If this attribute is set to true, we create the data structure in the Map
- *               if none exists for this prefix.
- */
-function getWatchedData(watcher, { createData = false } = {}) {
-  // Use WatcherActor ID as a key as we may have multiple clients willing to watch for targets.
-  // For example, a Browser Toolbox debugging everything and a Content Toolbox debugging
-  // just one tab. We might also have multiple watchers, on the same connection when using about:debugging.
-  const watcherActorID = watcher.actorID;
-  let watchedData = watchedDataByWatcherActor.get(watcherActorID);
-  if (!watchedData && createData) {
-    watchedData = {
-      // The Browser ID will be helpful to identify which BrowsingContext should be considered
-      // when running code in the content process. Browser ID, compared to BrowsingContext ID won't change
-      // if we navigate to the parent process or if a new BrowsingContext is used for the <browser> element
-      // we are currently inspecting.
-      browserId: watcher.browserId,
-      // The DevToolsServerConnection prefix will be used to compute actor IDs created in the content process
-      connectionPrefix: watcher.conn.prefix,
-    };
-    // Define empty default array for all data
-    for (const name of Object.values(SUPPORTED_DATA)) {
-      watchedData[name] = [];
-    }
-    watchedDataByWatcherActor.set(watcherActorID, watchedData);
-    watcherActors.set(watcherActorID, watcher);
-  }
-  return watchedData;
-}
-
 /**
  * Use `sharedData` to allow processes, early during their creation,
  * to know which resources should be listened to. This will be read
@@ -127,22 +88,47 @@ const WatcherRegistry = {
    *         Returns true if already watching.
    */
   isWatchingTargets(watcher, targetType) {
-    const watchedData = getWatchedData(watcher);
+    const watchedData = this.getWatchedData(watcher);
     return watchedData && watchedData.targets.includes(targetType);
   },
 
   /**
-   * Get currently watched resources for a given watcher.
+   * Retrieve the data saved into `sharedData` that is used to know
+   * about which type of targets and resources we care listening about.
+   * `watchedDataByWatcherActor` is saved into `sharedData` after each mutation,
+   * but `watchedDataByWatcherActor` is the source of truth.
    *
-   * @return Array<String>
-   *         Returns the list of currently watched resource types.
+   * @param WatcherActor watcher
+   *               The related WatcherActor which starts/stops observing.
+   * @param object options (optional)
+   *               A dictionary object with `createData` boolean attribute.
+   *               If this attribute is set to true, we create the data structure in the Map
+   *               if none exists for this prefix.
    */
-  getWatchedResources(watcher) {
-    const watchedData = getWatchedData(watcher);
-    if (watchedData) {
-      return watchedData.resources;
+  getWatchedData(watcher, { createData = false } = {}) {
+    // Use WatcherActor ID as a key as we may have multiple clients willing to watch for targets.
+    // For example, a Browser Toolbox debugging everything and a Content Toolbox debugging
+    // just one tab. We might also have multiple watchers, on the same connection when using about:debugging.
+    const watcherActorID = watcher.actorID;
+    let watchedData = watchedDataByWatcherActor.get(watcherActorID);
+    if (!watchedData && createData) {
+      watchedData = {
+        // The Browser ID will be helpful to identify which BrowsingContext should be considered
+        // when running code in the content process. Browser ID, compared to BrowsingContext ID won't change
+        // if we navigate to the parent process or if a new BrowsingContext is used for the <browser> element
+        // we are currently inspecting.
+        browserId: watcher.browserId,
+        // The DevToolsServerConnection prefix will be used to compute actor IDs created in the content process
+        connectionPrefix: watcher.conn.prefix,
+      };
+      // Define empty default array for all data
+      for (const name of Object.values(SUPPORTED_DATA)) {
+        watchedData[name] = [];
+      }
+      watchedDataByWatcherActor.set(watcherActorID, watchedData);
+      watcherActors.set(watcherActorID, watcher);
     }
-    return [];
+    return watchedData;
   },
 
   /**
@@ -168,7 +154,7 @@ const WatcherRegistry = {
    *               The values to be added to this type of data
    */
   addWatcherDataEntry(watcher, type, entries) {
-    const watchedData = getWatchedData(watcher, {
+    const watchedData = this.getWatchedData(watcher, {
       createData: true,
     });
 
@@ -203,7 +189,7 @@ const WatcherRegistry = {
    *         True if we such entry was already registered, for this watcher actor.
    */
   removeWatcherDataEntry(watcher, type, entries) {
-    const watchedData = getWatchedData(watcher);
+    const watchedData = this.getWatchedData(watcher);
     if (!watchedData) {
       return false;
     }

devtools/server/actors/watcher/target-helpers/frame-helper.js

@@ -18,10 +18,8 @@ const Targets = require("devtools/server/actors/targets/index");
  *
  * @param WatcherActor watcher
  *        The Watcher Actor requesting to watch for new targets.
- * @param Array<String> watchedResources
- *        List of currently watched resource types by this watcher actor.
  */
-async function createTargets(watcher, watchedResources) {
+async function createTargets(watcher) {
   // Go over all existing BrowsingContext in order to:
   // - Force the instantiation of a DevToolsFrameChild
   // - Have the DevToolsFrameChild to spawn the BrowsingContextTargetActor
@@ -43,7 +41,7 @@ async function createTargets(watcher, watchedResources) {
         watcherActorID: watcher.actorID,
         connectionPrefix: watcher.conn.prefix,
         browserId: watcher.browserId,
-        watchedResources: watcher.watchedResources,
+        watchedData: watcher.watchedData,
       });
     promises.push(promise);
   }

devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm

@@ -329,15 +329,11 @@ class DevToolsFrameChild extends JSWindowActorChild {
     }
     switch (message.name) {
       case "DevToolsFrameParent:instantiate-already-available": {
-        const {
-          watcherActorID,
-          connectionPrefix,
-          watchedResources,
-        } = message.data;
+        const { watcherActorID, connectionPrefix, watchedData } = message.data;
         return this._createTargetActor(
           watcherActorID,
           connectionPrefix,
-          watchedResources
+          watchedData
         );
       }
       case "DevToolsFrameParent:destroy": {

devtools/server/connectors/js-window-actor/DevToolsFrameParent.jsm

@@ -58,13 +58,13 @@ class DevToolsFrameParent extends JSWindowActorParent {
     watcherActorID,
     connectionPrefix,
     browserId,
-    watchedResources,
+    watchedData,
   }) {
     return this.sendQuery("DevToolsFrameParent:instantiate-already-available", {
       watcherActorID,
       connectionPrefix,
       browserId,
-      watchedResources,
+      watchedData,
     });
   }
 

devtools/shared/resources/resource-watcher.js

@@ -254,7 +254,7 @@ class ResourceWatcher {
       // to a new process.
       // In order to keep working resources that are being watched via the
       // Watcher actor, we have to unregister and re-register the resource
-      // types. This will force calling `watchTargetResources` on the new top
+      // types. This will force calling `Resources.watchResources` on the new top
       // level target.
       for (const resourceType of this._listenerCount.keys()) {
         await this._stopListening(resourceType, { bypassListenerCount: true });