Backed out 2 changesets (bug 1681698, bug 1681699) for causing failure on browser_browser_resources_console_messages.js. CLOSED TREE

Backed out changeset cf75ed5213a0 (bug 1681698)
Backed out changeset 283763c17ed7 (bug 1681699)

devtools/client/debugger/test/mochitest/browser.ini

@@ -86,7 +86,6 @@ skip-if = debug # Window leaks: bug 1575332
 [browser_dbg-expressions-error.js]
 [browser_dbg-expressions-focus.js]
 [browser_dbg-expressions-watch.js]
-[browser_dbg-fission-frame-breakpoint.js]
 [browser_dbg-fission-frame-sources.js]
 [browser_dbg-fission-switch-target.js]
 [browser_dbg-go-to-line.js]

devtools/client/debugger/test/mochitest/browser_dbg-fission-frame-breakpoint.js

@@ -1,46 +0,0 @@
-/* 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 TEST_COM_URI =
-  URL_ROOT_COM + "examples/doc_dbg-fission-frame-sources.html";
-
-add_task(async function() {
-  // Load a test page with a remote frame:
-  // simple1.js is imported by the main page. simple2.js comes from the remote frame.
-  const dbg = await initDebuggerWithAbsoluteURL(
-    TEST_COM_URI,
-    "simple1.js",
-    "simple2.js"
-  );
-  const {
-    selectors: { getSelectedSource, getIsPaused, getCurrentThread },
-  } = dbg;
-
-  // Add breakpoint within the iframe, which is hit early on load
-  await selectSource(dbg, "simple2.js");
-  await addBreakpoint(dbg, "simple2.js", 7);
-
-  const onBreakpoint = waitForDispatch(dbg, "SET_BREAKPOINT");
-  info("Reload the page to hit the breakpoint on load");
-  await reload(dbg);
-  await onBreakpoint
-  await waitForSelectedSource(dbg, "simple2.js");
-
-  ok(getSelectedSource().url.includes("simple2.js"), "Selected source is simple2.js");
-  assertPausedLocation(dbg);
-  assertDebugLine(dbg, 7);
-
-  await stepIn(dbg);
-  assertDebugLine(dbg, 7);
-  assertPausedLocation(dbg);
-
-  // We can't used `stepIn` helper as this last step will resume
-  // and the helper is expecting to pause again
-  await dbg.actions.stepIn(getThreadContext(dbg));
-  ok(!isPaused(dbg), "Stepping in two times resumes");
-
-  await dbg.toolbox.closeToolbox();
-});

devtools/client/fronts/targets/target-mixin.js

@@ -522,24 +522,6 @@ function TargetMixin(parentClass) {
         );
       }
       this.threadFront = await this.getFront("thread");
-
-      // Avoid attaching if the thread actor was already attached on target creation from the server side.
-      // This doesn't include:
-      // * targets that aren't yet supported by the Watcher (like web extensions),
-      // * workers, which still use a unique codepath for thread actor attach
-      // * all targets when connecting to an older server
-      // @backward-compat { version 87 } If all targets are supported by watcher actor, and workers no longer use
-      //                                 its unique attach sequence, we can assume the thread front is always attached.
-      const isAttached =
-        this.getTrait("supportsThreadActorIsAttached") &&
-        (await this.threadFront.isAttached());
-      if (isAttached) {
-        // If the Thread actor has already been attached from the server side
-        // by the Watcher Actor, we still have to pass options that aren't yet managed via
-        // the Watcher actor's addWatcherDataEntry codepath (bug 1687261).
-        await this.threadFront.reconfigure(options);
-        return this.threadFront;
-      }
       if (
         this.isDestroyedOrBeingDestroyed() ||
         this.threadFront.isDestroyed()

devtools/client/shared/test/browser.ini

@@ -25,6 +25,7 @@ support-files =
   doc_html_tooltip_doorhanger-02.xhtml
   doc_html_tooltip_hover.xhtml
   doc_html_tooltip_rtl.xhtml
+  doc_inline-debugger-statement.html
   doc_inplace-editor_autocomplete_offset.xhtml
   doc_layoutHelpers_getBoxQuads1.html
   doc_layoutHelpers_getBoxQuads2-a.html
@@ -77,6 +78,8 @@ support-files =
 [browser_cubic-bezier-06.js]
 [browser_cubic-bezier-07.js]
 tags = addons
+[browser_dbg_debugger-statement.js]
+skip-if = e10s && debug
 [browser_dbg_listworkers.js]
 [browser_filter-editor-01.js]
 [browser_filter-editor-02.js]

devtools/client/shared/test/browser_dbg_debugger-statement.js

@@ -0,0 +1,164 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Tests the behavior of the debugger statement.
+ */
+
+// Use distinct origins in order to use distinct processes when fission is enabled
+const TAB_URL = URL_ROOT_COM + "doc_inline-debugger-statement.html";
+const IFRAME_URL = URL_ROOT_ORG + "doc_inline-debugger-statement.html";
+
+add_task(async () => {
+  const tab = await addTab(TAB_URL);
+  const tabBrowsingContext = tab.linkedBrowser.browsingContext;
+
+  const iframeBrowsingContext = await SpecialPowers.spawn(
+    tabBrowsingContext,
+    [IFRAME_URL],
+    async function(url) {
+      const iframe = content.document.createElement("iframe");
+      const onLoad = new Promise(r =>
+        iframe.addEventListener("load", r, { once: true })
+      );
+      iframe.src = url;
+      content.document.body.appendChild(iframe);
+      await onLoad;
+      return iframe.browsingContext;
+    }
+  );
+
+  const target = await TargetFactory.forTab(tab);
+  await target.attach();
+  const { client } = target;
+
+  info("## Test debugger statement against the top level tab document");
+  // This function, by calling the debugger statement function, will bump the increment
+  const threadFront = await testEarlyDebuggerStatement(
+    client,
+    tabBrowsingContext,
+    target
+  );
+  await testDebuggerStatement(client, tabBrowsingContext, threadFront, 1);
+
+  info("## Test debugger statement againt a distinct origin iframe");
+  if (isFissionEnabled()) {
+    // We have to use the watcher in order to create the frame target
+    // and also have to attach to it in order to later be able to
+    // create the thread front
+    const watcherFront = await target.getWatcherFront();
+    await watcherFront.watchTargets("frame");
+    const iframeTarget = await target.getBrowsingContextTarget(
+      iframeBrowsingContext.id
+    );
+    await iframeTarget.attach();
+
+    // This function, by calling the debugger statement function, will bump the increment
+    const iframeThreadFront = await testEarlyDebuggerStatement(
+      client,
+      iframeBrowsingContext,
+      iframeTarget
+    );
+    await testDebuggerStatement(
+      client,
+      iframeBrowsingContext,
+      iframeThreadFront,
+      1
+    );
+  } else {
+    // But in this case, the increment will be 0 as the previous call to `testEarlyDebuggerStatement`
+    // bumped the tab's document increment and not the iframe's one.
+    await testDebuggerStatement(client, iframeBrowsingContext, threadFront, 0);
+  }
+
+  await target.destroy();
+});
+
+async function testEarlyDebuggerStatement(
+  client,
+  browsingContext,
+  targetFront
+) {
+  const onPaused = function(packet) {
+    ok(false, "Pause shouldn't be called before we've attached!");
+  };
+
+  // using the DevToolsClient to listen to the pause packet, as the
+  // threadFront is not yet attached.
+  client.on("paused", onPaused);
+
+  // This should continue without nesting an event loop and calling
+  // the onPaused hook, because we haven't attached yet.
+  const increment = await SpecialPowers.spawn(
+    browsingContext,
+    [],
+    async function() {
+      content.wrappedJSObject.runDebuggerStatement();
+      // Pile up another setTimeout in order to guarantee that the other one ran
+      await new Promise(r => content.setTimeout(r));
+      return content.wrappedJSObject.increment;
+    }
+  );
+  is(increment, 1, "As the thread wasn't paused, setTimeout worked");
+
+  client.off("paused", onPaused);
+
+  // Now attach
+  const threadFront = await targetFront.attachThread();
+  ok(true, "Pause wasn't called before we've attached.");
+
+  return threadFront;
+}
+
+async function testDebuggerStatement(
+  client,
+  browsingContext,
+  threadFront,
+  incrementOriginalValue
+) {
+  const onPaused = threadFront.once("paused");
+
+  // Reach around the debugging protocol and execute the debugger statement.
+  // Not that this will be paused and spawn will only resolve once
+  // the thread will be resumed
+  const onResumed = SpecialPowers.spawn(browsingContext, [], function() {
+    content.wrappedJSObject.runDebuggerStatement();
+  });
+
+  info("Waiting for paused event");
+  await onPaused;
+  ok(true, "The pause handler was triggered on a debugger statement.");
+
+  // Pile up another setTimeout in order to guarantee that the other did not run
+  /* eslint-disable-next-line mozilla/no-arbitrary-setTimeout */
+  await new Promise(r => setTimeout(r, 1000));
+
+  let increment = await SpecialPowers.spawn(
+    browsingContext,
+    [],
+    async function() {
+      return content.wrappedJSObject.increment;
+    }
+  );
+  is(
+    increment,
+    incrementOriginalValue,
+    "setTimeout are frozen while the thread is paused"
+  );
+
+  await threadFront.resume();
+  await onResumed;
+
+  increment = await SpecialPowers.spawn(browsingContext, [], async function() {
+    // Pile up another setTimeout in order to guarantee that the other did run
+    await new Promise(r => content.setTimeout(r));
+    return content.wrappedJSObject.increment;
+  });
+  is(
+    increment,
+    incrementOriginalValue + 1,
+    "setTimeout are resumed after the thread is resumed"
+  );
+}

