Bug 1491051 - Avoid awaiting a promise when all the content scripts are already precompiled and cached. r=robwu,mixedpuppy
authorLuca Greco <lgreco@mozilla.com>
Sat, 15 Sep 2018 18:42:23 +0000
changeset 436609 5d78ab715d4f
parent 436608 0c879e092971
child 436610 f8e1ccdf01a5
push id34650
push usernerli@mozilla.com
push date2018-09-16 09:49 +0000
treeherdermozilla-central@300d0f16cf6f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrobwu, mixedpuppy
bugs1491051
milestone64.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 1491051 - Avoid awaiting a promise when all the content scripts are already precompiled and cached. r=robwu,mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D5794
toolkit/components/extensions/ExtensionContent.jsm
toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js
toolkit/components/extensions/test/xpcshell/test_ext_userScripts.js
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -467,17 +467,20 @@ class Script {
         // the stylesheets on first load. We should fix this up if it does becomes
         // a problem.
         if (this.css.length > 0) {
           context.contentWindow.document.blockParsing(cssPromise, {blockScriptCreated: false});
         }
       }
     }
 
-    let scripts = await this.awaitCompiledScripts(context);
+    let scripts = this.getCompiledScripts(context);
+    if (scripts instanceof Promise) {
+      scripts = await scripts;
+    }
 
     let result;
 
     const {extension} = context;
 
     // The evaluations below may throw, in which case the promise will be
     // automatically rejected.
     ExtensionTelemetry.contentScriptInjection.stopwatchStart(extension, context);
@@ -492,34 +495,45 @@ class Script {
     } finally {
       ExtensionTelemetry.contentScriptInjection.stopwatchFinish(extension, context);
     }
 
     await cssPromise;
     return result;
   }
 
-  async awaitCompiledScripts(context) {
+  /**
+   *  Get the compiled scripts (if they are already precompiled and cached) or a promise which resolves
+   *  to the precompiled scripts (once they have been compiled and cached).
+   *
+   * @param {BaseContext} context
+   *        The document to block the parsing on, if the scripts are not yet precompiled and cached.
+   *
+   * @returns {Array<PreloadedScript> | Promise<Array<PreloadedScript>>}
+   *          Returns an array of preloaded scripts if they are already available, or a promise which
+   *          resolves to the array of the preloaded scripts once they are precompiled and cached.
+   */
+  getCompiledScripts(context) {
     let scriptPromises = this.compileScripts();
     let scripts = scriptPromises.map(promise => promise.script);
 
     // If not all scripts are already available in the cache, block
     // parsing and wait all promises to resolve.
     if (!scripts.every(script => script)) {
       let promise = Promise.all(scriptPromises);
 
       // If we're supposed to inject at the start of the document load,
       // and we haven't already missed that point, block further parsing
       // until the scripts have been loaded.
-      let {document} = context.contentWindow;
+      const {document} = context.contentWindow;
       if (this.runAt === "document_start" && document.readyState !== "complete") {
         document.blockParsing(promise, {blockScriptCreated: false});
       }
 
-      scripts = await promise;
+      return promise;
     }
 
     return scripts;
   }
 }
 
 // Represents a user script.
 class UserScript extends Script {
@@ -533,51 +547,40 @@ class UserScript extends Script {
     super(extension, matcher);
 
     // This is an opaque object that the extension provides, it is associated to
     // the particular userScript and it is passed as a parameter to the custom
     // userScripts APIs defined by the extension.
     this.scriptMetadata = matcher.userScriptOptions.scriptMetadata;
     this.apiScriptURL = extension.manifest.user_scripts && extension.manifest.user_scripts.api_script;
 
-    this.promiseAPIScript = null;
-    this.scriptPromises = null;
+    // Add the apiScript to the js scripts to compile.
+    if (this.apiScriptURL) {
+      this.js = [this.apiScriptURL].concat(this.js);
+    }
 
     // WeakMap<ContentScriptContextChild, Sandbox>
     this.sandboxes = new DefaultWeakMap((context) => {
       return this.createSandbox(context);
     });
   }
 
-  compileScripts() {
-    if (this.apiScriptURL && !this.promiseAPIScript) {
-      this.promiseAPIScript = this.scriptCache.get(this.apiScriptURL);
-    }
-
-    if (!this.scriptPromises) {
-      this.scriptPromises = this.js.map(url => this.scriptCache.get(url));
-    }
-
-    if (this.promiseAPIScript) {
-      return [this.promiseAPIScript, ...this.scriptPromises];
-    }
-
-    return this.scriptPromises;
-  }
-
   async inject(context) {
     const {extension} = context;
 
     DocumentManager.lazyInit();
 
-    let scripts = await this.awaitCompiledScripts(context);
+    let scripts = this.getCompiledScripts(context);
+    if (scripts instanceof Promise) {
+      scripts = await scripts;
+    }
 
     let apiScript, sandboxScripts;
 
-    if (this.promiseAPIScript) {
+    if (this.apiScriptURL) {
       [apiScript, ...sandboxScripts] = scripts;
     } else {
       sandboxScripts = scripts;
     }
 
     // Load and execute the API script once per context.
     if (apiScript) {
       context.executeAPIScript(apiScript);
--- a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript.js
@@ -153,26 +153,84 @@ add_task(async function test_contentscri
     },
   });
 
   await extension.startup();
 
   let url = `${BASE_URL}/file_document_open.html`;
   let contentPage = await ExtensionTestUtils.loadContentPage(url);
 
-  Assert.deepEqual(await extension.awaitMessage("content-script"),
-                   [url, true]);
+  let [pageURL, pageIsTop] = await extension.awaitMessage("content-script");
+
+  // Sometimes we get a content script load for the initial about:blank
+  // top level frame here, sometimes we don't. Either way is fine, as long as we
+  // don't get two loads into the same document.open() document.
+  if (pageURL === "about:blank") {
+    equal(pageIsTop, true);
+    [pageURL, pageIsTop] = await extension.awaitMessage("content-script");
+  }
+
+  Assert.deepEqual([pageURL, pageIsTop], [url, true]);
 
   let [frameURL, isTop] = await extension.awaitMessage("content-script");
   // Sometimes we get a content script load for the initial about:blank
-  // iframe here, sometimes we don't. Either way is fine, as long as we
-  // don't get two loads into the same document.open() document.
+  // iframe here, sometimes we don't.
   if (frameURL === "about:blank") {
     equal(isTop, false);
-
     [frameURL, isTop] = await extension.awaitMessage("content-script");
   }
 
   Assert.deepEqual([frameURL, isTop], [url, false]);
 
   await contentPage.close();
   await extension.unload();
 });
