Bug 1717802 - [devtools] Add support for event breakpoints via the watcher actor r=ochameau,devtools-backward-compat-reviewers,jdescottes
authorHubert Boma Manilla <hmanilla@mozilla.com>
Thu, 23 Sep 2021 08:53:57 +0000
changeset 593027 22900522693fd055fd0747b2ffd020df7aaea81d
parent 593026 be76cb91e85ae7748ed6d93f606b575f5ef48877
child 593028 38352a9dc127208982d92372c357e1a706fca246
push id38820
push usersmolnar@mozilla.com
push dateThu, 23 Sep 2021 21:45:25 +0000
treeherdermozilla-central@4eda9eb8926b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau, devtools-backward-compat-reviewers, jdescottes
bugs1717802
milestone94.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 1717802 - [devtools] Add support for event breakpoints via the watcher actor r=ochameau,devtools-backward-compat-reviewers,jdescottes Depends on D125039 Differential Revision: https://phabricator.services.mozilla.com/D124228
devtools/client/debugger/src/client/firefox/commands.js
devtools/server/actors/breakpoint-list.js
devtools/server/actors/targets/target-actor-mixin.js
devtools/server/actors/utils/event-breakpoints.js
devtools/server/actors/watcher.js
devtools/server/actors/watcher/WatchedDataHelpers.jsm
devtools/shared/specs/breakpoint-list.js
--- a/devtools/client/debugger/src/client/firefox/commands.js
+++ b/devtools/client/debugger/src/client/firefox/commands.js
@@ -329,20 +329,28 @@ async function blackBox(sourceActor, isB
 
 async function setSkipPausing(shouldSkip) {
   await commands.threadConfigurationCommand.updateConfiguration({
     skipBreakpoints: shouldSkip,
   });
 }
 
 async function setEventListenerBreakpoints(ids) {
-  return forEachThread(thread => thread.setActiveEventBreakpoints(ids));
+  // @backward-compat { version 94 } The `event-breakpoints` trait check is no longer needed, but
+  // keep the check for target watcher support to fallback for unsupported targets (e.g webextensions)
+  const hasWatcherSupport = commands.targetCommand.hasTargetWatcherSupport(
+    "event-breakpoints"
+  );
+  if (!hasWatcherSupport) {
+    return forEachThread(thread => thread.setActiveEventBreakpoints(ids));
+  }
+  const breakpointListFront = await commands.targetCommand.watcherFront.getBreakpointListActor();
+  await breakpointListFront.setActiveEventBreakpoints(ids);
 }
 
-// eslint-disable-next-line
 async function getEventListenerBreakpointTypes() {
   let categories;
   try {
     categories = await currentThreadFront().getAvailableEventBreakpoints();
 
     if (!Array.isArray(categories)) {
       // When connecting to older browser that had our placeholder
       // implementation of the 'getAvailableEventBreakpoints' endpoint, we
--- a/devtools/server/actors/breakpoint-list.js
+++ b/devtools/server/actors/breakpoint-list.js
@@ -5,17 +5,17 @@
 "use strict";
 
 const { ActorClassWithSpec, Actor } = require("devtools/shared/protocol");
 const { breakpointListSpec } = require("devtools/shared/specs/breakpoint-list");
 const {
   WatchedDataHelpers,
 } = require("devtools/server/actors/watcher/WatchedDataHelpers.jsm");
 const { SUPPORTED_DATA } = WatchedDataHelpers;
-const { BREAKPOINTS, XHR_BREAKPOINTS } = SUPPORTED_DATA;
+const { BREAKPOINTS, XHR_BREAKPOINTS, EVENT_BREAKPOINTS } = SUPPORTED_DATA;
 
 /**
  * This actor manages the breakpoints list.
  *
  * Breakpoints should be available as early as possible to new targets and
  * will be forwarded to the WatcherActor to populate the shared watcher data available to
  * all DevTools targets.
  *
@@ -62,11 +62,33 @@ const BreakpointListActor = ActorClassWi
    *
    * See setXHRBreakpoint for arguments definition.
    */
   removeXHRBreakpoint(path, method) {
     return this.watcherActor.removeDataEntry(XHR_BREAKPOINTS, [
       { path, method },
     ]);
   },
+
+  /**
+   * Set the active breakpoints
+   *
+   * @param {Array<String>} ids
+   *                        An array of eventlistener breakpoint ids. These
+   *                        are unique identifiers for event breakpoints.
+   *                        See devtools/server/actors/utils/event-breakpoints.js
+   *                        for details.
+   */
+  setActiveEventBreakpoints(ids) {
+    const existingIds = this.watcherActor.getWatchedData(EVENT_BREAKPOINTS);
+    const addIds = ids.filter(id => !existingIds.includes(id));
+    const removeIds = existingIds.filter(id => !ids.includes(id));
+
+    if (addIds.length) {
+      this.watcherActor.addDataEntry(EVENT_BREAKPOINTS, addIds);
+    }
+    if (removeIds.length) {
+      this.watcherActor.removeDataEntry(EVENT_BREAKPOINTS, removeIds);
+    }
+  },
 });
 
 exports.BreakpointListActor = BreakpointListActor;
--- a/devtools/server/actors/targets/target-actor-mixin.js
+++ b/devtools/server/actors/targets/target-actor-mixin.js
@@ -12,16 +12,17 @@ const {
 } = require("devtools/server/actors/watcher/WatchedDataHelpers.jsm");
 const { STATES: THREAD_STATES } = require("devtools/server/actors/thread");
 const {
   RESOURCES,
   BREAKPOINTS,
   TARGET_CONFIGURATION,
   THREAD_CONFIGURATION,
   XHR_BREAKPOINTS,
+  EVENT_BREAKPOINTS,
 } = WatchedDataHelpers.SUPPORTED_DATA;
 
 loader.lazyRequireGetter(
   this,
   "StyleSheetsManager",
   "devtools/server/actors/utils/stylesheets-manager",
   true
 );
@@ -40,16 +41,17 @@ module.exports = function(targetType, ta
      * @param string type
      *        The type of data to be added
      * @param Array<Object> entries
      *        The values to be added to this type of data
      * @param Boolean isDocumentCreation
      *        Set to true if this function is called just after a new document (and its
      *        associated target) is created.
      */
+    // eslint-disable-next-line complexity
     async addWatcherDataEntry(type, entries, isDocumentCreation = false) {
       if (type == RESOURCES) {
         await this._watchTargetResources(entries);
       } else if (type == BREAKPOINTS) {
         // Breakpoints require the target to be attached,
         // mostly to have the thread actor instantiated
         // (content process targets don't have attach method,
         //  instead they instantiate their ThreadActor immediately)
@@ -121,16 +123,30 @@ module.exports = function(targetType, ta
           await this.threadActor.attach();
         }
 
         await Promise.all(
           entries.map(({ path, method }) =>
             this.threadActor.setXHRBreakpoint(path, method)
           )
         );
+      } else if (type == EVENT_BREAKPOINTS) {
+        // Same as comments for XHR breakpoints. See lines 109-112
+        if (typeof this.attach == "function") {
+          this.attach();
+        }
+
+        // Same as comments for XHR breakpoints. See lines 117-118
+        if (
+          this.threadActor.state == THREAD_STATES.DETACHED &&
+          !this.targetType.endsWith("worker")
+        ) {
+          this.threadActor.attach();
+        }
+        await this.threadActor.setActiveEventBreakpoints(entries);
       }
     },
 
     removeWatcherDataEntry(type, entries) {
       if (type == RESOURCES) {
         return this._unwatchTargetResources(entries);
       } else if (type == BREAKPOINTS) {
         for (const { location } of entries) {
--- a/devtools/server/actors/utils/event-breakpoints.js
+++ b/devtools/server/actors/utils/event-breakpoints.js
@@ -468,8 +468,12 @@ function getAvailableEventBreakpoints() 
       events: items.map(item => ({
         id: item.id,
         name: item.name,
       })),
     });
   }
   return available;
 }
+exports.validateEventBreakpoint = validateEventBreakpoint;
+function validateEventBreakpoint(id) {
+  return !!EVENTS_BY_ID[id];
+}
--- a/devtools/server/actors/watcher.js
+++ b/devtools/server/actors/watcher.js
@@ -168,16 +168,18 @@ exports.WatcherActor = protocol.ActorCla
           [Resources.TYPES.SERVER_SENT_EVENT]: hasBrowserElement,
           [Resources.TYPES.WEBSOCKET]: hasBrowserElement,
         },
 
         // @backward-compat { version 93 } Starts supporting setSaveRequestAndResponseBodies on the NetworkParent actor
         saveRequestAndResponseBodies: true,
         // @backward-compat { version 93 } The network parent actor started exposing setPersist method.
         "network-persist": true,
+        // @backward-compat { version 94 } Full support for event breakpoints via the watcher actor
+        "event-breakpoints": true,
       },
     };
   },
 
   /**
    * Start watching for a new target type.
    *
    * This will instantiate Target Actors for existing debugging context of this type,
--- a/devtools/server/actors/watcher/WatchedDataHelpers.jsm
+++ b/devtools/server/actors/watcher/WatchedDataHelpers.jsm
@@ -6,28 +6,56 @@
 
 /**
  * Helper module alongside WatcherRegistry, which focus on updating the "watchedData" object.
  * This object is shared across processes and threads and have to be maintained in all these runtimes.
  */
 
 var EXPORTED_SYMBOLS = ["WatchedDataHelpers"];
 
