Bug 1605027 - Create remote FrameTargetActor with the correct docShell r=ochameau
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 27 Dec 2019 14:33:05 +0000
changeset 508414 00f9b55d3ac35fa6e301a2874b5f4075207c4ae4
parent 508413 474211787da1410492d916a06c7bf5bced9426da
child 508415 57e5afe6aade3d0303b3644311d96af108df8a8b
push id36958
push useraiakab@mozilla.com
push dateFri, 27 Dec 2019 21:54:40 +0000
treeherdermozilla-central@509cd3b3ec22 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1605027
milestone73.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 1605027 - Create remote FrameTargetActor with the correct docShell r=ochameau Differential Revision: https://phabricator.services.mozilla.com/D57684
devtools/server/actors/targets/frame.js
devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm
devtools/server/startup/frame.js
devtools/server/tests/browser/browser.ini
devtools/server/tests/browser/browser_resource_list-remote-frames.js
devtools/server/tests/browser/doc_iframe.html
devtools/server/tests/browser/doc_iframe_content.html
--- a/devtools/server/actors/targets/frame.js
+++ b/devtools/server/actors/targets/frame.js
@@ -30,71 +30,69 @@ const { frameTargetSpec } = require("dev
  */
 const frameTargetPrototype = extend({}, browsingContextTargetPrototype);
 
 /**
  * Target actor for a frame / docShell in the content process.
  *
  * @param connection DebuggerServerConnection
  *        The conection to the client.
- * @param chromeGlobal
- *        The content script global holding |content| and |docShell| properties for a tab.
- * @param prefix
- *        the prefix used in protocol to create IDs for each actor.
- *        Used as ID identifying this particular target actor from the parent process.
+ * @param docShell
+ *        The |docShell| for the debugged frame.
  */
-frameTargetPrototype.initialize = function(connection, chromeGlobal) {
-  this._chromeGlobal = chromeGlobal;
-  BrowsingContextTargetActor.prototype.initialize.call(
-    this,
-    connection,
-    chromeGlobal
-  );
+frameTargetPrototype.initialize = function(connection, docShell) {
+  BrowsingContextTargetActor.prototype.initialize.call(this, connection);
+
+  // Retrieve the message manager from the provided docShell.
+  // Note that messageManager also has a docShell property, but in some
+  // situations docShell.messageManager.docShell points to a different docShell.
+  this._messageManager = docShell.messageManager;
+
   this.traits.reconfigure = false;
   this._sendForm = this._sendForm.bind(this);
-  this._chromeGlobal.addMessageListener("debug:form", this._sendForm);
+  this._messageManager.addMessageListener("debug:form", this._sendForm);
 
   Object.defineProperty(this, "docShell", {
-    value: this._chromeGlobal.docShell,
+    value: docShell,
     configurable: true,
   });
 };
 
 Object.defineProperty(frameTargetPrototype, "title", {
   get: function() {
     return this.window.document.title;
   },
   enumerable: true,
   configurable: true,
 });
 
 frameTargetPrototype.exit = function() {
   if (this._sendForm) {
     try {
-      this._chromeGlobal.removeMessageListener("debug:form", this._sendForm);
+      this._messageManager.removeMessageListener("debug:form", this._sendForm);
     } catch (e) {
       if (e.result != Cr.NS_ERROR_NULL_POINTER) {
         throw e;
       }
       // In some cases, especially when using messageManagers in non-e10s mode, we reach
       // this point with a dead messageManager which only throws errors but does not
       // seem to indicate in any other way that it is dead.
     }
     this._sendForm = null;
   }
 
   BrowsingContextTargetActor.prototype.exit.call(this);
 
-  this._chromeGlobal = null;
+  this._messageManager = null;
 };
 
 /**
  * On navigation events, our URL and/or title may change, so we update our
  * counterpart in the parent process that participates in the tab list.
  */
 frameTargetPrototype._sendForm = function() {
-  this._chromeGlobal.sendAsyncMessage("debug:form", this.form());
+  this._messageManager.sendAsyncMessage("debug:form", this.form());
 };
 
 exports.FrameTargetActor = ActorClassWithSpec(
   frameTargetSpec,
   frameTargetPrototype
 );
--- a/devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm
+++ b/devtools/server/connectors/js-window-actor/DevToolsFrameChild.jsm
@@ -65,18 +65,17 @@ class DevToolsFrameChild extends JSWindo
     // We are going to spawn a FrameTargetActor instance in the next few lines,
     // it is going to act like a root actor without being one.
     DebuggerServer.registerActors({ target: true });
     DebuggerServer.on("connectionchange", this._onConnectionChange);
 
     const connection = DebuggerServer.connectToParentWindowActor(prefix, this);
 
     // Create the actual target actor.
-    const { messageManager } = this.docShell;
-    const targetActor = new FrameTargetActor(connection, messageManager);
+    const targetActor = new FrameTargetActor(connection, this.docShell);
 
     // Add the newly created actor to the connection pool.
     const actorPool = new ActorPool(connection);
     actorPool.addActor(targetActor);
     connection.addActorPool(actorPool);
 
     return { connection, targetActor };
   }
--- a/devtools/server/startup/frame.js
+++ b/devtools/server/startup/frame.js
@@ -74,17 +74,20 @@ try {
           chromeGlobal,
           prefix,
           addonId
         );
       } else {
         const {
           FrameTargetActor,
         } = require("devtools/server/actors/targets/frame");
-        actor = new FrameTargetActor(conn, chromeGlobal);
+        const { docShell } = chromeGlobal;
+        // For a script loaded via loadFrameScript, the global is the content
+        // message manager.
+        actor = new FrameTargetActor(conn, docShell);
       }
 
       const actorPool = new ActorPool(conn);
       actorPool.addActor(actor);
       conn.addActorPool(actorPool);
 
       sendAsyncMessage("debug:actor", { actor: actor.form(), prefix: prefix });
     });