devtools/client/shared/test/doc_inline-debugger-statement.html

@@ -0,0 +1,27 @@
+<!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
+<!doctype html>
+
+<html>
+  <head>
+    <meta charset="utf-8"/>
+    <title>Debugger test page</title>
+  </head>
+
+  <body>
+    <button>Click me!</button>
+
+    <script type="text/javascript">
+      "use strict";
+      
+      var increment = 0; 
+      /* exported runDebuggerStatement */
+      function runDebuggerStatement() {
+        setTimeout(() => increment++, 0);
+        // eslint-disable-next-line no-debugger
+        debugger;
+      }
+    </script>
+  </body>
+
+</html>

devtools/server/actors/root.js

@@ -133,11 +133,9 @@ exports.RootActor = protocol.ActorClassWithSpec(rootSpec, {
             "dom.worker.console.dispatch_events_to_main_thread"
           )
         : true,
-      // @backward-compat { version 86 } ThreadActor.attach no longer pauses the thread,
+      // @backward-compat { version 86 } ThreadActor.attach no longer pause the thread,
       //                                 so that we no longer have to resume.
       noPauseOnThreadActorAttach: true,
-      // @backward-compat { version 87 } ThreadActor supports isAttached request
-      supportsThreadActorIsAttached: true,
     };
   },
 