-// Allow this JSM to also be loaded as a CommonJS module
-// Because this module is used from the worker thread,
-// (via target-actor-mixin), and workers can't load JSMs via ChromeUtils.import.
-const { validateBreakpointLocation } =
-  typeof module == "object"
-    ? require("devtools/shared/validate-breakpoint.jsm")
-    : ChromeUtils.import("resource://devtools/shared/validate-breakpoint.jsm");
+if (typeof module == "object") {
+  // Allow this JSM to also be loaded as a CommonJS module
+  // Because this module is used from the worker thread,
+  // (via target-actor-mixin), and workers can't load JSMs via ChromeUtils.import.
+  loader.lazyRequireGetter(
+    this,
+    "validateBreakpointLocation",
+    "devtools/shared/validate-breakpoint.jsm",
+    true
+  );
+
+  loader.lazyRequireGetter(
+    this,
+    "validateEventBreakpoint",
+    "devtools/server/actors/utils/event-breakpoints",
+    true
+  );
+} else {
+  const { XPCOMUtils } = ChromeUtils.import(
+    "resource://gre/modules/XPCOMUtils.jsm"
+  );
+  XPCOMUtils.defineLazyGetter(this, "validateBreakpointLocation", () => {
+    return ChromeUtils.import(
+      "resource://devtools/shared/validate-breakpoint.jsm"
+    ).validateBreakpointLocation;
+  });
+  XPCOMUtils.defineLazyGetter(this, "validateEventBreakpoint", () => {
+    const { loader } = ChromeUtils.import(
+      "resource://devtools/shared/Loader.jsm"
+    );
+    return loader.require("devtools/server/actors/utils/event-breakpoints")
+      .validateEventBreakpoint;
+  });
+}
 
 // List of all arrays stored in `watchedData`, which are replicated across processes and threads
 const SUPPORTED_DATA = {
   BREAKPOINTS: "breakpoints",
   XHR_BREAKPOINTS: "xhr-breakpoints",
+  EVENT_BREAKPOINTS: "event-breakpoints",
   RESOURCES: "resources",
   TARGET_CONFIGURATION: "target-configuration",
   THREAD_CONFIGURATION: "thread-configuration",
   TARGETS: "targets",
 };
 
 // Optional function, if data isn't a primitive data type in order to produce a key
 // for the given data entry