--- a/devtools/server/tests/browser/browser.ini
+++ b/devtools/server/tests/browser/browser.ini
@@ -16,16 +16,17 @@ support-files =
   doc_accessibility_text_label_audit_frame.html
   doc_accessibility_text_label_audit.html
   doc_accessibility.html
   doc_allocations.html
   doc_force_cc.html
   doc_force_gc.html
   doc_innerHTML.html
   doc_iframe.html
+  doc_iframe_content.html
   doc_perf.html
   error-actor.js
   grid.html
   inspectedwindow-reload-target.sjs
   inspector-search-data.html
   inspector-traversal-data.html
   inspector-shadow.html
   navigate-first.html
--- a/devtools/server/tests/browser/browser_resource_list-remote-frames.js
+++ b/devtools/server/tests/browser/browser_resource_list-remote-frames.js
@@ -15,34 +15,38 @@ add_task(async function() {
 });
 
 async function testLocalListFrames(tabTarget) {
   // at this point, tabTarget is the tab with two iframes, one nested inside
   // the other.
   // we should move this to descriptorFront.listRemoteFrames once we have
   // tabDescriptors
   const { frames } = await tabTarget.listRemoteFrames();
-  is(frames.length, 2, "Got two frames");
+  is(frames.length, 3, "Got three frames");
 
   info("Check that we can connect to the remote targets");
+  const frameTargets = [];
   for (const frame of frames) {
     const frameTarget = await frame.getTarget();
     ok(frameTarget && frameTarget.actor, "Valid frame target retrieved");
+    frameTargets.push(frameTarget);
   }
 
-  // However we can confirm that the newly created iframe is there.
-  const browser = gBrowser.selectedBrowser;
-  const oopID = await SpecialPowers.spawn(browser, [], async () => {
-    const oop = content.document.querySelector("iframe");
-    return oop.frameLoader.browsingContext.id;
-  });
-  ok(
-    frames.find(f => f.id === oopID),
-    "tabTarget.listRemoteFrames returns the oop frame descriptor"
-  );
+  // Check that we retrieved frame targets for the expected URLs.
+  const expectedUrls = [
+    "data:text/html,<iframe src='data:text/html,foo'></iframe>",
+    "data:text/html,foo",
+    "http://example.com/browser/devtools/server/tests/browser/doc_iframe_content.html",
+  ];
+  for (const url of expectedUrls) {
+    ok(
+      frameTargets.find(target => target.url === url),
+      "Found a frame target for the expected url " + url
+    );
+  }
 }
 async function testBrowserListFrames(tabTarget) {
   // Now, we can test against the entire browser. getMainProcess will return
   // a target for the parentProcess, and will be able to enumerate over all
   // the tabs, the remote iframe, and the pair of frames, one nested inside the other.
   const target = await tabTarget.client.mainRoot.getMainProcess();
   await getFrames(target, tabTarget);
 }
--- a/devtools/server/tests/browser/doc_iframe.html
+++ b/devtools/server/tests/browser/doc_iframe.html
@@ -5,11 +5,13 @@
 <html>
   <head>
     <meta charset="utf-8"/>
     <title>iframe test page</title>
   </head>
 
   <body>
     <iframe id="better-not-ask" src="data:text/html,<iframe src='data:text/html,foo'></iframe>"></iframe>
+    <!-- This page is loaded on an example.org subdomain, so we switch to .com -->
+    <iframe id="remote-frame" src="http://example.com/browser/devtools/server/tests/browser/doc_iframe_content.html"></iframe>
   </body>
 
 </html>
new file mode 100644
--- /dev/null
+++ b/devtools/server/tests/browser/doc_iframe_content.html
@@ -0,0 +1,14 @@
+<!-- Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/ -->
+<!DOCTYPE html>
+
+<html>
+  <head>
+    <meta charset="utf-8"/>
+    <title>Frame for browser_resource_list-remote-frames.js</title>
+  </head>
+
+  <body>
+    <div>Remote frame content</div>
+  </body>
+</html>