Bug 1435100 - Ensure preloaded css and script caches are cleared when a WebExtension is shutting down. draft
authorLuca Greco <lgreco@mozilla.com>
Wed, 21 Feb 2018 12:53:56 +0100
changeset 758501 0ea33110b2b6fec907f222d2ffcbfa50f3e04f2b
parent 758497 5e9bd04333f20e00911b8c1dfbf2b2e070c61e2d
push id100079
push userluca.greco@alcacoop.it
push dateThu, 22 Feb 2018 15:39:08 +0000
bugs1435100
milestone60.0a1
Bug 1435100 - Ensure preloaded css and script caches are cleared when a WebExtension is shutting down. MozReview-Commit-ID: IHK7hBYVLMj
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionContent.jsm
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -664,16 +664,17 @@ class BrowserExtensionContent extends Ev
 
   shutdown() {
     ExtensionManager.extensions.delete(this.id);
     ExtensionContent.shutdownExtension(this);
     Services.cpmm.removeMessageListener(this.MESSAGE_EMIT_EVENT, this);
     if (isContentProcess) {
       MessageChannel.abortResponses({extensionId: this.id});
     }
+    this.emit("shutdown");
   }
 
   getContext(window) {
     return ExtensionContent.getContext(this, window);
   }
 
   emit(event, ...args) {
     Services.cpmm.sendAsyncMessage(this.MESSAGE_EMIT_EVENT, {event, args});
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -92,22 +92,29 @@ const SCRIPT_CLEAR_TIMEOUT_MS = 5 * 1000
 
 const CSS_EXPIRY_TIMEOUT_MS = 30 * 60 * 1000;
 const CSSCODE_EXPIRY_TIMEOUT_MS = 10 * 60 * 1000;
 
 const scriptCaches = new WeakSet();
 const sheetCacheDocuments = new DefaultWeakMap(() => new WeakSet());
 
 class CacheMap extends DefaultMap {
-  constructor(timeout, getter) {
+  constructor(timeout, getter, extension) {
     super(getter);
 
     this.expiryTimeout = timeout;
 
     scriptCaches.add(this);
+
+    // This ensures that all the cached scripts and stylesheets are deleted
+    // from the cache and the xpi is no longer actively used.
+    // See Bug 1435100 for rationale.
+    extension.once("shutdown", () => {
+      this.clear(-1);
+    });
   }
 
   get(url) {
     let promise = super.get(url);
 
     promise.lastUsed = Date.now();
     if (promise.timer) {
       promise.timer.cancel();
@@ -125,26 +132,29 @@ class CacheMap extends DefaultMap {
     }
 
     super.delete(url);
   }
 
   clear(timeout = SCRIPT_CLEAR_TIMEOUT_MS) {
     let now = Date.now();
     for (let [url, promise] of this.entries()) {
-      if (now - promise.lastUsed >= timeout) {
+      // Delete the entry if expired or if clear has been called with timeout -1
+      // (which is used to force the cache to clear all the entries, e.g. when the
+      // extension is shutting down).
+      if (timeout === -1 || (now - promise.lastUsed >= timeout)) {
         this.delete(url);
       }
     }
   }
 }
 
 class ScriptCache extends CacheMap {
-  constructor(options) {
-    super(SCRIPT_EXPIRY_TIMEOUT_MS);
+  constructor(options, extension) {
+    super(SCRIPT_EXPIRY_TIMEOUT_MS, null, extension);
     this.options = options;
   }
 
   defaultConstructor(url) {
     let promise = ChromeUtils.compileScript(url, this.options);
     promise.then(script => {
       promise.script = script;
     });
@@ -153,16 +163,20 @@ class ScriptCache extends CacheMap {
 }
 
 /**
  * Shared base class for the two specialized CSS caches:
  * CSSCache (for the "url"-based stylesheets) and CSSCodeCache
  * (for the stylesheet defined by plain CSS content as a string).
  */
 class BaseCSSCache extends CacheMap {
+  constructor(expiryTimeout, defaultConstructor, extension) {
+    super(expiryTimeout, defaultConstructor, extension);
+  }
+
   addDocument(key, document) {
     sheetCacheDocuments.get(this.get(key)).add(document);
   }
 
   deleteDocument(key, document) {
     sheetCacheDocuments.get(this.get(key)).delete(document);
   }
 
@@ -183,23 +197,23 @@ class BaseCSSCache extends CacheMap {
     super.delete(key);
   }
 }
 
 /**
  * Cache of the preloaded stylesheet defined by url.
  */
 class CSSCache extends BaseCSSCache {
-  constructor(sheetType) {
+  constructor(sheetType, extension) {
     super(CSS_EXPIRY_TIMEOUT_MS, url => {
       let uri = Services.io.newURI(url);
       return styleSheetService.preloadSheetAsync(uri, sheetType).then(sheet => {
         return {url, sheet};
       });
-    });
+    }, extension);
   }
 }
 
 /**
  * Cache of the preloaded stylesheet defined by plain CSS content as a string,
  * the key of the cached stylesheet is the hash of its "CSSCode" string.
  */
 class CSSCodeCache extends BaseCSSCache {
@@ -207,17 +221,17 @@ class CSSCodeCache extends BaseCSSCache 
     super(CSSCODE_EXPIRY_TIMEOUT_MS, (hash) => {
       if (!this.has(hash)) {
         // Do not allow the getter to be used to lazily create the cached stylesheet,
         // the cached CSSCode stylesheet has to be explicitly set.
         throw new Error("Unexistent cached cssCode stylesheet: " + Error().stack);
       }
 
       return super.get(hash);
-    });
+    }, extension);
 
     // Store the preferred sheetType (used to preload the expected stylesheet type in
     // the addCSSCode method).
     this.sheetType = sheetType;
   }
 
   addCSSCode(hash, cssCode) {
     if (this.has(hash)) {
@@ -228,30 +242,30 @@ class CSSCodeCache extends BaseCSSCache 
     const value = styleSheetService.preloadSheetAsync(uri, this.sheetType).then(sheet => {
       return {sheet, uri};
     });
 
     super.set(hash, value);
   }
 }
 
-defineLazyGetter(BrowserExtensionContent.prototype, "staticScripts", () => {
-  return new ScriptCache({hasReturnValue: false});
+defineLazyGetter(BrowserExtensionContent.prototype, "staticScripts", function() {
+  return new ScriptCache({hasReturnValue: false}, this);
 });
 
-defineLazyGetter(BrowserExtensionContent.prototype, "dynamicScripts", () => {
-  return new ScriptCache({hasReturnValue: true});
+defineLazyGetter(BrowserExtensionContent.prototype, "dynamicScripts", function() {
+  return new ScriptCache({hasReturnValue: true}, this);
 });
 
-defineLazyGetter(BrowserExtensionContent.prototype, "userCSS", () => {
-  return new CSSCache(Ci.nsIStyleSheetService.USER_SHEET);
+defineLazyGetter(BrowserExtensionContent.prototype, "userCSS", function() {
+  return new CSSCache(Ci.nsIStyleSheetService.USER_SHEET, this);
 });
 
-defineLazyGetter(BrowserExtensionContent.prototype, "authorCSS", () => {
-  return new CSSCache(Ci.nsIStyleSheetService.AUTHOR_SHEET);
+defineLazyGetter(BrowserExtensionContent.prototype, "authorCSS", function() {
+  return new CSSCache(Ci.nsIStyleSheetService.AUTHOR_SHEET, this);
 });
 
 // These two caches are similar to the above but specialized to cache the cssCode
 // using an hash computed from the cssCode string as the key (instead of the generated data
 // URI which can be pretty long for bigger injected cssCode).
 defineLazyGetter(BrowserExtensionContent.prototype, "userCSSCode", function() {
   return new CSSCodeCache(Ci.nsIStyleSheetService.USER_SHEET, this);
 });
@@ -314,17 +328,17 @@ class Script {
     return this.css.map(url => this.cssCache.get(url));
   }
 
   preload() {
     this.loadCSS();
     this.compileScripts();
   }
 
-  cleanup(window, forceCacheClear = false) {
+  cleanup(window) {
     if (this.requiresCleanup) {
       if (window) {
         let winUtils = getWinUtils(window);
 
         let type = this.cssOrigin === "user" ? winUtils.USER_SHEET : winUtils.AUTHOR_SHEET;
 
         for (let url of this.css) {
           this.cssCache.deleteDocument(url, window.document);
@@ -338,18 +352,18 @@ class Script {
             runSafeSyncWithoutClone(winUtils.removeSheet, uri, type);
           });
           this.cssCodeCache.deleteDocument(cssCodeHash, window.document);
         }
       }
 
       // Clear any sheets that were kept alive past their timeout as
       // a result of living in this document.
-      this.cssCodeCache.clear(forceCacheClear ? 0 : CSSCODE_EXPIRY_TIMEOUT_MS);
-      this.cssCache.clear(forceCacheClear ? 0 : CSS_EXPIRY_TIMEOUT_MS);
+      this.cssCodeCache.clear(CSSCODE_EXPIRY_TIMEOUT_MS);
+      this.cssCache.clear(CSS_EXPIRY_TIMEOUT_MS);
     }
   }
 
   matchesWindow(window) {
     return this.matcher.matchesWindow(window);
   }
 
   async injectInto(window) {
@@ -603,29 +617,23 @@ class ContentScriptContextChild extends 
   }
 
   addScript(script) {
     if (script.requiresCleanup) {
       this.scripts.push(script);
     }
   }
 
-  cleanupScripts(forceCacheClear = false) {
-    // Cleanup the scripts (even if the contentWindow have been destroyed) and their
-    // related CSS and Script caches.
-    for (let script of this.scripts) {
-      script.cleanup(this.contentWindow, forceCacheClear);
-    }
-  }
-
   close() {
     super.unload();
 
     // Cleanup the scripts even if the contentWindow have been destroyed.
-    this.cleanupScripts();
+    for (let script of this.scripts) {
+      script.cleanup(this.contentWindow);
+    }
 
     if (this.contentWindow) {
       // Overwrite the content script APIs with an empty object if the APIs objects are still
       // defined in the content window (See Bug 1214658).
       if (this.isExtensionPage) {
         Cu.createObjectIn(this.contentWindow, {defineAs: "browser"});
         Cu.createObjectIn(this.contentWindow, {defineAs: "chrome"});
       }
@@ -711,20 +719,16 @@ DocumentManager = {
   observe(subject, topic, data) {
     this.observers[topic].call(this, subject, topic, data);
   },
 
   shutdownExtension(extension) {
     for (let extensions of this.contexts.values()) {
       let context = extensions.get(extension);
       if (context) {
-        // Passing true to context.cleanupScripts causes the caches for this context
-        // to be cleared.
-        context.cleanupScripts(true);
-
         context.close();
         extensions.delete(extension);
       }
     }
   },
 
   getContexts(window) {
     let winId = getInnerWindowID(window);