Bug 1685105 - [devtools] Pass XHR breakpoint via the Watcher Actor. r=nchevobbe,bomsy,devtools-backward-compat-reviewers
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 04 Mar 2021 14:49:24 +0000
changeset 637004 5199ec2d73fa7daf423b72416be41383ac1042dc
parent 637003 41654d4eb1ac78477496960a52b57d4a550f0a63
child 637005 fb8b9841d82b7534eba074b5e362a5d56408fc67
push id15212
push userffxbld-merge
push dateMon, 22 Mar 2021 14:40:41 +0000
treeherdermozilla-beta@ad9bae9bb10c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe, bomsy, devtools-backward-compat-reviewers
bugs1685105
milestone88.0a1
first release with
nightly linux32
5199ec2d73fa / 88.0a1 / 20210304215542 / files
nightly linux64
5199ec2d73fa / 88.0a1 / 20210304215542 / files
nightly mac
5199ec2d73fa / 88.0a1 / 20210304215542 / files
nightly win32
5199ec2d73fa / 88.0a1 / 20210304215542 / files
nightly win64
5199ec2d73fa / 88.0a1 / 20210304215542 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1685105 - [devtools] Pass XHR breakpoint via the Watcher Actor. r=nchevobbe,bomsy,devtools-backward-compat-reviewers Differential Revision: https://phabricator.services.mozilla.com/D103375
devtools/client/debugger/src/client/firefox/commands.js
devtools/client/debugger/test/mochitest/browser_dbg-xhr-breakpoints.js
devtools/client/debugger/test/mochitest/examples/doc-early-xhr.html
devtools/client/debugger/test/mochitest/head.js
devtools/client/debugger/test/mochitest/helpers.js
devtools/server/actors/breakpoint-list.js
devtools/server/actors/targets/target-actor-mixin.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
@@ -132,22 +132,38 @@ function breakOnNext(thread) {
 
 async function sourceContents({ actor, thread }) {
   const sourceThreadFront = lookupThreadFront(thread);
   const sourceFront = sourceThreadFront.source({ actor });
   const { source, contentType } = await sourceFront.source();
   return { source, contentType };
 }
 
-function setXHRBreakpoint(path, method) {
-  return currentThreadFront().setXHRBreakpoint(path, method);
+async function setXHRBreakpoint(path, method) {
+  const hasWatcherSupport = targetList.hasTargetWatcherSupport(
+    "set-xhr-breakpoints"
+  );
+  if (!hasWatcherSupport) {
+    // Without watcher support, forward setXHRBreakpoint to all threads.
+    return forEachThread(thread => thread.setXHRBreakpoint(path, method));
+  }
+  const breakpointsFront = await targetList.watcherFront.getBreakpointListActor();
+  await breakpointsFront.setXHRBreakpoint(path, method);
 }
 
-function removeXHRBreakpoint(path, method) {
-  return currentThreadFront().removeXHRBreakpoint(path, method);
+async function removeXHRBreakpoint(path, method) {
+  const hasWatcherSupport = targetList.hasTargetWatcherSupport(
+    "set-xhr-breakpoints"
+  );
+  if (!hasWatcherSupport) {
+    // Without watcher support, forward setXHRBreakpoint to all threads.
+    return forEachThread(thread => thread.removeXHRBreakpoint(path, method));
+  }
+  const breakpointsFront = await targetList.watcherFront.getBreakpointListActor();
+  await breakpointsFront.removeXHRBreakpoint(path, method);
 }
 
 export function toggleJavaScriptEnabled(enabled) {
   return targetList.updateConfiguration({
     javascriptEnabled: enabled,
   });
 }
 
--- a/devtools/client/debugger/test/mochitest/browser_dbg-xhr-breakpoints.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg-xhr-breakpoints.js
@@ -1,33 +1,78 @@
 /* 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/>. */
 
 add_task(async function() {
+  info("Test XHR requests done very early during page load");
+
+  const dbg = await initDebugger("doc-xhr.html", "fetch.js");
+
+  await addXHRBreakpoint(dbg, "doc", "GET");
+
+  await SpecialPowers.spawn(
+    gBrowser.selectedBrowser,
+    [EXAMPLE_REMOTE_URL + "doc-early-xhr.html"],
+    (remoteUrl) => {
+      const firstIframe = content.document.createElement("iframe");
+      content.document.body.append(firstIframe);
+      firstIframe.src = remoteUrl;
+    }
+  );
+
+  await waitForPaused(dbg);
+  assertPausedLocation(dbg);
+  await resume(dbg);
+
+  await dbg.actions.removeXHRBreakpoint(0);
+
+  await SpecialPowers.spawn(
+    gBrowser.selectedBrowser,
+    [EXAMPLE_REMOTE_URL + "doc-early-xhr.html"],
+    (remoteUrl) => {
+      const secondIframe = content.document.createElement("iframe");
+      content.document.body.append(secondIframe);
+      secondIframe.src = remoteUrl;
+    }
+  );
+
+  // Wait for some time, in order to wait for it to be paused
+  // in case we regress
+  await waitForTime(1000);
+
+  assertNotPaused(dbg);
+});
+
+add_task(async function() {
+  info("Test simple XHR breakpoints set before doing the request");
+
   const dbg = await initDebugger("doc-xhr.html", "fetch.js");
 
   await addXHRBreakpoint(dbg, "doc", "GET");
 
   invokeInTab("main", "doc-xhr.html");
   await waitForPaused(dbg);
   assertPausedLocation(dbg);
   await resume(dbg);
 
   await dbg.actions.removeXHRBreakpoint(0);
   await invokeInTab("main", "doc-xhr.html");
   assertNotPaused(dbg);
 
+
+  info("Test that we do not pause on different method type");
   await addXHRBreakpoint(dbg, "doc", "POST");
   await invokeInTab("main", "doc-xhr.html");
   assertNotPaused(dbg);
 });
 
 // Tests the "pause on any URL" checkbox works properly
 add_task(async function() {
+  info("Test 'pause on any URL'");
   const dbg = await initDebugger("doc-xhr.html", "fetch.js");
 
   // Enable pause on any URL
   await clickPauseOnAny(dbg, "SET_XHR_BREAKPOINT");
 
   invokeInTab("main", "doc-xhr.html");
   await waitForPaused(dbg);
   await resume(dbg);
@@ -46,16 +91,17 @@ add_task(async function() {
   assertNotPaused(dbg);
 
   // Turn off the checkbox
   await dbg.actions.removeXHRBreakpoint(0);
 });
 
 // Tests removal works properly
 add_task(async function() {
+  info("Assert the frontend state when removing breakpoints");
   const dbg = await initDebugger("doc-xhr.html");
 
   const pauseOnAnyCheckbox = getXHRBreakpointCheckbox(dbg);
 
   await clickPauseOnAny(dbg, "SET_XHR_BREAKPOINT");
   await addXHRBreakpoint(dbg, "1");
   await addXHRBreakpoint(dbg, "2");
   await addXHRBreakpoint(dbg, "3");
new file mode 100644
--- /dev/null
+++ b/devtools/client/debugger/test/mochitest/examples/doc-early-xhr.html
@@ -0,0 +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/. -->
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8"/>
+    <title>Debugger test page</title>
+    <script>
+      fetch("doc-xhr.html").then(console.log);
+    </script>
+  </head>
+
+  <body></body>
+</html>
+
--- a/devtools/client/debugger/test/mochitest/head.js
+++ b/devtools/client/debugger/test/mochitest/head.js
@@ -39,8 +39,13 @@ Services.scriptloader.loadSubScript(
 
 Services.scriptloader.loadSubScript(
   "chrome://mochitests/content/browser/devtools/client/debugger/test/mochitest/helpers.js",
   this
 );
 
 const EXAMPLE_URL =
   "http://example.com/browser/devtools/client/debugger/test/mochitest/examples/";
+
+// This URL is remote compared to EXAMPLE_URL, as one uses .com and the other uses .org
+// Note that this depends on initDebugger to always use EXAMPLE_URL
+const EXAMPLE_REMOTE_URL =
+  "http://example.net/browser/devtools/client/debugger/test/mochitest/examples/";
--- a/devtools/client/debugger/test/mochitest/helpers.js
+++ b/devtools/client/debugger/test/mochitest/helpers.js
@@ -583,16 +583,19 @@ async function clearDebuggerPreferences(
  *
  * @memberof mochitest
  * @param {String} url
  * @return {Promise} dbg
  * @static
  */
 
 async function initDebugger(url, ...sources) {
+  // We depend on EXAMPLE_URLs origin to do cross origin/process iframes via
+  // EXAMPLE_REMOTE_URL. If the top level document origin changes,
+  // we may break this. So be careful if you want to change EXAMPLE_URL.
   return initDebuggerWithAbsoluteURL(EXAMPLE_URL + url, ...sources);
 }
 
 async function initDebuggerWithAbsoluteURL(url, ...sources) {
   await clearDebuggerPreferences();
   const toolbox = await openNewTabAndToolbox(url, "jsdebugger");
   const dbg = createDebuggerContext(toolbox);
 
--- 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 } = SUPPORTED_DATA;
+const { BREAKPOINTS, XHR_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.
  *
@@ -36,11 +36,37 @@ const BreakpointListActor = ActorClassWi
     return this.watcherActor.addDataEntry(BREAKPOINTS, [{ location, options }]);
   },
 
   removeBreakpoint(location, options) {
     return this.watcherActor.removeDataEntry(BREAKPOINTS, [
       { location, options },
     ]);
   },
+
+  /**
+   * Request to break on next XHR or Fetch request for a given URL and HTTP Method.
+   *
+   * @param {String} path
+   *                 If empty, will pause on regardless or the request's URL.
+   *                 Otherwise, will pause on any request whose URL includes this string.
+   *                 This is not specific to URL's path. It can match the URL origin.
+   * @param {String} method
+   *                 If set to "ANY", will pause regardless of which method is used.
+   *                 Otherwise, should be set to any valid HTTP Method (GET, POST, ...)
+   */
+  setXHRBreakpoint(path, method) {
+    return this.watcherActor.addDataEntry(XHR_BREAKPOINTS, [{ path, method }]);
+  },
+
+  /**
+   * Stop breakpoint on requests we ask to break on via setXHRBreakpoint.
+   *
+   * See setXHRBreakpoint for arguments definition.
+   */
+  removeXHRBreakpoint(path, method) {
+    return this.watcherActor.removeDataEntry(XHR_BREAKPOINTS, [
+      { path, method },
+    ]);
+  },
 });
 
 exports.BreakpointListActor = BreakpointListActor;
--- a/devtools/server/actors/targets/target-actor-mixin.js
+++ b/devtools/server/actors/targets/target-actor-mixin.js
@@ -5,18 +5,23 @@
 "use strict";
 
 const { ActorClassWithSpec } = require("devtools/shared/protocol");
 
 const Resources = require("devtools/server/actors/resources/index");
 const {
   WatchedDataHelpers,
 } = require("devtools/server/actors/watcher/WatchedDataHelpers.jsm");
-const { RESOURCES, BREAKPOINTS } = WatchedDataHelpers.SUPPORTED_DATA;
 const { STATES: THREAD_STATES } = require("devtools/server/actors/thread");
+const {
+  RESOURCES,
+  BREAKPOINTS,
+  TARGET_CONFIGURATION,
+  XHR_BREAKPOINTS,
+} = WatchedDataHelpers.SUPPORTED_DATA;
 
 module.exports = function(targetType, targetActorSpec, implementation) {
   const proto = {
     /**
      * Type of target, a string of Targets.TYPES.
      * @return {string}
      */
     targetType,
@@ -53,38 +58,65 @@ module.exports = function(targetType, ta
           // If addWatcherDataEntry is called for an existing target, set the new
           // breakpoints on the already running thread actor.
           await Promise.all(
             entries.map(({ location, options }) =>
               this.threadActor.setBreakpoint(location, options)
             )
           );
         }
-      } else if (type == "target-configuration") {
+      } else if (type == TARGET_CONFIGURATION) {
         // Only BrowsingContextTargetActor implements updateTargetConfiguration,
         // skip this data entry update for other targets.
         if (typeof this.updateTargetConfiguration == "function") {
           const options = {};
           for (const { key, value } of entries) {
             options[key] = value;
           }
           this.updateTargetConfiguration(options);
         }
+      } else if (type == XHR_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)
+        if (typeof this.attach == "function") {
+          this.attach();
+        }
+
+        // The thread actor has to be initialized in order to correctly
+        // retrieve the stack trace when hitting an XHR
+        if (
+          this.threadActor.state == "detached" &&
+          !this.targetType.endsWith("worker")
+        ) {
+          await this.threadActor.attach();
+        }
+
+        await Promise.all(
+          entries.map(({ path, method }) =>
+            this.threadActor.setXHRBreakpoint(path, method)
+          )
+        );
       }
     },
 
     removeWatcherDataEntry(type, entries) {
       if (type == RESOURCES) {
         return this._unwatchTargetResources(entries);
       } else if (type == BREAKPOINTS) {
         for (const { location } of entries) {
           this.threadActor.removeBreakpoint(location);
         }
-      } else if (type == "target-configuration") {
+      } else if (type == TARGET_CONFIGURATION) {
         // configuration data entries are always added/updated, never removed.
+      } else if (type == XHR_BREAKPOINTS) {
+        for (const { path, method } of entries) {
+          this.threadActor.removeXHRBreakpoint(path, method);
+        }
       }
 
       return Promise.resolve();
     },
 
     /**
      * These two methods will create and destroy resource watchers
      * for each resource type. This will end up calling `notifyResourceAvailable`
--- a/devtools/server/actors/watcher.js
+++ b/devtools/server/actors/watcher.js
@@ -164,16 +164,21 @@ exports.WatcherActor = protocol.ActorCla
         // that the Watcher is available
         "set-breakpoints": true,
         // @backward-compat { version 87 } Starting with FF87, if the watcher is
         // supported, the TargetConfiguration actor can be used to set configuration
         // flags which impact BrowsingContext targets.
         // When removing this trait, consumers should still check that the Watcher is
         // available.
         "target-configuration": true,
+        // @backward-compat { version 88 } Watcher now supports setting the XHR via
+        // the BreakpointListActor.
+        // When removing this trait, consumers should still check that the Watcher is
+        // available.
+        "set-xhr-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
@@ -9,16 +9,17 @@
  * This object is shared across processes and threads and have to be maintained in all these runtimes.
  */
 
 var EXPORTED_SYMBOLS = ["WatchedDataHelpers"];
 
 // List of all arrays stored in `watchedData`, which are replicated across processes and threads
 const SUPPORTED_DATA = {
   BREAKPOINTS: "breakpoints",
+  XHR_BREAKPOINTS: "xhr-breakpoints",
   RESOURCES: "resources",
   TARGET_CONFIGURATION: "target-configuration",
   TARGETS: "targets",
 };
 
 // Optional function, if data isn't a primitive data type in order to produce a key
 // for the given data entry
 const DATA_KEY_FUNCTION = {
@@ -53,16 +54,29 @@ const DATA_KEY_FUNCTION = {
     }
     return `${sourceUrl}:${sourceId}:${line}:${column}`;
   },
   [SUPPORTED_DATA.TARGET_CONFIGURATION]: function({ key }) {
     // Configuration data entries are { key, value } objects, `key` can be used
     // as the unique identifier for the entry.
     return key;
   },
+  [SUPPORTED_DATA.XHR_BREAKPOINTS]: function({ path, method }) {
+    if (typeof path != "string") {
+      throw new Error(
+        `XHR Breakpoints expect to have path string, got ${typeof path} instead.`
+      );
+    }
+    if (typeof method != "string") {
+      throw new Error(
+        `XHR Breakpoints expect to have method string, got ${typeof method} instead.`
+      );
+    }
+    return `${path}:${method}`;
+  },
 };
 
 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
@@ -21,12 +21,25 @@ const breakpointListSpec = generateActor
         options: Arg(1, "breakpoint-list.breakpoint-options"),
       },
     },
     removeBreakpoint: {
       request: {
         location: Arg(0, "json"),
       },
     },
+
+    setXHRBreakpoint: {
+      request: {
+        path: Arg(0, "string"),
+        method: Arg(1, "string"),
+      },
+    },
+    removeXHRBreakpoint: {
+      request: {
+        path: Arg(0, "string"),
+        method: Arg(1, "string"),
+      },
+    },
   },
 });
 
 exports.breakpointListSpec = breakpointListSpec;