Re-factor BasePreferences to essentially be a wrapper around AppOptions

In the MOZCENTRAL build the `BasePreferences` class is almost unused, since it's only being used to initialize and update the `AppOptions` which means that theoretically we could move the relevant code there.
However for the other build targets (e.g. GENERIC and CHROME) we still need to keep the `BasePreferences` class, although we can re-factor things to move the necessary validation inside of `AppOptions` and thus simplify the code and reduce duplication.

The patch also moves the event dispatching, for changed preference values, into `AppOptions` instead.

gulpfile.mjs

@@ -284,9 +284,6 @@ function createWebpackConfig(
     BUNDLE_VERSION: versionInfo.version,
     BUNDLE_BUILD: versionInfo.commit,
     TESTING: defines.TESTING ?? process.env.TESTING === "true",
-    BROWSER_PREFERENCES: defaultPreferencesDir
-      ? getBrowserPreferences(defaultPreferencesDir)
-      : {},
     DEFAULT_PREFERENCES: defaultPreferencesDir
       ? getDefaultPreferences(defaultPreferencesDir)
       : {},
@@ -856,13 +853,6 @@ async function parseDefaultPreferences(dir) {
     "./" + DEFAULT_PREFERENCES_DIR + dir + "app_options.mjs"
   );
 
-  const browserPrefs = AppOptions.getAll(
-    OptionKind.BROWSER,
-    /* defaultOnly = */ true
-  );
-  if (Object.keys(browserPrefs).length === 0) {
-    throw new Error("No browser preferences found.");
-  }
   const prefs = AppOptions.getAll(
     OptionKind.PREFERENCE,
     /* defaultOnly = */ true
@@ -871,23 +861,12 @@ async function parseDefaultPreferences(dir) {
     throw new Error("No default preferences found.");
   }
 
-  fs.writeFileSync(
-    DEFAULT_PREFERENCES_DIR + dir + "browser_preferences.json",
-    JSON.stringify(browserPrefs)
-  );
   fs.writeFileSync(
     DEFAULT_PREFERENCES_DIR + dir + "default_preferences.json",
     JSON.stringify(prefs)
   );
 }
 
-function getBrowserPreferences(dir) {
-  const str = fs
-    .readFileSync(DEFAULT_PREFERENCES_DIR + dir + "browser_preferences.json")
-    .toString();
-  return JSON.parse(str);
-}
-
 function getDefaultPreferences(dir) {
   const str = fs
     .readFileSync(DEFAULT_PREFERENCES_DIR + dir + "default_preferences.json")
@@ -1581,9 +1560,6 @@ function buildLib(defines, dir) {
     BUNDLE_VERSION: versionInfo.version,
     BUNDLE_BUILD: versionInfo.commit,
     TESTING: defines.TESTING ?? process.env.TESTING === "true",
-    BROWSER_PREFERENCES: getBrowserPreferences(
-      defines.SKIP_BABEL ? "lib/" : "lib-legacy/"
-    ),
     DEFAULT_PREFERENCES: getDefaultPreferences(
       defines.SKIP_BABEL ? "lib/" : "lib-legacy/"
     ),

web/app.js

@@ -391,9 +391,10 @@ const PDFViewerApplication = {
    */
   async _initializeViewerComponents() {
     const { appConfig, externalServices, l10n } = this;
+
     let eventBus;
     if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
-      eventBus = this.preferences.eventBus = new FirefoxEventBus(
+      eventBus = AppOptions.eventBus = new FirefoxEventBus(
         await this._allowedGlobalEventsPromise,
         externalServices,
         AppOptions.get("isInAutomation")

web/app_options.js

@@ -46,6 +46,7 @@ const OptionKind = {
   VIEWER: 0x02,
   API: 0x04,
   WORKER: 0x08,
+  EVENT_DISPATCH: 0x10,
   PREFERENCE: 0x80,
 };
 
@@ -98,7 +99,7 @@ const defaultOptions = {
   toolbarDensity: {
     /** @type {number} */
     value: 0, // 0 = "normal", 1 = "compact", 2 = "touch"
-    kind: OptionKind.BROWSER,
+    kind: OptionKind.BROWSER + OptionKind.EVENT_DISPATCH,
   },
 
   annotationEditorMode: {
@@ -461,6 +462,8 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING || LIB")) {
 }
 
 class AppOptions {
+  static eventBus;
+
   constructor() {
     throw new Error("Cannot initialize AppOptions.");
   }
@@ -488,28 +491,37 @@ class AppOptions {
     userOptions[name] = value;
   }
 
-  static setAll(options, init = false) {
+  static setAll(options, prefs = false) {
-    if ((typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) && init) {
+    let events;
-      if (this.get("disablePreferences")) {
-        // Give custom implementations of the default viewer a simpler way to
-        // opt-out of having the `Preferences` override existing `AppOptions`.
-        return;
-      }
-      for (const name in userOptions) {
-        // Ignore any compatibility-values in the user-options.
-        if (compatibilityParams[name] !== undefined) {
-          continue;
-        }
-        console.warn(
-          "setAll: The Preferences may override manually set AppOptions; " +
-            'please use the "disablePreferences"-option in order to prevent that.'
-        );
-        break;
-      }
-    }
 
     for (const name in options) {
-      userOptions[name] = options[name];
+      const userOption = options[name];
+
+      if (prefs) {
+        const defaultOption = defaultOptions[name];
+
+        if (!defaultOption) {
+          continue;
+        }
+        const { kind, value } = defaultOption;
+
+        if (!(kind & OptionKind.BROWSER || kind & OptionKind.PREFERENCE)) {
+          continue;
+        }
+        if (typeof userOption !== typeof value) {
+          continue;
+        }
+        if (this.eventBus && kind & OptionKind.EVENT_DISPATCH) {
+          (events ||= new Map()).set(name, userOption);
+        }
+      }
+      userOptions[name] = userOption;
+    }
+
+    if (events) {
+      for (const [name, value] of events) {
+        this.eventBus.dispatch(name.toLowerCase(), { source: this, value });
+      }
     }
   }
 
@@ -526,4 +538,26 @@ class AppOptions {
   }
 }
 
+if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
+  AppOptions._checkDisablePreferences = () => {
+    if (AppOptions.get("disablePreferences")) {
+      // Give custom implementations of the default viewer a simpler way to
+      // opt-out of having the `Preferences` override existing `AppOptions`.
+      return true;
+    }
+    for (const name in userOptions) {
+      // Ignore any compatibility-values in the user-options.
+      if (compatibilityParams[name] !== undefined) {
+        continue;
+      }
+      console.warn(
+        "The Preferences may override manually set AppOptions; " +
+          'please use the "disablePreferences"-option to prevent that.'
+      );
+      break;
+    }
+    return false;
+  };
+}
+
 export { AppOptions, OptionKind };

web/preferences.js

@@ -21,24 +21,14 @@ import { AppOptions, OptionKind } from "./app_options.js";
  *   or every time the viewer is loaded.
  */
 class BasePreferences {
-  #browserDefaults = Object.freeze(
-    typeof PDFJSDev === "undefined"
-      ? AppOptions.getAll(OptionKind.BROWSER, /* defaultOnly = */ true)
-      : PDFJSDev.eval("BROWSER_PREFERENCES")
-  );
-
   #defaults = Object.freeze(
     typeof PDFJSDev === "undefined"
       ? AppOptions.getAll(OptionKind.PREFERENCE, /* defaultOnly = */ true)
       : PDFJSDev.eval("DEFAULT_PREFERENCES")
   );
 
-  #prefs = Object.create(null);
-
   #initializedPromise = null;
 
-  static #eventToDispatch = new Set(["toolbarDensity"]);
-
   constructor() {
     if (this.constructor === BasePreferences) {
       throw new Error("Cannot initialize BasePreferences.");
@@ -54,27 +44,24 @@ class BasePreferences {
 
     this.#initializedPromise = this._readFromStorage(this.#defaults).then(
       ({ browserPrefs, prefs }) => {
-        const options = Object.create(null);
+        if (
-
+          (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
-        for (const [name, val] of Object.entries(this.#browserDefaults)) {
+          AppOptions._checkDisablePreferences()
-          const prefVal = browserPrefs?.[name];
+        ) {
-          options[name] = typeof prefVal === typeof val ? prefVal : val;
+          return;
         }
-        for (const [name, val] of Object.entries(this.#defaults)) {
+        AppOptions.setAll({ ...browserPrefs, ...prefs }, /* prefs = */ true);
-          const prefVal = prefs?.[name];
-          // Ignore preferences whose types don't match the default values.
-          options[name] = this.#prefs[name] =
-            typeof prefVal === typeof val ? prefVal : val;
-        }
-        AppOptions.setAll(options, /* init = */ true);
       }
     );
 
     if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
-      window.addEventListener("updatedPreference", evt => {
+      window.addEventListener(
-        this.#updatePref(evt.detail);
+        "updatedPreference",
-      });
+        async ({ detail: { name, value } }) => {
-      this.eventBus = null;
+          await this.#initializedPromise;
+          AppOptions.setAll({ [name]: value }, /* prefs = */ true);
+        }
+      );
     }
   }
 
@@ -98,31 +85,6 @@ class BasePreferences {
     throw new Error("Not implemented: _readFromStorage");
   }
 
-  async #updatePref({ name, value }) {
-    if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
-      throw new Error("Not implemented: #updatePref");
-    }
-    await this.#initializedPromise;
-
-    if (name in this.#browserDefaults) {
-      if (typeof value !== typeof this.#browserDefaults[name]) {
-        return; // Invalid preference value.
-      }
-    } else if (name in this.#defaults) {
-      if (typeof value !== typeof this.#defaults[name]) {
-        return; // Invalid preference value.
-      }
-      this.#prefs[name] = value;
-    } else {
-      return; // Invalid preference.
-    }
-    AppOptions.set(name, value);
-
-    if (BasePreferences.#eventToDispatch.has(name)) {
-      this.eventBus?.dispatch(name.toLowerCase(), { source: this, value });
-    }
-  }
-
   /**
    * Reset the preferences to their default values and update storage.
    * @returns {Promise} A promise that is resolved when the preference values
@@ -133,16 +95,9 @@ class BasePreferences {
       throw new Error("Please use `about:config` to change preferences.");
     }
     await this.#initializedPromise;
-    const oldPrefs = structuredClone(this.#prefs);
+    AppOptions.setAll(this.#defaults, /* prefs = */ true);
 
-    this.#prefs = Object.create(null);
-    try {
     await this._writeToStorage(this.#defaults);
-    } catch (reason) {
-      // Revert all preference values, since writing to storage failed.
-      this.#prefs = oldPrefs;
-      throw reason;
-    }
   }
 
   /**
@@ -157,37 +112,10 @@ class BasePreferences {
       throw new Error("Please use `about:config` to change preferences.");
     }
     await this.#initializedPromise;
-    const defaultValue = this.#defaults[name],
+    AppOptions.setAll({ [name]: value }, /* prefs = */ true);
-      oldPrefs = structuredClone(this.#prefs);
 
-    if (defaultValue === undefined) {
+    const prefs = AppOptions.getAll(OptionKind.PREFERENCE);
-      throw new Error(`Set preference: "${name}" is undefined.`);
+    await this._writeToStorage(prefs);
-    } else if (value === undefined) {
-      throw new Error("Set preference: no value is specified.");
-    }
-    const valueType = typeof value,
-      defaultType = typeof defaultValue;
-
-    if (valueType !== defaultType) {
-      if (valueType === "number" && defaultType === "string") {
-        value = value.toString();
-      } else {
-        throw new Error(
-          `Set preference: "${value}" is a ${valueType}, expected a ${defaultType}.`
-        );
-      }
-    } else if (valueType === "number" && !Number.isInteger(value)) {
-      throw new Error(`Set preference: "${value}" must be an integer.`);
-    }
-
-    this.#prefs[name] = value;
-    try {
-      await this._writeToStorage(this.#prefs);
-    } catch (reason) {
-      // Revert all preference values, since writing to storage failed.
-      this.#prefs = oldPrefs;
-      throw reason;
-    }
   }
 
   /**
@@ -201,12 +129,7 @@ class BasePreferences {
       throw new Error("Not implemented: get");
     }
     await this.#initializedPromise;
-    const defaultValue = this.#defaults[name];
+    return AppOptions.get(name);
-
-    if (defaultValue === undefined) {
-      throw new Error(`Get preference: "${name}" is undefined.`);
-    }
-    return this.#prefs[name] ?? defaultValue;
   }
 
   get initializedPromise() {