fix: ensure content script to be unregistered

src/background/index.js

@@ -73,7 +73,6 @@ Object.assign(commands, {
       // FF bug: the badge is reset because sometimes tabs get their real/internal url later
       if (ua.isFirefox) cache.put(`badge:${tab.id}${url}`, badgeData);
       Object.assign(res, data.inject);
-      data.registration?.then(r => r.unregister());
       // Injecting known content mode scripts without waiting for InjectionFeedback
       const inContent = res.scripts.map(s => !s.code && [s.dataKey, true]).filter(Boolean);
       if (inContent.length) {

src/background/utils/preinject.js

@@ -16,7 +16,14 @@ const API_CONFIG = {
 const TIME_AFTER_SEND = 10e3; // longer as establishing connection to sites may take time
 const TIME_AFTER_RECEIVE = 1e3; // shorter as response body will be coming very soon
 const TIME_KEEP_DATA = 60e3; // 100ms should be enough but the tab may hang or get paused in debugger
-const cache = initCache({ lifetime: TIME_KEEP_DATA });
+const cacheCode = initCache({ lifetime: TIME_KEEP_DATA });
+const cache = initCache({
+  lifetime: TIME_KEEP_DATA,
+  async onDispose(promise) {
+    const data = await promise;
+    data.unregister?.();
+  },
+});
 let injectInto;
 hookOptions(changes => {
   injectInto = changes.defaultInjectInto ?? injectInto;
@@ -30,7 +37,7 @@ postInitialize.push(() => {
 Object.assign(commands, {
   InjectionFeedback(feedback, { tab, frameId }) {
     feedback.forEach(([key, needsInjection]) => {
-      const code = cache.pop(key);
+      const code = cacheCode.pop(key);
       // see TIME_KEEP_DATA comment
       if (needsInjection && code) {
         browser.tabs.executeScript(tab.id, {
@@ -64,13 +71,12 @@ browser.storage.onChanged.addListener(async changes => {
         || data[prop]?.includes(prefix === storage.value.prefix ? +key : key);
     }));
   if (dirty) {
-    clearCache(cacheValues);
+    clearCache();
   }
 });
 
-async function clearCache(cacheValues) {
-  if (!cacheValues) cacheValues = await Promise.all(cache.getValues());
-  cacheValues.forEach(data => data.registration?.then(r => r.unregister()));
+function clearCache() {
+  cacheCode.destroy();
   cache.destroy();
 }
 
@@ -154,7 +160,7 @@ function prepareScript(script, index, scripts) {
     // Firefox lists .user.js among our own content scripts so a space at start will group them
     `\n//# sourceURL=${extensionRoot}${ua.isFirefox ? '%20' : ''}${name}.user.js#${id}`,
   ].join('');
-  cache.put(dataKey, injectedCode, TIME_KEEP_DATA);
+  cacheCode.put(dataKey, injectedCode, TIME_KEEP_DATA);
   scripts[index] = {
     ...script,
     dataKey,
@@ -180,7 +186,7 @@ const resolveDataCodeStr = `(${(data) => {
 }})`;
 
 function registerScriptDataFF(data, url, allFrames) {
-  data.registration = browser.contentScripts.register({
+  const promise = browser.contentScripts.register({
     allFrames,
     js: [{
       code: `${resolveDataCodeStr}(${JSON.stringify(data.inject)})`,
@@ -188,4 +194,9 @@ function registerScriptDataFF(data, url, allFrames) {
     matches: url.split('#', 1),
     runAt: 'document_start',
   });
+  data.unregister = async () => {
+    data.unregister = null;
+    const r = await promise;
+    r.unregister();
+  };
 }

src/common/cache.js

@@ -1,9 +1,8 @@
-import { hasOwnProperty } from '#/common/util';
-
 export default function initCache({
   lifetime: defaultLifetime = 3000,
+  onDispose,
 } = {}) {
-  let cache = {};
+  let cache = Object.create(null);
   // setTimeout call is very expensive when done frequently,
   // 1000 calls performed for 50 scripts consume 50ms on each tab load,
   // so we'll schedule trim() just once per event loop cycle,
@@ -43,15 +42,19 @@ export default function initCache({
       };
       reschedule(lifetime);
     } else {
-      delete cache[key];
+      del(key);
     }
     return value;
   }
   function del(key) {
-    delete cache[key];
+    const data = cache[key];
+    if (data) {
+      delete cache[key];
+      onDispose?.(data.value, key);
+    }
   }
   function has(key) {
-    return cache::hasOwnProperty(key);
+    return cache[key];
   }
   function hit(key, lifetime = defaultLifetime) {
     const entry = cache[key];
@@ -61,9 +64,18 @@ export default function initCache({
     }
   }
   function destroy() {
+    // delete all keys to make sure onDispose is called for each value
+    if (onDispose) {
+      // cache inherits null so we don't need to check hasOwnProperty
+      // eslint-disable-next-line guard-for-in
+      for (const key in cache) {
+        del(key);
+      }
+    } else {
+      cache = Object.create(null);
+    }
     clearTimeout(timer);
     timer = 0;
-    cache = {};
   }
   function reschedule(lifetime) {
     if (timer) {
@@ -78,14 +90,13 @@ export default function initCache({
     // so we'll sweep the upcoming expired entries in this run
     const now = performance.now() + 10;
     let closestExpiry = Number.MAX_SAFE_INTEGER;
+    // eslint-disable-next-line guard-for-in
     for (const key in cache) {
-      if (Object.hasOwnProperty.call(cache, key)) {
-        const { expiry } = cache[key];
-        if (expiry < now) {
-          delete cache[key];
-        } else if (expiry < closestExpiry) {
-          closestExpiry = expiry;
-        }
+      const { expiry } = cache[key];
+      if (expiry < now) {
+        del(key);
+      } else if (expiry < closestExpiry) {
+        closestExpiry = expiry;
       }
     }
     minLifetime = closestExpiry - now;