Skip to content

Commit

Permalink
Re-factor BasePreferences to essentially be a wrapper around `AppOp…
Browse files Browse the repository at this point in the history 
…tions`

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.
  • Loading branch information
@Snuffleupagus
Snuffleupagus committed Jul 9, 2024
1 parent 1bdd692 commit 76af87d
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 139 deletions.
24 changes: 0 additions & 24 deletions gulpfile.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
: {},
Expand Down Expand Up @@ -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
Expand All @@ -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")
Expand Down Expand Up @@ -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/"
),
Expand Down
3 changes: 2 additions & 1 deletion web/app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
70 changes: 52 additions & 18 deletions web/app_options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const OptionKind = {
VIEWER: 0x02,
API: 0x04,
WORKER: 0x08,
EVENT_DISPATCH: 0x10,
PREFERENCE: 0x80,
};

Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -461,6 +462,8 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING || LIB")) {
}

class AppOptions {
static eventBus;

constructor() {
throw new Error("Cannot initialize AppOptions.");
}
Expand Down Expand Up @@ -488,28 +491,37 @@ class AppOptions {
userOptions[name] = value;
}

static setAll(options, init = false) {
if ((typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) && init) {
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) {
static setAll(options, prefs = false) {
let events;

for (const name in options) {
const userOption = options[name];

if (prefs) {
const defaultOption = defaultOptions[name];

if (!defaultOption) {
continue;
}
console.warn(
"setAll: The Preferences may override manually set AppOptions; " +
'please use the "disablePreferences"-option in order to prevent that.'
);
break;
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;
}

for (const name in options) {
userOptions[name] = options[name];
if (events) {
for (const [name, value] of events) {
this.eventBus.dispatch(name.toLowerCase(), { source: this, value });
}
}
}

Expand All @@ -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 };
112 changes: 16 additions & 96 deletions web/preferences.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand All @@ -54,27 +44,21 @@ class BasePreferences {

this.#initializedPromise = this._readFromStorage(this.#defaults).then(
({ browserPrefs, prefs }) => {
const options = Object.create(null);

for (const [name, val] of Object.entries(this.#browserDefaults)) {
const prefVal = browserPrefs?.[name];
options[name] = typeof prefVal === typeof val ? prefVal : val;
}
for (const [name, val] of Object.entries(this.#defaults)) {
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;
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
AppOptions._checkDisablePreferences()
) {
return;
}
AppOptions.setAll(options, /* init = */ true);
AppOptions.setAll({ ...browserPrefs, ...prefs }, /* prefs = */ true);
}
);

if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
window.addEventListener("updatedPreference", evt => {
this.#updatePref(evt.detail);
window.addEventListener("updatedPreference", async evt => {
await this.#initializedPromise;
AppOptions.setAll(evt.detail, /* prefs = */ true);
});
this.eventBus = null;
}
}

Expand All @@ -98,31 +82,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
Expand All @@ -133,16 +92,9 @@ class BasePreferences {
throw new Error("Please use `about:config` to change preferences.");
}
await this.#initializedPromise;
const oldPrefs = structuredClone(this.#prefs);

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;
}
AppOptions.setAll(this.#defaults, /* prefs = */ true);

await this._writeToStorage(this.#defaults);
}

/**
Expand All @@ -157,37 +109,10 @@ class BasePreferences {
throw new Error("Please use `about:config` to change preferences.");
}
await this.#initializedPromise;
const defaultValue = this.#defaults[name],
oldPrefs = structuredClone(this.#prefs);
AppOptions.setAll({ [name]: value }, /* prefs = */ true);

if (defaultValue === undefined) {
throw new Error(`Set preference: "${name}" is undefined.`);
} 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;
}
const prefs = AppOptions.getAll(OptionKind.PREFERENCE);
await this._writeToStorage(prefs);
}

/**
Expand All @@ -201,12 +126,7 @@ class BasePreferences {
throw new Error("Not implemented: get");
}
await this.#initializedPromise;
const defaultValue = this.#defaults[name];

if (defaultValue === undefined) {
throw new Error(`Get preference: "${name}" is undefined.`);
}
return this.#prefs[name] ?? defaultValue;
return AppOptions.get(name);
}

get initializedPromise() {
Expand Down

0 comments on commit 76af87d

Please sign in to comment.