Backed out changeset aeaa527e6119 (bug 1587541) for mochitest failures on test_chrome_ext_contentscript_telemetry.html . CLOSED TREE
authorNarcis Beleuzu <nbeleuzu@mozilla.com>
Mon, 13 Jul 2020 05:28:48 +0300
changeset 540155 9a24ba7aab535843951830d1be95494522dd3a53
parent 540154 9cc546760752349288e91003be97d9d7e8a08d23
child 540156 2c8bc998c107bd6a6182b5fc4740fe77fc07cfca
push id121501
push usernbeleuzu@mozilla.com
push dateMon, 13 Jul 2020 02:44:06 +0000
treeherderautoland@2c8bc998c107 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1587541
milestone80.0a1
backs outaeaa527e6119fcd0db506892f9a0809df8d473a7
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
Backed out changeset aeaa527e6119 (bug 1587541) for mochitest failures on test_chrome_ext_contentscript_telemetry.html . CLOSED TREE
browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
toolkit/components/extensions/ExtensionContent.jsm
toolkit/components/extensions/ExtensionProcessScript.jsm
toolkit/components/extensions/parent/ext-tabs-base.js
toolkit/components/extensions/test/mochitest/test_ext_contentscript_fission_frame.html
toolkit/modules/ActorManagerParent.jsm
tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js
--- a/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
@@ -30,19 +30,16 @@ add_task(async function testExecuteScrip
 
   async function background() {
     try {
       let [tab] = await browser.tabs.query({
         active: true,
         currentWindow: true,
       });
       let frames = await browser.webNavigation.getAllFrames({ tabId: tab.id });
-      browser.test.assertEq(3, frames.length, "Expect exactly three frames");
-      browser.test.assertEq(0, frames[0].frameId, "Main frame has frameId:0");
-      browser.test.assertTrue(frames[1].frameId > 0, "Subframe has a valid id");
 
       browser.test.log(
         `FRAMES: ${frames[1].frameId} ${JSON.stringify(frames)}\n`
       );
       await Promise.all([
         browser.tabs
           .executeScript({
             code: "42",
@@ -349,17 +346,17 @@ add_task(async function testExecuteScrip
           .executeScript({
             code: "location.href;",
             frameId: frames[0].frameId,
           })
           .then(result => {
             browser.test.assertEq(1, result.length, "Expected one result");
             browser.test.assertTrue(
               /\/file_iframe_document\.html$/.test(result[0]),
-              `Result for main frame (frameId:0) is correct: ${result[0]}`
+              `Result for frameId[0] is correct: ${result[0]}`
             );
           }),
 
         browser.tabs
           .executeScript({
             code: "location.href;",
             frameId: frames[1].frameId,
           })
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -1,16 +1,16 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 /* 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";
 
-var EXPORTED_SYMBOLS = ["ExtensionContent", "ExtensionContentChild"];
+var EXPORTED_SYMBOLS = ["ExtensionContent"];
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   ExtensionProcessScript: "resource://gre/modules/ExtensionProcessScript.jsm",
@@ -1166,52 +1166,69 @@ var ExtensionContent = {
         tld,
         text,
         encoding,
       }).then(result => (result.language === "un" ? "und" : result.language));
     });
   },
 
   // Used to executeScript, insertCSS and removeCSS.
-  async handleActorExecute({ options, windows }) {
-    let policy = WebExtensionPolicy.getByID(options.extensionId);
-    let matcher = new WebExtensionContentScript(policy, options);
-
-    Object.assign(matcher, {
-      wantReturnValue: options.wantReturnValue,
-      removeCSS: options.removeCSS,
-      cssOrigin: options.cssOrigin,
-      jsCode: options.jsCode,
-    });
-    let script = contentScripts.get(matcher);
-
-    // Add the cssCode to the script, so that it can be converted into a cached URL.
-    await script.addCSSCode(options.cssCode);
-    delete options.cssCode;
-
-    const executeInWin = innerId => {
-      let wg = WindowGlobalChild.getByInnerWindowId(innerId);
-      let bc = wg && !wg.isClosed && wg.isCurrentGlobal && wg.browsingContext;
-
-      if (bc && script.matchesWindow(bc.window)) {
-        return script.injectInto(bc.window);
+  async handleExtensionExecute(global, target, options, script) {
+    let executeInWin = window => {
+      if (script.matchesWindow(window)) {
+        return script.injectInto(window);
       }
+      return null;
     };
 
-    let all = Promise.all(windows.map(executeInWin).filter(p => p));
-    let result = await all.catch(e => Promise.reject({ message: e.message }));
+    let promises;
+    try {
+      promises = Array.from(
+        this.enumerateWindows(global.docShell),
+        executeInWin
+      ).filter(promise => promise);
+    } catch (e) {
+      Cu.reportError(e);
+      return Promise.reject({ message: "An unexpected error occurred" });
+    }
+
+    if (!promises.length) {
+      if (options.frameID) {
+        return Promise.reject({
+          message: `Frame not found, or missing host permission`,
+        });
+      }
+
+      let frames = options.allFrames ? ", and any iframes" : "";
+      return Promise.reject({
+        message: `Missing host permission for the tab${frames}`,
+      });
+    }
+    if (!options.allFrames && promises.length > 1) {
+      return Promise.reject({
+        message: `Internal error: Script matched multiple windows`,
+      });
+    }
+
+    let result = await Promise.all(promises);
 
     try {
-      // Check if the result can be structured-cloned before sending back.
-      return Cu.cloneInto(result, this);
+      // Make sure we can structured-clone the result value before
+      // we try to send it back over the message manager.
+      Cu.cloneInto(result, target);
     } catch (e) {
-      let path = options.jsPaths.slice(-1)[0] ?? "<anonymous code>";
-      let message = `Script '${path}' result is non-structured-clonable data`;
-      return Promise.reject({ message, fileName: path });
+      const { jsPaths } = options;
+      const fileName = jsPaths.length
+        ? jsPaths[jsPaths.length - 1]
+        : "<anonymous code>";
+      const message = `Script '${fileName}' result is non-structured-clonable data`;
+      return Promise.reject({ message, fileName });
     }
+
+    return result;
   },
 
   handleWebNavigationGetFrame(global, { frameId }) {
     return WebNavigationFrames.getFrame(global.docShell, frameId);
   },
 
   handleWebNavigationGetAllFrames(global) {
     return WebNavigationFrames.getAllFrames(global.docShell);
@@ -1223,16 +1240,40 @@ var ExtensionContent = {
         return this.handleExtensionCapture(
           global,
           data.width,
           data.height,
           data.options
         );
       case "Extension:DetectLanguage":
         return this.handleDetectLanguage(global, target);
+      case "Extension:Execute":
+        let policy = WebExtensionPolicy.getByID(recipient.extensionId);
+
+        let matcher = new WebExtensionContentScript(policy, data.options);
+
+        Object.assign(matcher, {
+          wantReturnValue: data.options.wantReturnValue,
+          removeCSS: data.options.removeCSS,
+          cssOrigin: data.options.cssOrigin,
+          jsCode: data.options.jsCode,
+        });
+
+        let script = contentScripts.get(matcher);
+
+        // Add the cssCode to the script, so that it can be converted into a cached URL.
+        await script.addCSSCode(data.options.cssCode);
+        delete data.options.cssCode;
+
+        return this.handleExtensionExecute(
+          global,
+          target,
+          data.options,
+          script
+        );
       case "WebNavigation:GetFrame":
         return this.handleWebNavigationGetFrame(global, data.options);
       case "WebNavigation:GetAllFrames":
         return this.handleWebNavigationGetAllFrames(global);
     }
     return null;
   },
 
@@ -1249,23 +1290,8 @@ var ExtensionContent = {
         yield docShell.domWindow;
       } catch (e) {
         // This can fail if the docShell is being destroyed, so just
         // ignore the error.
       }
     }
   },
 };
-
-/**
- * Child side of the ExtensionContent process actor, handles some tabs.* APIs.
- */
-class ExtensionContentChild extends JSProcessActorChild {
-  receiveMessage({ name, data }) {
-    if (!isContentScriptProcess) {
-      return;
-    }
-    switch (name) {
-      case "Execute":
-        return ExtensionContent.handleActorExecute(data);
-    }
-  }
-}
--- a/toolkit/components/extensions/ExtensionProcessScript.jsm
+++ b/toolkit/components/extensions/ExtensionProcessScript.jsm
@@ -66,16 +66,17 @@ class ExtensionGlobal {
   constructor(global) {
     this.global = global;
     this.global.addMessageListener("Extension:SetFrameData", this);
 
     this.frameData = null;
 
     MessageChannel.addListener(global, "Extension:Capture", this);
     MessageChannel.addListener(global, "Extension:DetectLanguage", this);
+    MessageChannel.addListener(global, "Extension:Execute", this);
     MessageChannel.addListener(global, "WebNavigation:GetFrame", this);
     MessageChannel.addListener(global, "WebNavigation:GetAllFrames", this);
   }
 
   get messageFilterStrict() {
     return {
       innerWindowID: getInnerWindowID(this.global.content),
     };
@@ -98,16 +99,21 @@ class ExtensionGlobal {
         } else {
           this.frameData = data;
         }
         if (data.viewType && WebExtensionPolicy.isExtensionProcess) {
           ExtensionPageChild.expectViewLoad(this.global, data.viewType);
         }
         return;
     }
+    // Prevent script compilation in the parent process when we would never
+    // use them.
+    if (!isContentScriptProcess && messageName === "Extension:Execute") {
+      return;
+    }
 
     // SetFrameData does not have a recipient extension, or it would be
     // an extension process. Anything following this point must have
     // a recipient extension, so check access to the window.
     let policy = WebExtensionPolicy.getByID(recipient.extensionId);
     if (!policy.canAccessWindow(this.global.content)) {
       throw new Error("Extension cannot access window");
     }
--- a/toolkit/components/extensions/parent/ext-tabs-base.js
+++ b/toolkit/components/extensions/parent/ext-tabs-base.js
@@ -304,25 +304,16 @@ class TabBase {
    *        @readonly
    *        @abstract
    */
   get browser() {
     throw new Error("Not implemented");
   }
 
   /**
-   * @property {BrowsingContext} browsingContext
-   *        Returns the BrowsingContext for the given tab.
-   *        @readonly
-   */
-  get browsingContext() {
-    return this.browser?.browsingContext;
-  }
-
-  /**
    * @property {FrameLoader} frameLoader
    *        Returns the frameloader for the given tab.
    *        @readonly
    */
   get frameLoader() {
     return this.browser && this.browser.frameLoader;
   }
 
@@ -682,72 +673,16 @@ class TabBase {
         }
       }
     }
 
     return result;
   }
 
   /**
-   * Query each content process hosting subframes of the tab, return results.
-   * @param {string} message
-   * @param {object} options
-   * @param {number} options.frameID
-   * @param {boolean} options.allFrames
-   * @returns {Promise[]}
-   */
-  async queryContent(message, options) {
-    let { allFrames, frameID } = options;
-
-    /** @type {Map<nsIDOMProcessParent, innerWindowId[]>} */
-    let byProcess = new DefaultMap(() => []);
-
-    // Recursively walk the tab's BC tree, find all frames, group by process.
-    function visit(bc) {
-      let win = bc.currentWindowGlobal;
-      if (win?.domProcess && (!frameID || frameID === bc.id)) {
-        byProcess.get(win.domProcess).push(win.innerWindowId);
-      }
-      if (allFrames || (frameID && !byProcess.size)) {
-        bc.children.forEach(visit);
-      }
-    }
-    visit(this.browsingContext);
-
-    let promises = Array.from(byProcess.entries(), ([proc, windows]) =>
-      proc.getActor("ExtensionContent").sendQuery(message, { windows, options })
-    );
-
-    let results = await Promise.all(promises).catch(err => {
-      if (err.name === "DataCloneError") {
-        let fileName = options.jsPaths.slice(-1)[0] || "<anonymous code>";
-        let message = `Script '${fileName}' result is non-structured-clonable data`;
-        return Promise.reject({ message, fileName });
-      }
-      throw err;
-    });
-    results = results.flat();
-
-    if (!results.length) {
-      if (frameID) {
-        throw new ExtensionError("Frame not found, or missing host permission");
-      }
-
-      let frames = allFrames ? ", and any iframes" : "";
-      throw new ExtensionError(`Missing host permission for the tab${frames}`);
-    }
-
-    if (!allFrames && results.length > 1) {
-      throw new ExtensionError("Internal error: multiple windows matched");
-    }
-
-    return results;
-  }
-
-  /**
    * Inserts a script or stylesheet in the given tab, and returns a promise
    * which resolves when the operation has completed.
    *
    * @param {BaseContext} context
    *        The extension context for which to perform the injection.
    * @param {InjectDetails} details
    *        The InjectDetails object, specifying what to inject, where, and
    *        when.
@@ -761,17 +696,16 @@ class TabBase {
    *        Resolves to the result of the execution, once it has completed.
    * @private
    */
   _execute(context, details, kind, method) {
     let options = {
       jsPaths: [],
       cssPaths: [],
       removeCSS: method == "removeCSS",
-      extensionId: context.extension.id,
     };
 
     // We require a `code` or a `file` property, but we can't accept both.
     if ((details.code === null) == (details.file === null)) {
       return Promise.reject({
         message: `${method} requires either a 'code' or a 'file' property, but not both`,
       });
     }
@@ -815,17 +749,18 @@ class TabBase {
     }
     if (details.cssOrigin !== null) {
       options.cssOrigin = details.cssOrigin;
     } else {
       options.cssOrigin = "author";
     }
 
     options.wantReturnValue = true;
-    return this.queryContent("Execute", options);
+
+    return this.sendMessage(context, "Extension:Execute", { options });
   }
 
   /**
    * Executes a script in the tab's content window, and returns a Promise which
    * resolves to the result of the evaluation, or rejects to the value of any
    * error the injection generates.
    *
    * @param {BaseContext} context
--- a/toolkit/components/extensions/test/mochitest/test_ext_contentscript_fission_frame.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_contentscript_fission_frame.html
@@ -13,47 +13,26 @@ add_task(async function test_content_scr
 
   const extension = ExtensionTestUtils.loadExtension({
     manifest: {
       content_scripts: [{
         matches: ["http://example.net/*/file_sample.html"],
         all_frames: true,
         js: ["cs.js"],
       }],
-      permissions: ["http://example.net/"],
     },
 
     background() {
       browser.runtime.onConnect.addListener(port => {
         port.onMessage.addListener(async num => {
           let { tab, url, frameId } = port.sender;
 
           browser.test.assertTrue(frameId > 0, "sender frameId is ok");
           browser.test.assertTrue(url.endsWith("file_sample.html"), "url is ok");
 
-          let shared = await browser.tabs.executeScript(tab.id, {
-            allFrames: true,
-            code: `window.sharedVal`,
-          });
-          browser.test.assertEq(shared[0], 357, "CS runs in a shared Sandbox");
-
-          let code = "does.not.exist";
-          await browser.test.assertRejects(
-            browser.tabs.executeScript(tab.id, { allFrames: true, code }),
-            /does is not defined/,
-            "Got the expected rejection from tabs.executeScript"
-          );
-
-          code = "() => {}";
-          await browser.test.assertRejects(
-            browser.tabs.executeScript(tab.id, { allFrames: true, code }),
-            /Script .* result is non-structured-clonable data/,
-            "Got the expected rejection from tabs.executeScript"
-          );
-
           let result = await browser.tabs.sendMessage(tab.id, num);
           port.postMessage(result);
           port.disconnect();
         });
       });
     },
 
     files: {
@@ -66,18 +45,16 @@ add_task(async function test_content_scr
         browser.test.assertEq(manifest.name, "Generated extension");
 
         browser.runtime.onMessage.addListener(async num => {
           browser.test.log("content script received tabs.sendMessage");
           return num * 3;
         })
 
         let response;
-        window.sharedVal = 357;
-
         let port = browser.runtime.connect();
         port.onMessage.addListener(num => {
           response = num;
         });
         port.onDisconnect.addListener(() => {
           browser.test.assertEq(response, 21, "Got correct response");
           browser.test.notifyPass();
         });
--- a/toolkit/modules/ActorManagerParent.jsm
+++ b/toolkit/modules/ActorManagerParent.jsm
@@ -46,21 +46,16 @@ let JSPROCESSACTORS = {
   ContentPrefs: {
     parent: {
       moduleURI: "resource://gre/modules/ContentPrefServiceParent.jsm",
     },
     child: {
       moduleURI: "resource://gre/modules/ContentPrefServiceChild.jsm",
     },
   },
-  ExtensionContent: {
-    child: {
-      moduleURI: "resource://gre/modules/ExtensionContent.jsm",
-    },
-  },
 };
 
 /**
  * Fission-compatible JSWindowActor implementations.
  * Each actor options object takes the form of a WindowActorOptions dictionary.
  * Detailed documentation of these options is in dom/docs/Fission.rst,
  * available at https://firefox-source-docs.mozilla.org/dom/Fission.html#jswindowactor
  */
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/environments/privileged.js
@@ -735,18 +735,16 @@ module.exports = {
     WebGPUTextureUsage: false,
     WebGPUTextureView: false,
     WebGPUVertexFormat: false,
     WebKitCSSMatrix: false,
     WebSocket: false,
     WebrtcGlobalInformation: false,
     WheelEvent: false,
     Window: false,
-    WindowGlobalChild: false,
-    WindowGlobalParent: false,
     WindowRoot: false,
     Worker: false,
     Worklet: false,
     XMLDocument: false,
     XMLHttpRequest: false,
     XMLHttpRequestEventTarget: false,
     XMLHttpRequestUpload: false,
     XMLSerializer: false,