Bug 1348442: Part 3 - Do not evict cached stylesheets while they're still being used by extant documents. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 18 Mar 2017 16:07:58 -0700
changeset 501333 c9f6e9af5aade1054a4cef1d82f56d93466243fa
parent 501332 8998d4d545be46f0d84a0d32e73b5699db6750d1
child 501334 37580af14e979c3bea083eb53f9f019638869c47
push id49931
push usermaglione.k@gmail.com
push dateSun, 19 Mar 2017 23:22:27 +0000
reviewersaswan
bugs1348442
milestone55.0a1
Bug 1348442: Part 3 - Do not evict cached stylesheets while they're still being used by extant documents. r?aswan There are some optimizations, both existing and under way, that allow us to share some stylesheet data between documents that they're loaded into. Keeping cached sheets around as long as they're still in use should be a net memory savings. MozReview-Commit-ID: HUZzs6HhuFM
toolkit/components/extensions/ExtensionContent.jsm
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -51,16 +51,17 @@ XPCOMUtils.defineLazyServiceGetter(this,
 const Timer = Components.Constructor("@mozilla.org/timer;1", "nsITimer", "initWithCallback");
 
 Cu.import("resource://gre/modules/ExtensionChild.jsm");
 Cu.import("resource://gre/modules/ExtensionCommon.jsm");
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 const {
   DefaultMap,
+  DefaultWeakMap,
   EventEmitter,
   LocaleData,
   defineLazyGetter,
   flushJarCache,
   getInnerWindowID,
   getWinUtils,
   promiseDocumentReady,
   runSafeSyncWithoutClone,
@@ -111,16 +112,17 @@ var apiManager = new class extends Schem
 }();
 
 const SCRIPT_EXPIRY_TIMEOUT_MS = 5 * 60 * 1000;
 const SCRIPT_CLEAR_TIMEOUT_MS = 5 * 1000;
 
 const CSS_EXPIRY_TIMEOUT_MS = 30 * 60 * 1000;
 
 const scriptCaches = new WeakSet();
+const sheetCacheDocuments = new DefaultWeakMap(() => new WeakSet());
 
 class CacheMap extends DefaultMap {
   constructor(timeout, getter) {
     super(getter);
 
     this.expiryTimeout = timeout;
 
     scriptCaches.add(this);
@@ -169,16 +171,41 @@ class CSSCache extends CacheMap {
   constructor(sheetType) {
     super(CSS_EXPIRY_TIMEOUT_MS, url => {
       let uri = Services.io.newURI(url);
       return styleSheetService.preloadSheetAsync(uri, sheetType).then(sheet => {
         return {url, sheet};
       });
     });
   }
+
+  addDocument(url, document) {
+    sheetCacheDocuments.get(this.get(url)).add(document);
+  }
+
+  deleteDocument(url, document) {
+    sheetCacheDocuments.get(this.get(url)).delete(document);
+  }
+
+  delete(url) {
+    if (this.has(url)) {
+      let promise = this.get(url);
+
+      // Never remove a sheet from the cache if it's still being used by a
+      // document. Rule processors can be shared between documents with the
+      // same preloaded sheet, so we only lose by removing them while they're
+      // still in use.
+      let docs = ChromeUtils.nondeterministicGetWeakSetKeys(sheetCacheDocuments.get(promise));
+      if (docs.length) {
+        return;
+      }
+    }
+
+    super.delete(url);
+  }
 }
 
 // Represents a content script.
 class Script {
   constructor(extension, options, deferred = PromiseUtils.defer()) {
     this.extension = extension;
     this.options = options;
     this.run_at = this.options.run_at;
@@ -296,23 +323,28 @@ class Script {
     } else if (!this.options.all_frames && window.top != window) {
       return false;
     }
 
     return true;
   }
 
   cleanup(window) {
-    if (!this.remove_css) {
+    if (!this.remove_css && this.cssURLs.length) {
       let winUtils = getWinUtils(window);
 
       let type = this.css_origin === "user" ? winUtils.USER_SHEET : winUtils.AUTHOR_SHEET;
       for (let url of this.cssURLs) {
+        this.cssCache.deleteDocument(url, window.document);
         runSafeSyncWithoutClone(winUtils.removeSheetUsingURIString, url, type);
       }
+
+      // Clear any sheets that were kept alive past their timeout as
+      // a result of living in this document.
+      this.cssCache.clear(CSS_EXPIRY_TIMEOUT_MS);
     }
   }
 
   /**
    * Tries to inject this script into the given window and sandbox, if
    * there are pending operations for the window's current load state.
    *
    * @param {Window} window
@@ -341,28 +373,32 @@ class Script {
       let winUtils = getWinUtils(window);
 
       let innerWindowID = winUtils.currentInnerWindowID;
 
       let type = this.css_origin === "user" ? winUtils.USER_SHEET : winUtils.AUTHOR_SHEET;
 
       if (this.remove_css) {
         for (let url of this.cssURLs) {
+          this.cssCache.deleteDocument(url, window.document);
+
           runSafeSyncWithoutClone(winUtils.removeSheetUsingURIString, url, type);
         }
 
         this.deferred.resolve();
       } else {
         this.deferred.resolve(
           Promise.all(this.loadCSS()).then(sheets => {
             if (winUtils.currentInnerWindowID !== innerWindowID) {
               return;
             }
 
-            for (let {sheet} of sheets) {
+            for (let {url, sheet} of sheets) {
+              this.cssCache.addDocument(url, window.document);
+
               runSafeSyncWithoutClone(winUtils.addSheet, sheet, type);
             }
           }));
       }
     }
 
     let scheduled = this.run_at || "document_idle";
     if (shouldRun(scheduled)) {