devtools/server/actors/targets/target-actor-mixin.js

@@ -7,11 +7,6 @@
 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");
 
 module.exports = function(targetType, targetActorSpec, implementation) {
   const proto = {
@@ -30,9 +25,9 @@ module.exports = function(targetType, targetActorSpec, implementation) {
      *        The values to be added to this type of data
      */
     async addWatcherDataEntry(type, entries) {
-      if (type == RESOURCES) {
+      if (type == "resources") {
         await this._watchTargetResources(entries);
-      } else if (type == BREAKPOINTS) {
+      } 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,
@@ -41,30 +36,18 @@ module.exports = function(targetType, targetActorSpec, implementation) {
           this.attach();
         }
 
-        const isTargetCreation =
-          this.threadActor.state == THREAD_STATES.DETACHED;
-        if (isTargetCreation && !this.targetType.endsWith("worker")) {
-          // If addWatcherDataEntry is called during target creation, attach the
-          // thread actor automatically and pass the initial breakpoints.
-          // However, do not attach the thread actor for Workers. They use a codepath
-          // which releases the worker on `attach`. For them, the client will call `attach`. (bug 1691986)
-          await this.threadActor.attach({ breakpoints: entries });
-        } else {
-          // 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)
-            )
-          );
-        }
+        await Promise.all(
+          entries.map(({ location, options }) =>
+            this.threadActor.setBreakpoint(location, options)
+          )
+        );
       }
     },
 
     removeWatcherDataEntry(type, entries) {
-      if (type == RESOURCES) {
+      if (type == "resources") {
         return this._unwatchTargetResources(entries);
-      } else if (type == BREAKPOINTS) {
+      } else if (type == "breakpoints") {
         for (const { location } of entries) {
           this.threadActor.removeBreakpoint(location);
         }

devtools/server/actors/thread.js

@@ -149,7 +149,6 @@ const STATES = {
   // When paused on any type of breakpoint, or, when the client requested an interrupt.
   PAUSED: "paused",
 };
-exports.STATES = STATES;
 
 // Possible values for the `why.type` attribute in "paused" event
 const PAUSE_REASONS = {
@@ -399,22 +398,8 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
     Actor.prototype.destroy.call(this);
   },
 
-  /**
-   * Tells if the thread actor has been initialized/attached on target creation
-   * by the server codebase. (And not late, from the frontend, by the TargetMixinFront class)
-   */
-  isAttached() {
-    return !!this.alreadyAttached;
-  },
-
   // Request handlers
   attach(options) {
-    // Note that the client avoids trying to call attach if already attached.
-    // But just in case, avoid any possible duplicate call to attach.
-    if (this.alreadyAttached) {
-      return;
-    }
-
     if (this.state === STATES.EXITED) {
       throw {
         error: "exited",
@@ -441,14 +426,12 @@ const ThreadActor = ActorClassWithSpec(threadSpec, {
       thread: this,
     });
 
+    this.dbg.enable();
     this.reconfigure(options);
 
-    // Switch state from DETACHED to RUNNING
+    // Set everything up so that breakpoint can work
     this._state = STATES.RUNNING;
 
-    this.alreadyAttached = true;
-    this.dbg.enable();
-
     // Notify the parent that we've finished attaching. If this is a worker
     // thread which was paused until attaching, this will allow content to
     // begin executing.

devtools/server/actors/watcher/WatchedDataHelpers.jsm

@@ -122,10 +122,3 @@ const WatchedDataHelpers = {
     return true;
   },
 };
-
-// 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.
-if (typeof module == "object") {
-  module.exports.WatchedDataHelpers = WatchedDataHelpers;
-}

devtools/shared/resources/tests/browser_resources_thread_states.js

@@ -10,28 +10,17 @@ const {
 } = require("devtools/shared/resources/resource-watcher");
 
 const BREAKPOINT_TEST_URL = URL_ROOT_SSL + "breakpoint_document.html";
-const REMOTE_IFRAME_URL =
-  "https://example.org/document-builder.sjs?html=" +
-  encodeURIComponent("<script>debugger;</script>");
 
 add_task(async function() {
-  // Check hitting the "debugger;" statement before and after calling
-  // watchResource(THREAD_TYPES). Both should break. First will
-  // be a cached resource and second will be a live one.
   await checkBreakpointBeforeWatchResources();
+
   await checkBreakpointAfterWatchResources();
 
-  // Check setting a real breakpoint on a given line
   await checkRealBreakpoint();
 
-  // Check the "pause on exception" setting
   await checkPauseOnException();
 
-  // Check an edge case where spamming setBreakpoints calls causes issues
   await checkSetBeforeWatch();
-
-  // Check debugger statement for (remote) iframes
-  await checkDebuggerStatementInIframes();
 });
 
 async function checkBreakpointBeforeWatchResources() {
@@ -426,95 +415,6 @@ async function checkSetBeforeWatch() {
   await client.close();
 }
 
-async function checkDebuggerStatementInIframes() {
-  info("Check whether ResourceWatcher gets breakpoint for (remote) iframes");
-
-  const tab = await addTab(BREAKPOINT_TEST_URL);
-
-  const { client, resourceWatcher, targetList } = await initResourceWatcher(
-    tab
-  );
-
-  info("Call watchResources");
-  const availableResources = [];
-  await resourceWatcher.watchResources([ResourceWatcher.TYPES.THREAD_STATE], {
-    onAvailable: resources => availableResources.push(...resources),
-  });
-
-  is(
-    availableResources.length,
-    0,
-    "Got no THREAD_STATE when calling watchResources"
-  );
-
-  info("Inject the iframe with an inline 'debugger' statement");
-  // Note that we do not wait for the resolution of spawn as it will be paused
-  SpecialPowers.spawn(
-    gBrowser.selectedBrowser,
-    [REMOTE_IFRAME_URL],
-    async function(url) {
-      const iframe = content.document.createElement("iframe");
-      iframe.src = url;
-      content.document.body.appendChild(iframe);
-    }
-  );
-
-  await waitFor(
-    () => availableResources.length == 1,
-    "Got the THREAD_STATE related to the iframe's debugger statement"
-  );
-  const threadState = availableResources.pop();
-
-  assertPausedResource(threadState, {
-    state: "paused",
-    why: {
-      type: "debuggerStatement",
-    },
-    frame: {
-      type: "global",
-      asyncCause: null,
-      state: "on-stack",
-      // this: object actor's form referring to `this` variable
-      displayName: "(global)",
-      // arguments: []
-      where: {
-        line: 1,
-        column: 0,
-      },
-    },
-  });
-
-  const iframeTarget = threadState.targetFront;
-  if (isFissionEnabled()) {
-    is(
-      iframeTarget.url,
-      REMOTE_IFRAME_URL,
-      "With fission, the pause is from the iframe's target"
-    );
-  } else {
-    is(
-      iframeTarget,
-      targetList.targetFront,
-      "Without fission, the pause is from the top level target"
-    );
-  }
-  const { threadFront } = iframeTarget;
-
-  await threadFront.resume();
-
-  await waitFor(
-    () => availableResources.length == 1,
-    "Wait until we receive the resumed event"
-  );
-
-  const resumed = availableResources.pop();
-
-  assertResumedResource(resumed);
-
-  targetList.destroy();
-  await client.close();
-}
-
 async function assertPausedResource(resource, expected) {
   is(
     resource.resourceType,

devtools/shared/resources/transformers/thread-states.js

@@ -9,23 +9,7 @@ const { Front, types } = require("devtools/shared/protocol.js");
 module.exports = function({ resource, watcherFront, targetFront }) {
   // only "paused" have a frame attribute, and legacy listeners are already passing a FrameFront
   if (resource.frame && !(resource.frame instanceof Front)) {
-    // Use ThreadFront as parent as debugger's commands.js expects FrameFront to be children
-    // of the ThreadFront.
-    resource.frame = types
-      .getType("frame")
-      .read(resource.frame, targetFront.threadFront);
-  }
-
-  // If we are using server side request (i.e. watcherFront is defined)
-  // Fake paused and resumed events as the thread front depends on them.
-  // We can't emit "EventEmitter" events, as ThreadFront uses `Front.before`
-  // to listen for paused and resumed. ("before" is part of protocol.js Front and not part of EventEmitter)
-  if (watcherFront) {
-    if (resource.state == "paused") {
-      targetFront.threadFront._beforePaused(resource);
-    } else if (resource.state == "resumed") {
-      targetFront.threadFront._beforeResumed(resource);
-    }
+    resource.frame = types.getType("frame").read(resource.frame, targetFront);
   }
 
   return resource;

devtools/shared/specs/thread.js

@@ -179,13 +179,6 @@ const threadSpec = generateActorSpec({
         logEventBreakpoints: Arg(0, "string"),
       },
     },
-
-    isAttached: {
-      request: {},
-      response: {
-        value: RetVal("boolean"),
-      },
-    },
   },
 });
 

devtools/shared/worker/loader.js

@@ -173,9 +173,7 @@ function WorkerDebuggerLoader(options) {
     }
 
     // If the url has no extension, use ".js" by default.
-    // Also allow loading JSMs, but they would need a shim in order to
-    // be loaded as a CommonJS module. (See WatchedDataHelpers.jsm)
-    return url.endsWith(".js") || url.endsWith(".jsm") ? url : url + ".js";
+    return url.endsWith(".js") ? url : url + ".js";
   }
 
   /**

tools/lint/file-whitespace.yml

@@ -21,6 +21,7 @@ file-whitespace:
         - devtools/client/inspector/markup/test
         - devtools/client/inspector/rules/test
         - devtools/client/inspector/test
+        - devtools/client/shared/test/doc_inline-debugger-statement.html
         # Excluded because tests were failing unexpectedly
         - devtools/client/styleeditor/test/sync_with_csp.css
         - devtools/client/webconsole/test/browser/test-message-categories-css-parser.css