Bug 1269141: Remove content script CSS when disabling an extension. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 04 Jun 2016 21:42:55 -0700
changeset 375402 76376b369739cbba218cb7741b09ae040dafb6b4
parent 375401 0ac19ca034d9097ded455362e46b56017f3a64a4
child 522861 74f55bb36da47a09c71aceb5f4969d358d1ab0e0
push id20266
push usermaglione.k@gmail.com
push dateSun, 05 Jun 2016 04:45:42 +0000
reviewersaswan
bugs1269141
milestone49.0a1
Bug 1269141: Remove content script CSS when disabling an extension. r?aswan MozReview-Commit-ID: Jhg4XzddIkA
toolkit/components/extensions/ExtensionContent.jsm
toolkit/components/extensions/test/mochitest/mochitest.ini
toolkit/components/extensions/test/mochitest/test_ext_contentscript_css.html
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -149,35 +149,53 @@ var api = context => {
         let result = detectLanguage(text);
         return context.wrapPromise(result, callback);
       },
     },
   };
 };
 
 // Represents a content script.
-function Script(options, deferred = PromiseUtils.defer()) {
+function Script(extension, options, deferred = PromiseUtils.defer()) {
+  this.extension = extension;
   this.options = options;
   this.run_at = this.options.run_at;
   this.js = this.options.js || [];
   this.css = this.options.css || [];
   this.remove_css = this.options.remove_css;
 
   this.deferred = deferred;
 
   this.matches_ = new MatchPattern(this.options.matches);
   this.exclude_matches_ = new MatchPattern(this.options.exclude_matches || null);
   // TODO: MatchPattern should pre-mangle host-only patterns so that we
   // don't need to call a separate match function.
   this.matches_host_ = new MatchPattern(this.options.matchesHost || null);
   this.include_globs_ = new MatchGlobs(this.options.include_globs);
   this.exclude_globs_ = new MatchGlobs(this.options.exclude_globs);
+
+  this.requiresCleanup = !this.remove_css && (this.css.length > 0 || options.cssCode);
 }
 
 Script.prototype = {
+  get cssURLs() {
+    // We can handle CSS urls (css) and CSS code (cssCode).
+    let urls = [];
+    for (let url of this.css) {
+      urls.push(this.extension.baseURI.resolve(url));
+    }
+
+    if (this.options.cssCode) {
+      let url = "data:text/css;charset=utf-8," + encodeURIComponent(this.options.cssCode);
+      urls.push(url);
+    }
+
+    return urls;
+  },
+
   matches(window) {
     let uri = window.document.documentURIObject;
     if (!(this.matches_.matches(uri) || this.matches_host_.matchesIgnoringPath(uri))) {
       return false;
     }
 
     if (this.exclude_matches_.matches(uri)) {
       return false;
@@ -201,61 +219,60 @@ Script.prototype = {
       return false;
     }
 
     // TODO: match_about_blank.
 
     return true;
   },
 
-  tryInject(extension, window, sandbox, shouldRun) {
+  cleanup(window) {
+    if (!this.remove_css) {
+      let winUtils = window.QueryInterface(Ci.nsIInterfaceRequestor)
+                           .getInterface(Ci.nsIDOMWindowUtils);
+
+      for (let url of this.cssURLs) {
+        runSafeSyncWithoutClone(winUtils.removeSheetUsingURIString, url, winUtils.AUTHOR_SHEET);
+      }
+    }
+  },
+
+  tryInject(window, sandbox, shouldRun) {
     if (!this.matches(window)) {
       this.deferred.reject({message: "No matching window"});
       return;
     }
 
     if (shouldRun("document_start")) {
       let winUtils = window.QueryInterface(Ci.nsIInterfaceRequestor)
                            .getInterface(Ci.nsIDOMWindowUtils);
 
-      // We can handle CSS urls (css) and CSS code (cssCode).
-      let cssUrls = [];
-      for (let cssUrl of this.css) {
-        cssUrl = extension.baseURI.resolve(cssUrl);
-        cssUrls.push(cssUrl);
-      }
+      let {cssURLs} = this;
+      if (cssURLs.length > 0) {
+        let method = this.remove_css ? winUtils.removeSheetUsingURIString : winUtils.loadSheetUsingURIString;
+        for (let url of cssURLs) {
+          runSafeSyncWithoutClone(method, url, winUtils.AUTHOR_SHEET);
+        }
 
-      if (this.options.cssCode) {
-        let cssUrl = "data:text/css;charset=utf-8," + encodeURIComponent(this.options.cssCode);
-        cssUrls.push(cssUrl);
-      }
-
-      // We can insertCSS and removeCSS.
-      let method = this.remove_css ? winUtils.removeSheetUsingURIString : winUtils.loadSheetUsingURIString;
-      for (let cssUrl of cssUrls) {
-        runSafeSyncWithoutClone(method, cssUrl, winUtils.AUTHOR_SHEET);
-      }
-
-      if (cssUrls.length > 0) {
         this.deferred.resolve();
       }
     }
 
     let result;
     let scheduled = this.run_at || "document_idle";
     if (shouldRun(scheduled)) {
       for (let url of this.js) {
         // On gonk we need to load the resources asynchronously because the
         // app: channels only support asyncOpen. This is safe only in the
         // `document_idle` state.
         if (AppConstants.platform == "gonk" && scheduled != "document_idle") {
           Cu.reportError(`Script injection: ignoring ${url} at ${scheduled}`);
           continue;
         }
-        url = extension.baseURI.resolve(url);
+        url = this.extension.baseURI.resolve(url);
 
         let options = {
           target: sandbox,
           charset: "UTF-8",
           async: AppConstants.platform == "gonk",
         };
         try {
           result = Services.scriptloader.loadSubScriptWithOptions(url, options);
@@ -412,43 +429,49 @@ class ExtensionContext extends BaseConte
     }
   }
 
   get cloneScope() {
     return this.sandbox;
   }
 
   execute(script, shouldRun) {
-    script.tryInject(this.extension, this.contentWindow, this.sandbox, shouldRun);
+    script.tryInject(this.contentWindow, this.sandbox, shouldRun);
   }
 
   addScript(script) {
     let state = DocumentManager.getWindowState(this.contentWindow);
     this.execute(script, scheduled => isWhenBeforeOrSame(scheduled, state));
 
     // Save the script in case it has pending operations in later load
-    // states, but only if we're before document_idle.
-    if (state != "document_idle") {
+    // states, but only if we're before document_idle, or require cleanup.
+    if (state != "document_idle" || script.requiresCleanup) {
       this.scripts.push(script);
     }
   }
 
   triggerScripts(documentState) {
     for (let script of this.scripts) {
       this.execute(script, scheduled => scheduled == documentState);
     }
     if (documentState == "document_idle") {
       // Don't bother saving scripts after document_idle.
-      this.scripts.length = 0;
+      this.scripts = this.scripts.filter(script => script.requiresCleanup);
     }
   }
 
   close() {
     super.unload();
 
+    for (let script of this.scripts) {
+      if (script.requiresCleanup) {
+        script.cleanup(this.contentWindow);
+      }
+    }
+
     this.childManager.close();
 
     // Overwrite the content script APIs with an empty object if the APIs objects are still
     // defined in the content window (See Bug 1214658 for rationale).
     if (this.isExtensionPage && !Cu.isDeadWrapper(this.contentWindow) &&
         Cu.waiveXrays(this.contentWindow).browser === this.chromeObj) {
       Cu.createObjectIn(this.contentWindow, {defineAs: "browser"});
       Cu.createObjectIn(this.contentWindow, {defineAs: "chrome"});
@@ -560,19 +583,21 @@ DocumentManager = {
       this.trigger("document_end", window);
     } else if (event.type == "load") {
       this.trigger("document_idle", window);
     }
   },
 
   // Used to executeScript, insertCSS and removeCSS.
   executeScript(global, extensionId, options) {
+    let extension = ExtensionManager.get(extensionId);
+
     let executeInWin = (window) => {
       let deferred = PromiseUtils.defer();
-      let script = new Script(options, deferred);
+      let script = new Script(extension, options, deferred);
 
       if (script.matches(window)) {
         let context = this.getContentScriptContext(extensionId, window);
         context.addScript(script);
         return deferred.promise;
       }
       return null;
     };
@@ -710,17 +735,17 @@ DocumentManager = {
   },
 };
 
 // Represents a browser extension in the content process.
 function BrowserExtensionContent(data) {
   this.id = data.id;
   this.uuid = data.uuid;
   this.data = data;
-  this.scripts = data.content_scripts.map(scriptData => new Script(scriptData));
+  this.scripts = data.content_scripts.map(scriptData => new Script(this, scriptData));
   this.webAccessibleResources = new MatchGlobs(data.webAccessibleResources);
   this.whiteListedHosts = new MatchPattern(data.whiteListedHosts);
 
   this.localeData = new LocaleData(data.localeData);
 
   this.manifest = data.manifest;
   this.baseURI = Services.io.newURI(data.baseURL, null, null);
 
--- a/toolkit/components/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest.ini
@@ -42,16 +42,17 @@ skip-if = os == 'android' # Android does
 [test_ext_simple.html]
 [test_ext_geturl.html]
 [test_ext_content_security_policy.html]
 [test_ext_contentscript.html]
 skip-if = buildapp == 'b2g' # runat != document_idle is not supported.
 [test_ext_contentscript_api_injection.html]
 [test_ext_contentscript_create_iframe.html]
 [test_ext_contentscript_devtools_metadata.html]
+[test_ext_contentscript_css.html]
 [test_ext_downloads.html]
 [test_ext_exclude_include_globs.html]
 [test_ext_i18n_css.html]
 [test_ext_generate.html]
 [test_ext_idle.html]
 [test_ext_localStorage.html]
 [test_ext_onmessage_removelistener.html]
 [test_ext_notifications.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_contentscript_css.html
@@ -0,0 +1,48 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for content script</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+add_task(function* test_content_script_css() {
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      "content_scripts": [{
+        "matches": ["http://mochi.test/*/file_sample.html"],
+        "css": ["content.css"],
+      }],
+    },
+
+    files: {
+      "content.css": "body { max-width: 42px; }",
+    },
+  });
+
+  yield extension.startup();
+
+  let win = window.open("file_sample.html");
+  yield waitForLoad(win);
+
+  let style = win.getComputedStyle(win.document.body);
+  is(style.maxWidth, "42px", "Stylesheet correctly applied");
+
+  yield extension.unload();
+
+  style = win.getComputedStyle(win.document.body);
+  is(style.maxWidth, "none", "Stylesheet correctly removed");
+
+  win.close();
+});
+</script>
+
+</body>
+</html>