Bug 1491388 - Clear userScripts revoked urls and refresh sharedData contentScripts on register/unregister. r=mixedpuppy
authorLuca Greco <lgreco@mozilla.com>
Sat, 15 Sep 2018 20:38:43 +0000
changeset 436610 f8e1ccdf01a5
parent 436609 5d78ab715d4f
child 436611 9a379342cfff
push id34650
push usernerli@mozilla.com
push dateSun, 16 Sep 2018 09:49:57 +0000
treeherdermozilla-central@300d0f16cf6f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1491388
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 1491388 - Clear userScripts revoked urls and refresh sharedData contentScripts on register/unregister. r=mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D5885
toolkit/components/extensions/child/ext-userScripts.js
toolkit/components/extensions/parent/ext-userScripts.js
toolkit/components/extensions/test/xpcshell/test_ext_userScripts.js
--- a/toolkit/components/extensions/child/ext-userScripts.js
+++ b/toolkit/components/extensions/child/ext-userScripts.js
@@ -2,16 +2,17 @@
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 Cu.importGlobalProperties(["crypto", "TextEncoder"]);
 
 var {
   DefaultMap,
   ExtensionError,
+  getUniqueId,
 } = ExtensionUtils;
 
 /**
  * Represents a registered userScript in the child extension process.
  *
  * @param {ExtensionPageContextChild} context
  *        The extension context which has registered the user script.
  * @param {string} scriptId
@@ -60,68 +61,71 @@ this.userScripts = class extends Extensi
     const blobURLsByHash = new Map();
 
     // Keep track of the userScript that are sharing the same blob urls,
     // so that we can revoke any blob url that is not used by a registered
     // userScripts:
     //   Map<blobURL, Set<scriptId>>
     const userScriptsByBlobURL = new DefaultMap(() => new Set());
 
-    function trackBlobURLs(scriptId, options) {
-      for (let url of options.js) {
-        if (userScriptsByBlobURL.has(url)) {
-          userScriptsByBlobURL.get(url).add(scriptId);
-        }
-      }
-    }
+    function revokeBlobURLs(scriptId, options) {
+      let revokedUrls = new Set();
 
-    function revokeBlobURLs(scriptId, options) {
       for (let url of options.js) {
         if (userScriptsByBlobURL.has(url)) {
           let scriptIds = userScriptsByBlobURL.get(url);
           scriptIds.delete(scriptId);
+
           if (scriptIds.size === 0) {
+            revokedUrls.add(url);
             userScriptsByBlobURL.delete(url);
             context.cloneScope.URL.revokeObjectURL(url);
           }
         }
       }
+
+      // Remove all the removed urls from the map of known computed hashes.
+      for (let [hash, url] of blobURLsByHash) {
+        if (revokedUrls.has(url)) {
+          blobURLsByHash.delete(hash);
+        }
+      }
     }
 
     // Convert a script code string into a blob URL (and use a cached one
     // if the script hash is already associated to a blob URL).
-    const getBlobURL = async (text) => {
+    const getBlobURL = async (text, scriptId) => {
       // Compute the hash of the js code string and reuse the blob url if we already have
       // for the same hash.
       const buffer = await crypto.subtle.digest("SHA-1", new TextEncoder().encode(text));
       const hash = String.fromCharCode(...new Uint16Array(buffer));
 
       let blobURL = blobURLsByHash.get(hash);
 
       if (blobURL) {
+        userScriptsByBlobURL.get(blobURL).add(scriptId);
         return blobURL;
       }
 
       const blob = new context.cloneScope.Blob([text], {type: "text/javascript"});
       blobURL = context.cloneScope.URL.createObjectURL(blob);
 
       // Start to track this blob URL.
-      userScriptsByBlobURL.get(blobURL);
+      userScriptsByBlobURL.get(blobURL).add(scriptId);
 
       blobURLsByHash.set(hash, blobURL);
 
       return blobURL;
     };
 
     function convertToAPIObject(scriptId, options) {
       const registeredScript = new UserScriptChild({
         context, scriptId,
         onScriptUnregister: () => revokeBlobURLs(scriptId, options),
       });
-      trackBlobURLs(scriptId, options);
 
       const scriptAPI = Cu.cloneInto(registeredScript.api(), context.cloneScope,
                                      {cloneFunctions: true});
       return scriptAPI;
     }
 
     // Revoke all the created blob urls once the context is destroyed.
     context.callOnClose({
@@ -134,23 +138,24 @@ this.userScripts = class extends Extensi
           context.cloneScope.URL.revokeObjectURL(blobURL);
         }
       },
     });
 
     return {
       userScripts: {
         register(options) {
+          let scriptId = getUniqueId();
           return context.cloneScope.Promise.resolve().then(async () => {
+            options.scriptId = scriptId;
             options.js = await Promise.all(options.js.map(js => {
-              return js.file || getBlobURL(js.code);
+              return js.file || getBlobURL(js.code, scriptId);
             }));
 
-            const scriptId = await context.childManager.callParentAsyncFunction(
-              "userScripts.register", [options]);
+            await context.childManager.callParentAsyncFunction("userScripts.register", [options]);
 
             return convertToAPIObject(scriptId, options);
           });
         },
         setScriptAPIs(exportedAPIMethods) {
           context.setUserScriptAPIs(exportedAPIMethods);
         },
       },
--- a/toolkit/components/extensions/parent/ext-userScripts.js
+++ b/toolkit/components/extensions/parent/ext-userScripts.js
@@ -1,30 +1,29 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 ChromeUtils.import("resource://gre/modules/ExtensionUtils.jsm");
 
 var {
   ExtensionError,
-  getUniqueId,
 } = ExtensionUtils;
 
 /**
  * Represents (in the main browser process) a user script.
  *
  * @param {UserScriptOptions} details
  *        The options object related to the user script
  *        (which has the properties described in the user_scripts.json
  *        JSON API schema file).
  */
 class UserScriptParent {
   constructor(details) {
-    this.scriptId = getUniqueId();
+    this.scriptId = details.scriptId;
     this.options = this._convertOptions(details);
   }
 
   destroy() {
     if (this.destroyed) {
       throw new Error("Unable to destroy UserScriptParent twice");
     }
 
@@ -64,27 +63,33 @@ this.userScripts = class extends Extensi
   }
 
   getAPI(context) {
     const {extension} = context;
 
     // Set of the scriptIds registered from this context.
     const registeredScriptIds = new Set();
 
-    function unregisterContentScripts(scriptIds) {
-      for (let scriptId of registeredScriptIds) {
+    const unregisterContentScripts = (scriptIds) => {
+      if (scriptIds.length === 0) {
+        return Promise.resolve();
+      }
+
+      for (let scriptId of scriptIds) {
+        registeredScriptIds.delete(scriptId);
         extension.registeredContentScripts.delete(scriptId);
         this.userScriptsMap.delete(scriptId);
       }
+      extension.updateContentScripts();
 
       return context.extension.broadcast("Extension:UnregisterContentScripts", {
         id: context.extension.id,
         scriptIds,
       });
-    }
+    };
 
     // Unregister all the scriptId related to a context when it is closed,
     // and revoke all the created blob urls once the context is destroyed.
     context.callOnClose({
       close() {
         unregisterContentScripts(Array.from(registeredScriptIds));
       },
     });
@@ -107,16 +112,17 @@ this.userScripts = class extends Extensi
 
           await extension.broadcast("Extension:RegisterContentScript", {
             id: extension.id,
             options: scriptOptions,
             scriptId,
           });
 
           extension.registeredContentScripts.set(scriptId, scriptOptions);
+          extension.updateContentScripts();
 
           return scriptId;
         },
 
         // This method is not available to the extension code, the extension code
         // doesn't have access to the internally used scriptId, on the contrary
         // the extension code will call script.unregister on the script API object
         // that is resolved from the register API method returned promise.
--- a/toolkit/components/extensions/test/xpcshell/test_ext_userScripts.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_userScripts.js
@@ -106,63 +106,82 @@ add_task(async function test_userScripts
 // Test that userScripts sandboxes:
 // - can be registered/unregistered from an extension page
 // - have no WebExtensions APIs available
 // - are able to access the target window and document
 add_task(async function test_userScripts_no_webext_apis() {
   async function background() {
     const matches = ["http://localhost/*/file_sample.html"];
 
-    const script = await browser.userScripts.register({
-      js: [{
+    const sharedCode = {code: "console.log(\"js code shared by multiple userScripts\");"};
+
+    let script = await browser.userScripts.register({
+      js: [sharedCode, {
         code: `
-          const webextAPINamespaces = this.browser ? Object.keys(this.browser) : undefined;
-          document.body.innerHTML = "userScript loaded - " + JSON.stringify(webextAPINamespaces);
+          window.addEventListener("load", () => {
+            const webextAPINamespaces = this.browser ? Object.keys(this.browser) : undefined;
+            document.body.innerHTML = "userScript loaded - " + JSON.stringify(webextAPINamespaces);
+          }, {once: true});
         `,
       }],
-      runAt: "document_end",
+      runAt: "document_start",
+      matches,
+      scriptMetadata: {
+        name: "test-user-script",
+        arrayProperty: ["el1"],
+        objectProperty: {nestedProp: "nestedValue"},
+        nullProperty: null,
+      },
+    });
+
+    // Unregister and then register the same js code again, to verify that the last registered
+    // userScript doesn't get assigned a revoked blob url (otherwise Extensioncontent.jsm
+    // ScriptCache raises an error because it fails to compile the revoked blob url and the user
+    // script will never be loaded).
+    script.unregister();
+    script = await browser.userScripts.register({
+      js: [sharedCode, {
+        code: `
+          window.addEventListener("load", () => {
+            const webextAPINamespaces = this.browser ? Object.keys(this.browser) : undefined;
+            document.body.innerHTML = "userScript loaded - " + JSON.stringify(webextAPINamespaces);
+          }, {once: true});
+        `,
+      }],
+      runAt: "document_start",
       matches,
       scriptMetadata: {
         name: "test-user-script",
         arrayProperty: ["el1"],
         objectProperty: {nestedProp: "nestedValue"},
         nullProperty: null,
       },
     });
 
     const scriptToRemove = await browser.userScripts.register({
-      js: [{
-        code: 'document.body.innerHTML = "unexpected unregistered userScript loaded";',
+      js: [sharedCode, {
+        code: `
+          window.addEventListener("load", () => {
+            document.body.innerHTML = "unexpected unregistered userScript loaded";
+          }, {once: true});
+        `,
       }],
-      runAt: "document_end",
+      runAt: "document_start",
       matches,
       scriptMetadata: {
         name: "user-script-to-remove",
       },
     });
 
     browser.test.assertTrue("unregister" in script,
                             "Got an unregister method on the userScript API object");
 
     // Remove the last registered user script.
     await scriptToRemove.unregister();
 
-    await browser.contentScripts.register({
-      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: {
       permissions: [
         "http://localhost/*/file_sample.html",
       ],
@@ -176,36 +195,49 @@ add_task(async function test_userScripts
 
   await extension.awaitMessage("background-ready");
 
   // Test in an existing process (where the registered userScripts has been received from the
   // Extension:RegisterContentScript message sent to all the processes).
   info("Test content script loaded in a process created before any registered userScript");
   let url = `${BASE_URL}/file_sample.html#remote-false`;
   let contentPage = await ExtensionTestUtils.loadContentPage(url, {remote: false});
-  const reply = await extension.awaitMessage("page-loaded");
-  Assert.deepEqual(reply, {
+  let result = await contentPage.spawn(undefined, async () => {
+    return {
+      textContent: this.content.document.body.textContent,
+      url: this.content.location.href,
+      readyState: this.content.document.readyState,
+    };
+  });
+  Assert.deepEqual(result, {
     textContent: "userScript loaded - undefined",
     url,
+    readyState: "complete",
   }, "The userScript executed on the expected url and no access to the WebExtensions APIs");
   await contentPage.close();
 
   // Test in a new process (where the registered userScripts has to be retrieved from the extension
   // representation from the shared memory data).
   // NOTE: this part is currently skipped on Android, where e10s content is not yet supported and
   // the xpcshell test crash when we create contentPage2 with `remote = true`.
   if (ExtensionTestUtils.remoteContentScripts) {
     info("Test content script loaded in a process created after the userScript has been registered");
     let url2 = `${BASE_URL}/file_sample.html#remote-true`;
     let contentPage2 = await ExtensionTestUtils.loadContentPage(url2, {remote: true});
-    // Load an url that matches and check that the userScripts has been loaded.
-    const reply2 = await extension.awaitMessage("page-loaded");
-    Assert.deepEqual(reply2, {
+    let result2 = await contentPage2.spawn(undefined, async () => {
+      return {
+        textContent: this.content.document.body.textContent,
+        url: this.content.location.href,
+        readyState: this.content.document.readyState,
+      };
+    });
+    Assert.deepEqual(result2, {
       textContent: "userScript loaded - undefined",
       url: url2,
+      readyState: "complete",
     }, "The userScript executed on the expected url and no access to the WebExtensions APIs");
     await contentPage2.close();
   }
 
   await extension.unload();
 });
 
 add_task(async function test_userScripts_exported_APIs() {