@@ -54,16 +82,29 @@ const DATA_KEY_FUNCTION = {
     }
     if (typeof method != "string") {
       throw new Error(
         `XHR Breakpoints expect to have method string, got ${typeof method} instead.`
       );
     }
     return `${path}:${method}`;
   },
+  [SUPPORTED_DATA.EVENT_BREAKPOINTS]: function(id) {
+    if (typeof id != "string") {
+      throw new Error(
+        `Event Breakpoints expect the id to be a string , got ${typeof id} instead.`
+      );
+    }
+    if (!validateEventBreakpoint(id)) {
+      throw new Error(
+        `The id string should be a valid event breakpoint id, ${id} is not.`
+      );
+    }
+    return id;
+  },
 };
 
 function idFunction(v) {
   if (typeof v != "string") {
     throw new Error(
       `Expect data entry values to be string, or be using custom data key functions. Got ${typeof v} type instead.`
     );
   }
--- a/devtools/shared/specs/breakpoint-list.js
+++ b/devtools/shared/specs/breakpoint-list.js
@@ -34,12 +34,17 @@ const breakpointListSpec = generateActor
       },
     },
     removeXHRBreakpoint: {
       request: {
         path: Arg(0, "string"),
         method: Arg(1, "string"),
       },
     },
+    setActiveEventBreakpoints: {
+      request: {
+        ids: Arg(0, "array:string"),
+      },
+    },
   },
 });
 
 exports.breakpointListSpec = breakpointListSpec;