+
+// This test verify that a cached script is still able to catch the document
+// while it is still loading (when we do not block the document parsing as
+// we do for a non cached script).
+add_task(async function test_cached_contentscript_on_document_start() {
+  let extension =  ExtensionTestUtils.loadExtension({
+    manifest: {
+      content_scripts: [
+        {
+          "matches": ["http://localhost/*/file_document_open.html"],
+          "js": ["content_script.js"],
+          "run_at": "document_start",
+        },
+      ],
+    },
+
+    files: {
+      "content_script.js": `
+        browser.test.sendMessage("content-script-loaded", {
+          url: window.location.href,
+          documentReadyState: document.readyState,
+        });
+      `,
+    },
+  });
+
+  await extension.startup();
+
+  let url = `${BASE_URL}/file_document_open.html`;
+  let contentPage = await ExtensionTestUtils.loadContentPage(url);
+
+  let msg = await extension.awaitMessage("content-script-loaded");
+  Assert.deepEqual(msg, {
+    url,
+    documentReadyState: "loading",
+  }, "Got the expected url and document.readyState from a non cached script");
+
+  // Reload the page and check that the cached content script is still able to
+  // run on document_start.
+  await contentPage.loadURL(url);
+
+  let msgFromCached = await extension.awaitMessage("content-script-loaded");
+  Assert.deepEqual(msgFromCached, {
+    url,
+    documentReadyState: "loading",
+  }, "Got the expected url and document.readyState from a cached script");
+
+  await extension.unload();
+
+  await contentPage.close();
+});
--- a/toolkit/components/extensions/test/xpcshell/test_ext_userScripts.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_userScripts.js
@@ -149,16 +149,17 @@ add_task(async function test_userScripts
       js: [{
         code: `
           browser.test.sendMessage("page-loaded", {
             textContent: document.body.textContent,
             url: window.location.href,
           }); true;
         `,
       }],
+      runAt: "document_end",
       matches,
     });
 
     browser.test.sendMessage("background-ready");
   }
 
   let extensionData = {
     manifest: {
@@ -351,8 +352,82 @@ add_task(async function test_userScripts
     asyncAPIResult: "resolved_value",
     expectedError: "Only serializable parameters are supported",
   }, "Got the expected userScript API results");
 
   await extension.unload();
 
   await contentPage.close();
 });
+
+// This test verify that a cached script is still able to catch the document
+// while it is still loading (when we do not block the document parsing as
+// we do for a non cached script).
+add_task(async function test_cached_userScript_on_document_start() {
+  function apiScript() {
+    browser.userScripts.setScriptAPIs({
+      sendTestMessage([name, params]) {
+        return browser.test.sendMessage(name, params);
+      },
+    });
+  }
+
+  async function background() {
+    function userScript() {
+      this.sendTestMessage("user-script-loaded", {
+        url: window.location.href,
+        documentReadyState: document.readyState,
+      });
+    }
+
+    await browser.userScripts.register({
+      js: [{
+        code: `(${userScript})();`,
+      }],
+      runAt: "document_start",
+      matches: [
+        "http://localhost/*/file_sample.html",
+      ],
+    });
+
+    browser.test.sendMessage("user-script-registered");
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: [
+        "http://localhost/*/file_sample.html",
+      ],
+      user_scripts: {
+        api_script: "api-script.js",
+      },
+    },
+    background,
+    files: {
+      "api-script.js": apiScript,
+    },
+  });
+
+  await extension.startup();
+  await extension.awaitMessage("user-script-registered");
+
+  let url = `${BASE_URL}/file_sample.html`;
+  let contentPage = await ExtensionTestUtils.loadContentPage(url);
+
+  let msg = await extension.awaitMessage("user-script-loaded");
+  Assert.deepEqual(msg, {
+    url,
+    documentReadyState: "loading",
+  }, "Got the expected url and document.readyState from a non cached user script");
+
+  // Reload the page and check that the cached content script is still able to
+  // run on document_start.
+  await contentPage.loadURL(url);
+
+  let msgFromCached = await extension.awaitMessage("user-script-loaded");
+  Assert.deepEqual(msgFromCached, {
+    url,
+    documentReadyState: "loading",
+  }, "Got the expected url and document.readyState from a cached user script");
+
+  await contentPage.close();
+  await extension.unload();
+});