fix: unify adblocker engines update process (#1878)

ghostery · Sep 11, 2024 · a10e086 · a10e086
1 parent 40a2852
commit a10e086
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 50 deletions.
diff --git a/extension-manifest-v3/src/background/adblocker.js b/extension-manifest-v3/src/background/adblocker.js
@@ -19,9 +19,9 @@ import { parse } from 'tldts-experimental';
 import Options, { observe, ENGINES, isPaused } from '/store/options.js';
 
 import * as engines from '/utils/engines.js';
+import * as trackerdb from '/utils/trackerdb.js';
 import Request from '/utils/request.js';
 import asyncSetup from '/utils/setup.js';
-import { getMetadata } from '/utils/trackerdb.js';
 
 import { tabStats, updateTabStats } from './stats.js';
 import { getException } from './exceptions.js';
@@ -52,8 +52,9 @@ function getEnabledEngines(config) {
   return [];
 }
 
-const HOUR_IN_MS = 60 * 60 * 1000;
-async function reloadEngine(enabledEngines) {
+async function reloadMainEngine() {
+  const enabledEngines = getEnabledEngines(options);
+
   if (enabledEngines.length) {
     engines.replace(
       engines.MAIN_ENGINE,
@@ -70,18 +71,48 @@ async function reloadEngine(enabledEngines) {
     );
 
     console.info(
-      `[adblocker] engine reloaded with: ${enabledEngines.join(', ')}`,
+      `[adblocker] Main engine reloaded with: ${enabledEngines.join(', ')}`,
     );
   } else {
     engines.create(engines.MAIN_ENGINE);
-    console.info('[adblocker] Engine reloaded with no filters');
+    console.info('[adblocker] Main engine reloaded with no filters');
   }
 }
 
-engines.addChangeListener(engines.CUSTOM_ENGINE, async () => {
-  reloadEngine(getEnabledEngines(options));
-});
+engines.addChangeListener(engines.CUSTOM_ENGINE, reloadMainEngine);
+
+let updating = false;
+async function updateEngines() {
+  if (updating) return;
+
+  try {
+    updating = true;
+    const enabledEngines = getEnabledEngines(options);
+
+    if (enabledEngines.length) {
+      let updated = false;
+      // Update engines from the list of enabled engines
+      for (const id of enabledEngines) {
+        if (id === engines.CUSTOM_ENGINE) continue;
+        updated = (await engines.update(id).catch(() => false)) || updated;
+      }
+
+      // Reload the main engine after all engines are updated
+      if (updated) await reloadMainEngine();
 
+      // Update TrackerDB engine
+      trackerdb.setup.pending && (await trackerdb.setup.pending);
+      await engines.update(engines.TRACKERDB_ENGINE).catch(() => null);
+
+      // Update timestamp after the engines are updated
+      await store.set(Options, { filtersUpdatedAt: Date.now() });
+    }
+  } finally {
+    updating = false;
+  }
+}
+
+const HOUR_IN_MS = 60 * 60 * 1000;
 export const setup = asyncSetup([
   observe(async (value, lastValue) => {
     options = value;
@@ -95,44 +126,26 @@ export const setup = asyncSetup([
       // Enabled engines changed
       (prevEnabledEngines &&
         (enabledEngines.length !== prevEnabledEngines.length ||
-          enabledEngines.some((id, i) => id !== prevEnabledEngines[i]))) ||
-      // Engine filters updatedAt changed after successful update
-      (lastValue && value.filtersUpdatedAt > lastValue.filtersUpdatedAt)
+          enabledEngines.some((id, i) => id !== prevEnabledEngines[i])))
     ) {
       // The regional filters engine is no longer used, so we must remove it
       // from the storage. We do it as rarely as possible, to avoid unnecessary loads.
       // TODO: this can be removed in the future release when most of the users will have
       // the new version of the extension
       engines.remove('regional-filters');
 
-      await reloadEngine(enabledEngines);
+      await reloadMainEngine();
     }
 
-    if (
-      // Experimental filters enabled
-      (options.experimentalFilters &&
-        lastValue?.experimentalFilters === false) ||
-      // Filters outdated (1 hour)
-      options.filtersUpdatedAt < Date.now() - HOUR_IN_MS
-    ) {
-      const updateEngines = getEnabledEngines(options)
-        // Remove the custom engine, as it is created client-side only
-        .filter((name) => name !== engines.CUSTOM_ENGINE)
-        // Add engines outside of the main engine
-        .concat(engines.TRACKERDB_ENGINE);
-
-      (async () => {
-        for (const id of updateEngines) {
-          await engines.update(id).catch(() => null);
-        }
-
-        // Trigger update of the main engine after all other engines are updated
-        store.set(Options, { filtersUpdatedAt: Date.now() });
-      })();
+    if (options.filtersUpdatedAt < Date.now() - HOUR_IN_MS) {
+      updateEngines();
     }
   }),
-  observe('experimentalFilters', async (value) => {
+  observe('experimentalFilters', async (value, lastValue) => {
     engines.setEnv('env_experimental', value);
+
+    // Experimental filters changed to enabled
+    if (lastValue !== undefined && value) updateEngines();
   }),
 ]);
 
@@ -419,7 +432,7 @@ function isTrusted(request, type) {
     return false;
   }
 
-  const metadata = getMetadata(request);
+  const metadata = trackerdb.getMetadata(request);
 
   // Get exception for known tracker (metadata id) or
   // by the request hostname (unidentified tracker)

diff --git a/extension-manifest-v3/src/utils/engines.js b/extension-manifest-v3/src/utils/engines.js
@@ -176,16 +176,9 @@ async function loadFromFile(name) {
 
     saveToMemory(name, engine);
 
-    await saveToStorage(name)
-      .then(() =>
-        // After initial load from disk, schedule an update
-        // as it is done only once on the first run.
-        // After loading from disk, it should be loaded from the storage
-        update(name).catch(() => null),
-      )
-      .catch(() => {
-        console.error(`[engines] Failed to save engine "${name}" to storage`);
-      });
+    await saveToStorage(name).catch(() => {
+      console.error(`[engines] Failed to save engine "${name}" to storage`);
+    });
 
     return engine;
   } catch (e) {
@@ -241,7 +234,7 @@ export async function update(name) {
       `[engines] Skipping update for engine "${name}" as the engine is not available`,
     );
 
-    return;
+    return false;
   }
 
   try {
@@ -252,7 +245,7 @@ export async function update(name) {
 
     const listURL = `https://${CDN_HOSTNAME}/adblocker/configs/${urlName}/allowed-lists.json`;
 
-    console.info(`[engines] Updating engine "${name}" from ${listURL}`);
+    console.info(`[engines] Updating engine "${name}...`);
 
     const data = await fetch(listURL)
       .then(check)
@@ -313,7 +306,7 @@ export async function update(name) {
 
       console.info(`Engine "${name}" reloaded`);
 
-      return engine;
+      return true;
     }
 
     // At this point we know that no list needs to be removed anymore. What
@@ -419,9 +412,11 @@ export async function update(name) {
 
       // Save the new engine to storage
       saveToStorage(name);
+
+      return true;
     }
 
-    return engine;
+    return false;
   } catch (e) {
     console.error(`[engines] Failed to update engine "${name}"`, e);
     throw e;

diff --git a/extension-manifest-v3/src/utils/trackerdb.js b/extension-manifest-v3/src/utils/trackerdb.js
@@ -34,7 +34,7 @@ const categoryOrder = [
   'other',
 ];
 
-const setup = asyncSetup([engines.init(engines.TRACKERDB_ENGINE)]);
+export const setup = asyncSetup([engines.init(engines.TRACKERDB_ENGINE)]);
 
 export function getUnidentifiedTracker(hostname) {
   return {