Bug 1708243 - Part 2: stop using sender data from the child process r…

…=robwu,agi

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

browser/components/extensions/parent/.eslintrc.js

@@ -26,7 +26,6 @@ module.exports = {
     replaceUrlInTab: true,
     searchInitialized: true,
     sidebarActionFor: true,
-    tabGetSender: true,
     tabTracker: true,
     waitForTabLoaded: true,
     windowTracker: true,

browser/components/extensions/parent/ext-browser.js

@@ -44,34 +44,6 @@ function isPrivateTab(nativeTab) {
   return PrivateBrowsingUtils.isBrowserPrivate(nativeTab.linkedBrowser);
 }
 
-// This function is pretty tightly tied to Extension.jsm.
-// Its job is to fill in the |tab| property of the sender.
-const getSender = (extension, target, sender) => {
-  let tabId;
-  if ("tabId" in sender) {
-    // The message came from a privileged extension page running in a tab. In
-    // that case, it should include a tabId property (which is filled in by the
-    // page-open listener below).
-    tabId = sender.tabId;
-    delete sender.tabId;
-  } else if (
-    ExtensionCommon.instanceOf(target, "XULFrameElement") ||
-    ExtensionCommon.instanceOf(target, "HTMLIFrameElement")
-  ) {
-    tabId = tabTracker.getBrowserData(target).tabId;
-  }
-
-  if (tabId) {
-    let tab = extension.tabManager.get(tabId, null);
-    if (tab) {
-      sender.tab = tab.convert();
-    }
-  }
-};
-
-// Used by Extension.jsm
-global.tabGetSender = getSender;
-
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("uninstalling", (msg, extension) => {
   if (extension.uninstallURL) {

mobile/android/components/extensions/ext-android.js

@@ -599,31 +599,6 @@ extensions.on("startup", (type, extension) => {
   );
 });
 
-// This function is pretty tightly tied to Extension.jsm.
-// Its job is to fill in the |tab| property of the sender.
-const getSender = (extension, target, sender) => {
-  let tabId = -1;
-  if ("tabId" in sender) {
-    // The message came from a privileged extension page running in a tab. In
-    // that case, it should include a tabId property (which is filled in by the
-    // page-open listener below).
-    tabId = sender.tabId;
-    delete sender.tabId;
-  } else if (ChromeUtils.getClassName(target) == "XULFrameElement") {
-    tabId = tabTracker.getBrowserData(target).tabId;
-  }
-
-  if (tabId != null && tabId >= 0) {
-    const tab = extension.tabManager.get(tabId, null);
-    if (tab) {
-      sender.tab = tab.convert();
-    }
-  }
-};
-
-// Used by Extension.jsm
-global.tabGetSender = getSender;
-
 /* eslint-disable mozilla/balanced-listeners */
 extensions.on("page-shutdown", (type, context) => {
   if (context.viewType == "tab") {

mobile/android/modules/geckoview/GeckoViewWebExtension.jsm

@@ -224,7 +224,7 @@ class GeckoViewConnection {
     this.nativeApp = nativeApp;
     this.allowContentMessaging = allowContentMessaging;
 
-    if (!this.allowContentMessaging && !sender.verified) {
+    if (!allowContentMessaging && sender.envType !== "addon_child") {
       throw new Error(`Unexpected messaging sender: ${JSON.stringify(sender)}`);
     }
   }

toolkit/components/extensions/ConduitsParent.jsm

@@ -56,6 +56,10 @@ const { BaseConduit } = ChromeUtils.import(
   "resource://gre/modules/ConduitsChild.jsm"
 );
 
+const { WebNavigationFrames } = ChromeUtils.import(
+  "resource://gre/modules/WebNavigationFrames.jsm"
+);
+
 const BATCH_TIMEOUT_MS = 250;
 const ADDON_ENV = new Set(["addon_child", "devtools_child"]);
 
@@ -135,13 +139,23 @@ const Hub = {
   },
 
   /**
-   * Save info about a new remote conduit.
+   * Fill in common address fields knowable from the parent process.
    * @param {ConduitAddress} address
    * @param {ConduitsParent} actor
    */
-  recvConduitOpened(address, actor) {
+  fillInAddress(address, actor) {
     address.actor = actor;
     address.verified = this.verifyEnv(address);
+    address.frameId = WebNavigationFrames.getFrameId(actor.browsingContext);
+  },
+
+  /**
+   * Save info about a new remote conduit.
+   * @param {ConduitAddress} address
+   * @param {ConduitsParent} actor
+   */
+  recvConduitOpened(address, actor) {
+    this.fillInAddress(address, actor);
     this.remotes.set(address.id, address);
     this.byActor.get(actor).add(address);
   },

toolkit/components/extensions/ExtensionChild.jsm

@@ -305,11 +305,9 @@ class Port {
  * basics of sendMessage, onMessage, connect and onConnect.
  */
 class Messenger {
-  constructor(context, sender) {
+  constructor(context) {
     this.context = context;
     this.conduit = context.openConduit(this, {
-      url: sender.url,
-      frameId: sender.frameId,
       childId: context.childManager.id,
       query: ["NativeMessage", "RuntimeMessage", "PortConnect"],
       recv: ["RuntimeMessage", "PortConnect"],

toolkit/components/extensions/ExtensionContent.jsm

@@ -955,7 +955,7 @@ class ContentScriptContextChild extends BaseContext {
 }
 
 defineLazyGetter(ContentScriptContextChild.prototype, "messenger", function() {
-  return new Messenger(this, { frameId: this.frameId, url: this.url });
+  return new Messenger(this);
 });
 
 defineLazyGetter(

toolkit/components/extensions/ExtensionPageChild.jsm

@@ -31,11 +31,6 @@ ChromeUtils.defineModuleGetter(
   "Schemas",
   "resource://gre/modules/Schemas.jsm"
 );
-ChromeUtils.defineModuleGetter(
-  this,
-  "WebNavigationFrames",
-  "resource://gre/modules/WebNavigationFrames.jsm"
-);
 
 const CATEGORY_EXTENSION_SCRIPTS_ADDON = "webextension-scripts-addon";
 const CATEGORY_EXTENSION_SCRIPTS_DEVTOOLS = "webextension-scripts-devtools";
@@ -182,21 +177,13 @@ class ExtensionBaseContextChild extends BaseContext {
     this.setContentWindow(contentWindow);
     this.browsingContextId = contentWindow.docShell.browsingContext.id;
 
-    // This is the MessageSender property passed to extension.
-    let sender = { id: extension.id };
     if (viewType == "tab") {
-      sender.frameId = WebNavigationFrames.getFrameId(contentWindow);
-      sender.tabId = tabId;
       Object.defineProperty(this, "tabId", {
         value: tabId,
         enumerable: true,
         configurable: true,
       });
     }
-    if (uri) {
-      sender.url = uri.spec;
-    }
-    this.sender = sender;
 
     Schemas.exportLazyGetter(contentWindow, "browser", () => {
       let browserObj = Cu.createObjectIn(contentWindow);
@@ -255,7 +242,7 @@ class ExtensionBaseContextChild extends BaseContext {
 }
 
 defineLazyGetter(ExtensionBaseContextChild.prototype, "messenger", function() {
-  return new Messenger(this, this.sender);
+  return new Messenger(this);
 });
 
 class ExtensionPageContextChild extends ExtensionBaseContextChild {

toolkit/components/extensions/ExtensionParent.jsm

@@ -274,15 +274,11 @@ const ProxyMessenger = {
 
   openNative(nativeApp, sender) {
     let context = ParentAPIManager.getContextById(sender.childId);
-    let { extension } = context;
-    if (extension.hasPermission("geckoViewAddons")) {
-      let allowMessagingFromContent = extension.hasPermission(
-        "nativeMessagingFromContent"
-      );
+    if (context.extension.hasPermission("geckoViewAddons")) {
       return new GeckoViewConnection(
-        sender,
+        this.getSender(context, sender),
         nativeApp,
-        allowMessagingFromContent
+        context.extension.hasPermission("nativeMessagingFromContent")
       );
     } else if (sender.verified) {
       return new NativeApp(context, nativeApp);
@@ -294,11 +290,21 @@ const ProxyMessenger = {
     return this.openNative(nativeApp, sender).sendMessage(holder);
   },
 
-  getSender(extension, source) {
-    let { extensionId, envType, frameId, url, actor, id } = source;
-    let sender = { id: extensionId, envType, frameId, url, contextId: id };
-    let target = actor.browsingContext.top.embedderElement;
-    apiManager.global.tabGetSender(extension, target, sender);
+  getSender(context, source) {
+    let sender = {
+      contextId: source.id,
+      id: source.extensionId,
+      envType: source.envType,
+      frameId: source.frameId,
+      url: context.uri.spec,
+    };
+
+    let { tabId } = apiManager.global.tabTracker.getBrowserData(
+      source.actor.browsingContext.top.embedderElement
+    );
+    if (tabId > 0) {
+      sender.tab = context.extension.tabManager.get(tabId, null)?.convert();
+    }
     return sender;
   },
 
@@ -322,7 +328,8 @@ const ProxyMessenger = {
     }
     await extension.wakeupBackground?.();
 
-    arg.sender = this.getSender(extension, sender);
+    let context = ParentAPIManager.getContextById(sender.childId);
+    arg.sender = this.getSender(context, sender);
     arg.topBC = arg.tabId && this.getTopBrowsingContextId(arg.tabId);
     return arg.tabId ? "tab" : "messenger";
   },

toolkit/components/extensions/ExtensionTestCommon.jsm

@@ -26,6 +26,11 @@ ChromeUtils.defineModuleGetter(
   "AddonManager",
   "resource://gre/modules/AddonManager.jsm"
 );
+ChromeUtils.defineModuleGetter(
+  this,
+  "AppConstants",
+  "resource://gre/modules/AppConstants.jsm"
+);
 ChromeUtils.defineModuleGetter(
   this,
   "Extension",
@@ -455,6 +460,12 @@ ExtensionTestCommon = class ExtensionTestCommon {
     let fileURI = Services.io.newFileURI(file);
     let jarURI = Services.io.newURI("jar:" + fileURI.spec + "!/");
 
+    // An android-browser shared test, needs addon manager on GeckoView.
+    if (data.androidBrowserTest && AppConstants.platform === "android") {
+      data.useAddonManager ??= "permanent";
+      this.setExtensionID(data);
+    }
+
     // This may be "temporary" or "permanent".
     if (data.useAddonManager) {
       return new MockExtension(file, jarURI, data);

toolkit/components/extensions/test/mochitest/test_ext_tabs_sendMessage.html

@@ -13,11 +13,8 @@
 
 add_task(async function test_tabs_sendMessage_to_extension_page_frame() {
   let extension = ExtensionTestUtils.loadExtension({
-    useAddonManager: "permanent",
+    androidBrowserTest: true,
     manifest: {
-      applications: {
-        gecko: { id: "blah@android" },
-      },
       content_scripts: [{
         matches: ["http://mochi.test/*/file_sample.html?tabs.sendMessage"],
         js: ["cs.js"],
@@ -90,6 +87,68 @@
   await extension.unload();
 });
 
+add_task(async function test_tabs_sendMessage_using_frameId() {
+  let extension = ExtensionTestUtils.loadExtension({
+    androidBrowserTest: true,
+    manifest: {
+      content_scripts: [
+        {
+          matches: ["http://mochi.test/*/file_contains_iframe.html"],
+          run_at: "document_start",
+          js: ["cs_top.js"],
+        },
+        {
+          matches: ["http://example.org/*/file_contains_img.html"],
+          js: ["cs_iframe.js"],
+          all_frames: true,
+        },
+      ],
+    },
+
+    background() {
+      browser.runtime.onMessage.addListener(async (msg, sender) => {
+        let { tab, frameId } = sender;
+        browser.test.assertEq(msg, "cs_iframe_ready", "Iframe cs ready.");
+        browser.test.assertTrue(frameId > 0, "Not from the top frame.");
+
+        let response = await browser.tabs.sendMessage(tab.id, "msg");
+        browser.test.assertEq(response, "cs_top", "Top cs responded first.");
+
+        response = await browser.tabs.sendMessage(tab.id, "msg", { frameId });
+        browser.test.assertEq(response, "cs_iframe", "Iframe cs reponded.");
+
+        browser.test.sendMessage("done");
+      });
+      browser.test.sendMessage("ready");
+    },
+
+    files: {
+      "cs_top.js"() {
+        browser.test.log("Top content script loaded.")
+        browser.runtime.onMessage.addListener(async () => "cs_top");
+      },
+      "cs_iframe.js"() {
+        browser.test.log("Iframe content script loaded.")
+        browser.runtime.onMessage.addListener((msg, sender, sendResponse) => {
+          browser.test.log("Iframe content script received message.")
+          setTimeout(() => sendResponse("cs_iframe"), 100);
+          return true;
+        });
+        browser.runtime.sendMessage("cs_iframe_ready");
+      },
+    },
+  });
+
+  await extension.startup();
+  await extension.awaitMessage("ready");
+
+  let win = window.open("file_contains_iframe.html");
+  await extension.awaitMessage("done");
+  win.close();
+
+  await extension.unload();
+});
+
 </script>
 </body>
 </html>