Bug 1660892 - Ignore window objects related to remote iframes in thread actor. r=nchevobbe
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 26 Aug 2020 14:25:31 +0000
changeset 546582 28f5c2767938c5ff6e881d33c27eebd5689d90e0
parent 546581 e3f460a309d690faf1f5399ac34df380e154f32e
child 546583 def80b3ac3f69102911ad879de49161945557603
push id125111
push userapoirot@mozilla.com
push dateThu, 27 Aug 2020 08:57:21 +0000
treeherderautoland@def80b3ac3f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchevobbe
bugs1660892
milestone82.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 1660892 - Ignore window objects related to remote iframes in thread actor. r=nchevobbe Differential Revision: https://phabricator.services.mozilla.com/D88074
devtools/client/shared/test/browser_dbg_debugger-statement.js
devtools/client/shared/test/code_frame-script.js
devtools/client/shared/test/doc_inline-debugger-statement.html
devtools/client/shared/test/helper_workers.js
devtools/server/actors/utils/event-loop.js
--- a/devtools/client/shared/test/browser_dbg_debugger-statement.js
+++ b/devtools/client/shared/test/browser_dbg_debugger-statement.js
@@ -2,66 +2,164 @@
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
 /**
  * Tests the behavior of the debugger statement.
  */
 
-// Import helpers for the workers
-/* import-globals-from helper_workers.js */
-Services.scriptloader.loadSubScript(
-  "chrome://mochitests/content/browser/devtools/client/shared/test/helper_workers.js",
-  this
-);
-
-const TAB_URL = TEST_URI_ROOT + "doc_inline-debugger-statement.html";
+// 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;
 
-  const threadFront = await testEarlyDebuggerStatement(client, tab, target);
-  await testDebuggerStatement(client, tab, threadFront);
+  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 watcher = await target.getWatcher();
+    await watcher.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, tab, targetFront) {
+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.
-  callInTab(tab, "runDebuggerStatement");
+  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 and resume...
   const threadFront = await targetFront.attachThread();
   await threadFront.resume();
   ok(true, "Pause wasn't called before we've attached.");
 
   return threadFront;
 }
 
-async function testDebuggerStatement(client, tab, threadFront) {
-  const onPaused = new Promise(resolve => {
-    threadFront.on("paused", async packet => {
-      await threadFront.resume();
-      ok(true, "The pause handler was triggered on a debugger statement.");
-      resolve();
-    });
+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();
   });
 
-  // Reach around the debugging protocol and execute the debugger statement.
-  callInTab(tab, "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));
 
-  return onPaused;
+  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"
+  );
 }
--- a/devtools/client/shared/test/code_frame-script.js
+++ b/devtools/client/shared/test/code_frame-script.js
@@ -15,25 +15,16 @@ EventUtils._EU_Cc = Cc;
 EventUtils.navigator = content.navigator;
 EventUtils.KeyboardEvent = content.KeyboardEvent;
 loadSubScript("chrome://mochikit/content/tests/SimpleTest/EventUtils.js", EventUtils);
 
 dump("Frame script loaded.\n");
 
 var workers = {};
 
-this.call = function(name, args) {
-  dump("Calling function with name " + name + ".\n");
-
-  dump("args " + JSON.stringify(args) + "\n");
-  return XPCNativeWrapper
-    .unwrap(content)[name]
-    .apply(undefined, Cu.cloneInto(args, content));
-};
-
 this._eval = function(string) {
   dump("Evalling string.\n");
 
   return content.eval(string);
 };
 
 this.generateMouseClick = function(path) {
   dump("Generating mouse click.\n");
--- a/devtools/client/shared/test/doc_inline-debugger-statement.html
+++ b/devtools/client/shared/test/doc_inline-debugger-statement.html
@@ -9,17 +9,19 @@
   </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>
--- a/devtools/client/shared/test/helper_workers.js
+++ b/devtools/client/shared/test/helper_workers.js
@@ -72,22 +72,16 @@ function generateMouseClickInTab(tab, pa
 }
 
 function evalInTab(tab, string) {
   info("Evalling string in tab.");
 
   return jsonrpc(tab, "_eval", [string]);
 }
 
-function callInTab(tab, name) {
-  info("Calling function with name '" + name + "' in tab.");
-
-  return jsonrpc(tab, "call", [name, Array.prototype.slice.call(arguments, 2)]);
-}
-
 function connect(client) {
   info("Connecting client.");
   return client.connect();
 }
 
 function close(client) {
   info("Waiting for client to close.\n");
   return client.close();
--- a/devtools/server/actors/utils/event-loop.js
+++ b/devtools/server/actors/utils/event-loop.js
@@ -113,43 +113,61 @@ EventLoop.prototype = {
     }
     return false;
   },
 
   /**
    * Retrieve the list of all DOM Windows debugged by the current thread actor.
    */
   getAllWindowDebuggees() {
-    return (
-      this._thread.dbg
-        .getDebuggees()
-        .filter(debuggee => {
-          // Select only debuggee that relates to windows
-          // e.g. ignore sandboxes, jsm and such
-          return debuggee.class == "Window";
-        })
-        .map(debuggee => {
-          // Retrieve the JS reference for these windows
-          return debuggee.unsafeDereference();
-        })
-        // Ignore iframes as they will be paused automatically when pausing their
-        // owner top level document
-        .filter(window => {
-          try {
-            return window.top === window;
-          } catch (e) {
-            // Warn if this is throwing for an unknown reason, but suppress the
-            // exception regardless so that we can enter the nested event loop.
-            if (!Cu.isDeadWrapper(window) && !/not initialized/.test(e)) {
-              console.warn(`Exception in getAllWindowDebuggees: ${e}`);
-            }
-            return false;
+    return this._thread.dbg
+      .getDebuggees()
+      .filter(debuggee => {
+        // Select only debuggee that relates to windows
+        // e.g. ignore sandboxes, jsm and such
+        return debuggee.class == "Window";
+      })
+      .map(debuggee => {
+        // Retrieve the JS reference for these windows
+        return debuggee.unsafeDereference();
+      })
+
+      .filter(window => {
+        // Ignore document which have already been nuked,
+        // so navigated to another location and removed from memory completely.
+        if (Cu.isDeadWrapper(window)) {
+          return false;
+        }
+        // Also ignore document which are closed, as trying to access window.parent or top would throw NS_ERROR_NOT_INITIALIZED
+        if (window.closed) {
+          return false;
+        }
+        // Ignore remote iframes, which will be debugged by another thread actor,
+        // running in the remote process
+        if (Cu.isRemoteProxy(window)) {
+          return false;
+        }
+        // Accept "top remote iframe document":
+        // document of iframe whose immediate parent is in another process.
+        if (Cu.isRemoteProxy(window.parent) && !Cu.isRemoteProxy(window)) {
+          return true;
+        }
+        try {
+          // Ignore iframes running in the same process as their parent document,
+          // as they will be paused automatically when pausing their owner top level document
+          return window.top === window;
+        } catch (e) {
+          // Warn if this is throwing for an unknown reason, but suppress the
+          // exception regardless so that we can enter the nested event loop.
+          if (!/not initialized/.test(e)) {
+            console.warn(`Exception in getAllWindowDebuggees: ${e}`);
           }
-        })
-    );
+          return false;
+        }
+      });
   },
 
   /**
    * Prepare to enter a nested event loop by disabling debuggee events.
    */
   preNest() {
     const windows = [];
     // Disable events in all open windows.