Bug 1700092 - [devtools] Add supported targets and resources to session context. r=bomsy,ochameau.
authorNicolas Chevobbe <nchevobbe@mozilla.com>
Thu, 19 May 2022 05:24:53 +0000
changeset 618194 ea32c4c1d987ddef9f831edbb1c4cb187930dc16
parent 618193 ee2ca87ef3d6d503a4a1ed79d501fc8cd29dd701
child 618195 aae4df69e66b79c0be40de234872386f5267b35f
push id39719
push usersmolnar@mozilla.com
push dateThu, 19 May 2022 16:03:14 +0000
treeherdermozilla-central@cc776278c4ea [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbomsy, ochameau
bugs1700092
milestone102.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 1700092 - [devtools] Add supported targets and resources to session context. r=bomsy,ochameau. This allows us to simplify and fix `hasStyleSheetWatcherSupportForTarget`. In the browser toolbox, since we're in a "all" session context type, `TargetActorRegistry.getTargetActors` would only return an array of a single element, the parent process target. In case of window global targets, in the browser toolbox context, we wouldn't find the target we're looking for, which was making `hasStyleSheetWatcherSupportForTarget` return false. We can now simply look into the targetActor session context. Depends on D146584 Differential Revision: https://phabricator.services.mozilla.com/D146193
devtools/server/actors/utils/stylesheets-manager.js
devtools/server/actors/watcher.js
devtools/server/actors/watcher/WatcherRegistry.jsm
devtools/server/actors/watcher/session-context.js
devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm
--- a/devtools/server/actors/utils/stylesheets-manager.js
+++ b/devtools/server/actors/utils/stylesheets-manager.js
@@ -1,17 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const EventEmitter = require("devtools/shared/event-emitter");
 const { Ci } = require("chrome");
-const Services = require("Services");
 const { fetch } = require("devtools/shared/DevToolsUtils");
 const InspectorUtils = require("InspectorUtils");
 const {
   getSourcemapBaseURL,
 } = require("devtools/server/actors/utils/source-map-utils");
 const { TYPES } = require("devtools/server/actors/resources/index");
 
 loader.lazyRequireGetter(
@@ -27,23 +26,16 @@ loader.lazyRequireGetter(
   true
 );
 loader.lazyRequireGetter(
   this,
   ["getSheetOwnerNode", "UPDATE_GENERAL", "UPDATE_PRESERVING_RULES"],
   "devtools/server/actors/style-sheet",
   true
 );
-loader.lazyRequireGetter(
-  this,
-  "TargetActorRegistry",
-  "devtools/server/actors/targets/target-actor-registry.jsm",
-  true
-);
-const SHARED_DATA_KEY_NAME = "DevTools:watchedPerWatcher";
 
 const TRANSITION_PSEUDO_CLASS = ":-moz-styleeditor-transitioning";
 const TRANSITION_DURATION_MS = 500;
 const TRANSITION_BUFFER_MS = 1000;
 const TRANSITION_RULE_SELECTOR = `:root${TRANSITION_PSEUDO_CLASS}, :root${TRANSITION_PSEUDO_CLASS} *`;
 const TRANSITION_SHEET =
   "data:text/css;charset=utf-8," +
   encodeURIComponent(`
@@ -877,37 +869,17 @@ class StyleSheetsManager extends EventEm
         ": ",
         e
       );
     }
   }
 }
 
 function hasStyleSheetWatcherSupportForTarget(targetActor) {
-  // Check if the watcher actor supports stylesheet resources.
-  // This is a temporary solution until we have a reliable way of propagating sessionData
-  // to all targets (so we'll be able to store this information via addDataEntry).
-  // This will be done in Bug 1700092.
-  const { sharedData } = Services.cpmm;
-  const sessionDataByWatcherActor = sharedData.get(SHARED_DATA_KEY_NAME);
-  if (!sessionDataByWatcherActor) {
-    return false;
-  }
-
-  const watcherSessionData = Array.from(
-    sessionDataByWatcherActor.values()
-  ).find(sessionData => {
-    const actors = TargetActorRegistry.getTargetActors(
-      sessionData.sessionContext,
-      sessionData.connectionPrefix
-    );
-    return actors.includes(targetActor);
-  });
-
   return (
-    watcherSessionData?.watcherTraits?.resources?.[TYPES.STYLESHEET] || false
+    targetActor.sessionContext.supportedResources?.[TYPES.STYLESHEET] || false
   );
 }
 
 module.exports = {
   StyleSheetsManager,
   hasStyleSheetWatcherSupportForTarget,
 };
--- a/devtools/server/actors/watcher.js
+++ b/devtools/server/actors/watcher.js
@@ -186,66 +186,23 @@ exports.WatcherActor = protocol.ActorCla
    * @return Array<String>
    *         Returns the list of currently watched resource types.
    */
   get sessionData() {
     return WatcherRegistry.getSessionData(this);
   },
 
   form() {
-    // All target types and all resources types are supported for tab debugging and web extensions.
-    // But worker target type and most watcher classes are still disabled for the browser toolbox (sessionContext.type=all).
-    // And they may also be disabled for workers once we start supporting them by the watcher.
-    //
-    // So keep the traits to false for all the resources that we don't support yet
-    // and keep using the legacy listeners.
-    const shouldEnableAllWatchers =
-      this.sessionContext.type == "browser-element" ||
-      this.sessionContext.type == "webextension";
-
     return {
       actor: this.actorID,
       // The resources and target traits should be removed all at the same time since the
       // client has generic ways to deal with all of them (See Bug 1680280).
       traits: {
-        [Targets.TYPES.FRAME]: true,
-        [Targets.TYPES.PROCESS]: true,
-        [Targets.TYPES.WORKER]: shouldEnableAllWatchers,
-        resources: {
-          // In Firefox 81 we added support for:
-          // - CONSOLE_MESSAGE
-          // - CSS_CHANGE
-          // - CSS_MESSAGE
-          // - DOCUMENT_EVENT
-          // - ERROR_MESSAGE
-          // - PLATFORM_MESSAGE
-          //
-          // We enabled them for content toolboxes only because we don't support
-          // content process targets yet. Bug 1620248 should help supporting
-          // them and enable this more broadly.
-          [Resources.TYPES.CONSOLE_MESSAGE]: true,
-          [Resources.TYPES.CSS_CHANGE]: shouldEnableAllWatchers,
-          [Resources.TYPES.CSS_MESSAGE]: true,
-          [Resources.TYPES.DOCUMENT_EVENT]: shouldEnableAllWatchers,
-          [Resources.TYPES.CACHE_STORAGE]: shouldEnableAllWatchers,
-          [Resources.TYPES.COOKIE]: shouldEnableAllWatchers,
-          [Resources.TYPES.ERROR_MESSAGE]: true,
-          [Resources.TYPES.INDEXED_DB]: shouldEnableAllWatchers,
-          [Resources.TYPES.LOCAL_STORAGE]: shouldEnableAllWatchers,
-          [Resources.TYPES.SESSION_STORAGE]: shouldEnableAllWatchers,
-          [Resources.TYPES.PLATFORM_MESSAGE]: true,
-          [Resources.TYPES.NETWORK_EVENT]: true,
-          [Resources.TYPES.NETWORK_EVENT_STACKTRACE]: true,
-          [Resources.TYPES.REFLOW]: true,
-          [Resources.TYPES.STYLESHEET]: shouldEnableAllWatchers,
-          [Resources.TYPES.SOURCE]: shouldEnableAllWatchers,
-          [Resources.TYPES.THREAD_STATE]: shouldEnableAllWatchers,
-          [Resources.TYPES.SERVER_SENT_EVENT]: shouldEnableAllWatchers,
-          [Resources.TYPES.WEBSOCKET]: shouldEnableAllWatchers,
-        },
+        ...this.sessionContext.supportedTargets,
+        resources: this.sessionContext.supportedResources,
       },
     };
   },
 
   /**
    * Start watching for a new target type.
    *
    * This will instantiate Target Actors for existing debugging context of this type,
--- a/devtools/server/actors/watcher/WatcherRegistry.jsm
+++ b/devtools/server/actors/watcher/WatcherRegistry.jsm
@@ -109,19 +109,16 @@ const WatcherRegistry = {
     let sessionData = sessionDataByWatcherActor.get(watcherActorID);
     if (!sessionData && createData) {
       sessionData = {
         // The "session context" object help understand what should be debugged and which target should be created.
         // See WatcherActor constructor for more info.
         sessionContext: watcher.sessionContext,
         // The DevToolsServerConnection prefix will be used to compute actor IDs created in the content process
         connectionPrefix: watcher.conn.prefix,
-        // Expose watcher traits so we can retrieve them in content process.
-        // This should be removed as part of Bug 1700092.
-        watcherTraits: watcher.form().traits,
       };
       // Define empty default array for all data
       for (const name of Object.values(SUPPORTED_DATA)) {
         sessionData[name] = [];
       }
       sessionDataByWatcherActor.set(watcherActorID, sessionData);
       watcherActors.set(watcherActorID, watcher);
     }
--- a/devtools/server/actors/watcher/session-context.js
+++ b/devtools/server/actors/watcher/session-context.js
@@ -8,24 +8,34 @@
 //
 // These are static JSON serializable object that help describe
 // the debugged context. It is passed around to most of the server codebase
 // in order to know which object to consider inspecting and communicating back to the client.
 //
 // These objects are all instantiated by the Descriptor actors
 // and passed as a constructor argument to the Watcher actor.
 //
-// These objects have only two attributes used by all the Session contexts:
+// These objects have attributes used by all the Session contexts:
 // - type: String
 //   Describes which type of context we are debugging.
 //   See SESSION_TYPES for all possible values.
 //   See each create* method for more info about each type and their specific attributes.
 // - isServerTargetSwitchingEnabled: Boolean
 //   If true, targets should all be spawned by the server codebase.
 //   Especially the first, top level target.
+// - supportedTargets: Boolean
+//   An object keyed by target type, whose value indicates if we have watcher support
+//   for the target.
+// - supportedResources: Boolean
+//   An object keyed by resource type, whose value indicates if we have watcher support
+//   for the resource.
+
+const Services = require("Services");
+const Targets = require("devtools/server/actors/targets/index");
+const Resources = require("devtools/server/actors/resources/index");
 
 const SESSION_TYPES = {
   ALL: "all",
   BROWSER_ELEMENT: "browser-element",
   CONTENT_PROCESS: "content-process",
   WEBEXTENSION: "webextension",
   WORKER: "worker",
 };
@@ -35,43 +45,49 @@ const SESSION_TYPES = {
  *
  * This context means debugging everything.
  * The whole browser:
  * - all processes: parent and content,
  * - all privileges: privileged/chrome and content/web,
  * - all components/targets: HTML documents, processes, workers, add-ons,...
  */
 function createBrowserSessionContext() {
+  const type = SESSION_TYPES.ALL;
   return {
-    type: SESSION_TYPES.ALL,
+    type,
     // For now, the top level target (ParentProcessTargetActor) is created via ProcessDescriptor.getTarget
     // and is never replaced by any other, nor is it created by the WatcherActor.
     isServerTargetSwitchingEnabled: false,
+    supportedTargets: getWatcherSupportedTargets(type),
+    supportedResources: getWatcherSupportedResources(type),
   };
 }
 
 /**
  * Create the SessionContext used by the regular web page toolboxes as well as remote debugging android device tabs.
  *
  * @param {BrowserElement} browserElement
  *        The tab to debug. It should be a reference to a <browser> element.
  * @param {Object} config
  *        An object with optional configuration. Only supports "isServerTargetSwitchingEnabled" attribute.
  *        See jsdoc in this file header for more info.
  */
 function createBrowserElementSessionContext(browserElement, config) {
+  const type = SESSION_TYPES.BROWSER_ELEMENT;
   return {
-    type: SESSION_TYPES.BROWSER_ELEMENT,
+    type,
     browserId: browserElement.browserId,
     // Nowaday, it should always be enabled except for WebExtension special
     // codepath and some tests.
     isServerTargetSwitchingEnabled: config.isServerTargetSwitchingEnabled,
     // Should we instantiate targets for popups opened in distinct tabs/windows?
     // Driven by devtools.popups.debug=true preference.
     isPopupDebuggingEnabled: config.isPopupDebuggingEnabled,
+    supportedTargets: getWatcherSupportedTargets(type),
+    supportedResources: getWatcherSupportedResources(type),
   };
 }
 
 /**
  * Create the SessionContext used by the web extension toolboxes.
  *
  * @param {Object} addon
  *        First object argument to describe the add-on.
@@ -88,44 +104,123 @@ function createBrowserElementSessionCont
  * @param {Object} config
  *        An object with optional configuration. Only supports "isServerTargetSwitchingEnabled" attribute.
  *        See jsdoc in this file header for more info.
  */
 function createWebExtensionSessionContext(
   { addonId, browsingContextID, innerWindowId },
   config
 ) {
+  const type = SESSION_TYPES.WEBEXTENSION;
   return {
-    type: SESSION_TYPES.WEBEXTENSION,
+    type,
     addonId: addonId,
     addonBrowsingContextID: browsingContextID,
     addonInnerWindowId: innerWindowId,
     // For now, there is only one target (WebExtensionTargetActor), it is never replaced,
     // and is only created via WebExtensionDescriptor.getTarget (and never by the watcher actor).
     isServerTargetSwitchingEnabled: config.isServerTargetSwitchingEnabled,
+    supportedTargets: getWatcherSupportedTargets(type),
+    supportedResources: getWatcherSupportedResources(type),
   };
 }
 
 /**
  * Create the SessionContext used by the Browser Content Toolbox, to debug only one content process.
  * Or when debugging XpcShell via about:debugging, where we instantiate only one content process target.
  */
 function createContentProcessSessionContext() {
+  const type = SESSION_TYPES.CONTENT_PROCESS;
   return {
-    type: SESSION_TYPES.CONTENT_PROCESS,
+    type,
+    supportedTargets: getWatcherSupportedTargets(type),
+    supportedResources: getWatcherSupportedResources(type),
   };
 }
 
 /**
  * Create the SessionContext used when debugging one specific Service Worker or special chrome worker.
  * This is only used from about:debugging.
  */
 function createWorkerSessionContext() {
+  const type = SESSION_TYPES.WORKER;
   return {
-    type: SESSION_TYPES.WORKER,
+    type,
+    supportedTargets: getWatcherSupportedTargets(type),
+    supportedResources: getWatcherSupportedResources(type),
+  };
+}
+
+/**
+ * Get the supported targets by the watcher given a session context type.
+ *
+ * @param {String} type
+ * @returns {Object}
+ */
+function getWatcherSupportedTargets(type) {
+  // We're completely bypassing the watcher for the non-multiprocess Browser Toolbox.
+  if (
+    type == SESSION_TYPES.ALL &&
+    !Services.prefs.getBoolPref("devtools.browsertoolbox.fission", false)
+  ) {
+    return {};
+  }
+
+  return {
+    [Targets.TYPES.FRAME]: true,
+    [Targets.TYPES.PROCESS]: true,
+    [Targets.TYPES.WORKER]:
+      type == SESSION_TYPES.BROWSER_ELEMENT ||
+      type == SESSION_TYPES.WEBEXTENSION,
+  };
+}
+
+/**
+ * Get the supported resources by the watcher given a session context type.
+ *
+ * @param {String} type
+ * @returns {Object}
+ */
+function getWatcherSupportedResources(type) {
+  // We're completely bypassing the watcher for the non-multiprocess Browser Toolbox.
+  if (
+    type == SESSION_TYPES.ALL &&
+    !Services.prefs.getBoolPref("devtools.browsertoolbox.fission", false)
+  ) {
+    return {};
+  }
+
+  // All resources types are supported for tab debugging and web extensions.
+  // Some watcher classes are still disabled for the Multiprocess Browser Toolbox (type=SESSION_TYPES.ALL).
+  // And they may also be disabled for workers once we start supporting them by the watcher.
+  // So set the traits to false for all the resources that we don't support yet
+  // and keep using the legacy listeners.
+  const isTabOrWebExtensionToolbox =
+    type == SESSION_TYPES.BROWSER_ELEMENT || type == SESSION_TYPES.WEBEXTENSION;
+
+  return {
+    [Resources.TYPES.CONSOLE_MESSAGE]: true,
+    [Resources.TYPES.CSS_CHANGE]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.CSS_MESSAGE]: true,
+    [Resources.TYPES.DOCUMENT_EVENT]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.CACHE_STORAGE]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.COOKIE]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.ERROR_MESSAGE]: true,
+    [Resources.TYPES.INDEXED_DB]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.LOCAL_STORAGE]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.SESSION_STORAGE]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.PLATFORM_MESSAGE]: true,
+    [Resources.TYPES.NETWORK_EVENT]: true,
+    [Resources.TYPES.NETWORK_EVENT_STACKTRACE]: true,
+    [Resources.TYPES.REFLOW]: true,
+    [Resources.TYPES.STYLESHEET]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.SOURCE]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.THREAD_STATE]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.SERVER_SENT_EVENT]: isTabOrWebExtensionToolbox,
+    [Resources.TYPES.WEBSOCKET]: isTabOrWebExtensionToolbox,
   };
 }
 
 module.exports = {
   createBrowserSessionContext,
   createBrowserElementSessionContext,
   createWebExtensionSessionContext,
   createContentProcessSessionContext,
--- a/devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm
+++ b/devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm
@@ -281,18 +281,18 @@ class DevToolsFrameChild extends JSWindo
     this.sendAsyncMessage("DevToolsFrameChild:connectFromContent", {
       watcherActorID,
       forwardingPrefix,
       actor: targetActor.form(),
     });
 
     // Pass initialization data to the target actor
     for (const type in sessionData) {
-      // `sessionData` will also contain `browserId` and `watcherTraits`,
-      // as well as entries with empty arrays, which shouldn't be processed.
+      // `sessionData` will also contain `browserId` as well as entries with empty arrays,
+      // which shouldn't be processed.
       const entries = sessionData[type];
       if (!Array.isArray(entries) || entries.length == 0) {
         continue;
       }
       targetActor.addSessionDataEntry(type, entries, isDocumentCreation);
     }
   }