Bug 1908095 - [devtools] removeSessionDataEntry for webextension toolboxes and watcher targets r=ochameau,devtools-reviewers

Fix removing breakpoints in webextension toolboxes and watcher targets, add tests to cover both scenarios

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

devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_addons_debug_debugger.js

@@ -2,6 +2,11 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
+Services.scriptloader.loadSubScript(
+  "chrome://mochitests/content/browser/devtools/client/debugger/test/mochitest/shared-head.js",
+  this
+);
+
 /* import-globals-from helper-addons.js */
 Services.scriptloader.loadSubScript(CHROME_URL_ROOT + "helper-addons.js", this);
 
@@ -9,10 +14,10 @@ const L10N = new LocalizationHelper(
   "devtools/client/locales/toolbox.properties"
 );
 
-add_task(async () => {
-  const EXTENSION_NAME = "temporary-web-extension";
-  const EXTENSION_ID = "test-devtools@mozilla.org";
+const EXTENSION_NAME = "temporary-web-extension";
+const EXTENSION_ID = "test-devtools@mozilla.org";
 
+add_task(async function testOpenDebuggerReload() {
   await enableExtensionDebugging();
 
   info(
@@ -84,3 +89,61 @@ add_task(async () => {
   await removeTemporaryExtension(EXTENSION_NAME, document);
   await removeTab(tab);
 });
+
+add_task(async function testAddAndRemoveBreakpoint() {
+  await enableExtensionDebugging();
+
+  const { document, tab, window } = await openAboutDebugging();
+  await selectThisFirefoxPage(document, window.AboutDebugging.store);
+
+  await installTemporaryExtensionFromXPI(
+    {
+      background() {
+        window.invokeLogFromWebextension = () => {
+          console.log("From webextension");
+        };
+      },
+      id: EXTENSION_ID,
+      name: EXTENSION_NAME,
+    },
+    document
+  );
+
+  // Select the debugger right away to avoid any noise coming from the inspector.
+  await pushPref("devtools.toolbox.selectedTool", "jsdebugger");
+  const { devtoolsWindow } = await openAboutDevtoolsToolbox(
+    document,
+    tab,
+    window,
+    EXTENSION_NAME
+  );
+  const toolbox = getToolbox(devtoolsWindow);
+  const dbg = createDebuggerContext(toolbox);
+
+  info("Select the source and add a breakpoint");
+  // Note: the background script filename is dynamically generated id, so we
+  // simply get the first source from the list.
+  const displayedSources = dbg.selectors.getDisplayedSourcesList();
+  const backgroundScript = displayedSources[0];
+  await selectSource(dbg, backgroundScript);
+  await addBreakpoint(dbg, backgroundScript, 3);
+
+  info("Trigger the breakpoint and wait for the debugger to pause");
+  const webconsole = await toolbox.selectTool("webconsole");
+  const { hud } = webconsole;
+  hud.ui.wrapper.dispatchEvaluateExpression("invokeLogFromWebextension()");
+  await waitForPaused(dbg);
+
+  info("Resume and remove the breakpoint");
+  await resume(dbg);
+  await removeBreakpoint(dbg, backgroundScript.id, 3);
+
+  info("Trigger the function again and check the debugger does not pause");
+  hud.ui.wrapper.dispatchEvaluateExpression("invokeLogFromWebextension()");
+  await wait(500);
+  assertNotPaused(dbg);
+
+  await closeWebExtAboutDevtoolsToolbox(devtoolsWindow, window);
+  await removeTemporaryExtension(EXTENSION_NAME, document);
+  await removeTab(tab);
+});

devtools/client/debugger/test/mochitest/browser_dbg-features-breakable-lines.js

@@ -19,12 +19,14 @@ add_task(async function testBreakableLinesOverReloads() {
   );
 
   info("Assert breakable lines of the first html page load");
-  await assertBreakableLines(dbg, "index.html", 78, [
+  await assertBreakableLines(dbg, "index.html", 85, [
     ...getRange(16, 17),
     21,
     ...getRange(24, 25),
     30,
     36,
+    39,
+    ...getRange(41, 43),
   ]);
 
   info("Assert breakable lines of the first original source file, original.js");

devtools/client/debugger/test/mochitest/browser_dbg-features-breakable-positions.js

@@ -30,7 +30,7 @@ add_task(async function testBreakableLinesOverReloads() {
   );
 
   info("Assert breakable lines of the first html page load");
-  await assertBreakablePositions(dbg, "index.html", 78, [
+  await assertBreakablePositions(dbg, "index.html", 85, [
     { line: 16, columns: [6, 14] },
     { line: 17, columns: [] },
     { line: 21, columns: [12, 20, 48] },
@@ -38,16 +38,23 @@ add_task(async function testBreakableLinesOverReloads() {
     { line: 25, columns: [] },
     { line: 30, columns: [] },
     { line: 36, columns: [] },
+    { line: 39, columns: [] },
+    { line: 41, columns: [8, 18] },
+    { line: 42, columns: [] },
+    { line: 43, columns: [] },
   ]);
 
   info("Pretty print first html page load and assert breakable lines");
   await prettyPrint(dbg);
-  await assertBreakablePositions(dbg, "index.html:formatted", 87, [
+  await assertBreakablePositions(dbg, "index.html:formatted", 96, [
     { line: 16, columns: [0, 8] },
     { line: 22, columns: [0, 8, 35] },
     { line: 27, columns: [0, 8] },
     { line: 28, columns: [] },
     { line: 36, columns: [] },
+    { line: 48, columns: [] },
+    { line: 50, columns: [2, 12] },
+    { line: 53, columns: [] },
   ]);
   await closeTab(dbg, "index.html:formatted");
 

devtools/client/debugger/test/mochitest/browser_dbg-features-breakpoints.js

@@ -71,3 +71,62 @@ add_task(
     await assertNotPaused(dbg);
   }
 );
+
+/**
+ * Tests that the source tree works with all the various types of sources
+ * coming from the integration test page.
+ *
+ * Also assert a few extra things on sources with query strings:
+ *  - they can be pretty printed,
+ *  - quick open matches them,
+ *  - you can set breakpoint on them.
+ */
+add_task(async function testSourceTreeOnTheIntegrationTestPage() {
+  const dbg = await initDebuggerWithAbsoluteURL("about:blank");
+
+  await navigateToAbsoluteURL(
+    dbg,
+    TEST_URL,
+    "index.html",
+    "script.js",
+    "log-worker.js"
+  );
+
+  info("Select the source and add a breakpoint");
+  await selectSource(dbg, "script.js");
+  await addBreakpoint(dbg, "script.js", 7);
+
+  info("Trigger the breakpoint and wait for the debugger to pause");
+  invokeInTab("nonSourceMappedFunction");
+  await waitForPaused(dbg);
+
+  info("Resume and remove the breakpoint");
+  await resume(dbg);
+  await removeBreakpoint(dbg, findSource(dbg, "script.js").id, 7);
+
+  info("Trigger the function again and check the debugger does not pause");
+  invokeInTab("nonSourceMappedFunction");
+  await wait(500);
+  assertNotPaused(dbg);
+
+  info("[worker] Select the source and add a breakpoint");
+  await selectSource(dbg, "log-worker.js");
+  await addBreakpoint(dbg, "log-worker.js", 2);
+
+  info("[worker] Trigger the breakpoint and wait for the debugger to pause");
+  invokeInTab("invokeLogWorker");
+  await waitForPaused(dbg);
+
+  info("[worker] Resume and remove the breakpoint");
+  await resume(dbg);
+  await removeBreakpoint(dbg, findSource(dbg, "log-worker.js").id, 2);
+
+  info(
+    "[worker] Trigger the function again and check the debugger does not pause"
+  );
+  invokeInTab("invokeLogWorker");
+  await wait(500);
+  assertNotPaused(dbg);
+
+  dbg.toolbox.closeToolbox();
+});

devtools/client/debugger/test/mochitest/browser_dbg-features-source-tree.js

@@ -320,11 +320,12 @@ add_task(async function testSourceTreeOnTheIntegrationTestPage() {
     .getAllThreads()
     .find(thread => thread.name == "Main Thread");
 
+  // When EFT is disabled the iframe's source is meld into the main target.
+  const expectedSameUrlSources = isEveryFrameTargetEnabled() ? 3 : 4;
   is(
     sourceActors.filter(actor => actor.thread == mainThread.actor).length,
-    // When EFT is disabled the iframe's source is meld into the main target
-    isEveryFrameTargetEnabled() ? 3 : 4,
-    "same-url.js is loaded 3 times in the main thread"
+    expectedSameUrlSources,
+    `same-url.js is loaded ${expectedSameUrlSources} times in the main thread`
   );
 
   if (isEveryFrameTargetEnabled()) {

devtools/client/debugger/test/mochitest/examples/sourcemaps-reload-uncompressed/v1/index.html

@@ -35,6 +35,13 @@
       `);
     </script>
 
+    <script>
+      const logWorker = new Worker("log-worker.js");
+      function invokeLogWorker() {
+        logWorker.postMessage({yo: 'yo'})
+      }
+    </script>
+
     <!-- Test JS file with encoded characters as well as custom protocol -->
     <script>
     //# sourceURL=foo://bar/%e6%96%87%e5%ad%97%e3%82%b3%e3%83%bc%e3%83%89.js

devtools/client/debugger/test/mochitest/examples/sourcemaps-reload-uncompressed/v1/log-worker.js

@@ -0,0 +1,3 @@
+self.onmessage = function onmessage() {
+  console.log("hi");
+};

devtools/client/debugger/test/mochitest/head.js

@@ -136,6 +136,7 @@ const INTEGRATION_TEST_PAGE_SOURCES = [
   // But there is even more source actors (named evals and duplicated script tags).
   "same-url.sjs",
   "same-url.sjs",
+  "log-worker.js",
 ];
 // The iframe one is only available when fission is enabled, or EFT
 if (isFissionEnabled() || isEveryFrameTargetEnabled()) {

devtools/server/connectors/js-process-actor/DevToolsProcessChild.sys.mjs

@@ -461,18 +461,51 @@ export class DevToolsProcessChild extends JSProcessActorChild {
     if (!watcherDataObject) {
       return;
     }
+    const { actors, sessionData, watchingTargetTypes } = watcherDataObject;
 
     // Maintain the copy of `sessionData` so that it is up-to-date when
     // a new worker target needs to be instantiated
-    lazy.SessionDataHelpers.removeSessionDataEntry(
-      watcherDataObject.sessionData,
-      type,
-      entries
-    );
+    lazy.SessionDataHelpers.removeSessionDataEntry(sessionData, type, entries);
 
-    for (const targetActor of watcherDataObject.actors) {
+    for (const targetActor of actors) {
       targetActor.removeSessionDataEntry(type, entries);
     }
+
+    // Special code paths for webextension toolboxes and worker targets
+    // See addOrSetSessionDataEntry for more details.
+
+    if (sessionData.sessionContext.type == "webextension") {
+      const connectionPrefix = watcherActorID.replace(/watcher\d+$/, "");
+      const targetActors = lazy.TargetActorRegistry.getTargetActors(
+        sessionData.sessionContext,
+        connectionPrefix
+      );
+      if (targetActors.length) {
+        targetActors[0].removeSessionDataEntry(type, entries);
+      }
+    }
+
+    if (watchingTargetTypes.includes("worker")) {
+      this.#watchers.worker.watcher.removeSessionDataEntry(
+        watcherDataObject,
+        type,
+        entries
+      );
+    }
+    if (watchingTargetTypes.includes("service_worker")) {
+      this.#watchers.service_worker.watcher.removeSessionDataEntry(
+        watcherDataObject,
+        type,
+        entries
+      );
+    }
+    if (watchingTargetTypes.includes("shared_worker")) {
+      this.#watchers.shared_worker.watcher.removeSessionDataEntry(
+        watcherDataObject,
+        type,
+        entries
+      );
+    }
   }
 
   /**

devtools/server/connectors/js-process-actor/target-watchers/worker.sys.mjs

@@ -76,6 +76,24 @@ export class WorkerTargetWatcherClass {
     await Promise.all(promises);
   }
 
+  removeSessionDataEntry(watcherDataObject, type, entries) {
+    for (const { dbg, workerThreadServerForwardingPrefix } of watcherDataObject
+      .workers[this.#workerTargetType]) {
+      if (!isWorkerDebuggerAlive(dbg)) {
+        continue;
+      }
+
+      dbg.postMessage(
+        JSON.stringify({
+          type: "remove-session-data-entry",
+          forwardingPrefix: workerThreadServerForwardingPrefix,
+          dataEntryType: type,
+          entries,
+        })
+      );
+    }
+  }
+
   /**
    * Called whenever a new Worker is instantiated in the current process
    *