From b0fa02e8456b6a27d6d3b242c4301a4e70937b17 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 30 Jul 2018 16:28:39 +0200 Subject: [PATCH 1/6] Refactor the `IL10n` implementations to utilize `async` methods rather than manually returning `Promise`s This changes the methods signatures of `GenericL10n`, `MozL10n`, and `NullL10n`. --- web/firefoxcom.js | 15 +++++++-------- web/genericl10n.js | 28 ++++++++++++---------------- web/interfaces.js | 8 ++++---- web/ui_utils.js | 16 +++++++--------- 4 files changed, 30 insertions(+), 37 deletions(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 99026e9faac07..2aea61b0f0809 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -145,21 +145,20 @@ class MozL10n { this.mozL10n = mozL10n; } - getLanguage() { - return Promise.resolve(this.mozL10n.getLanguage()); + async getLanguage() { + return this.mozL10n.getLanguage(); } - getDirection() { - return Promise.resolve(this.mozL10n.getDirection()); + async getDirection() { + return this.mozL10n.getDirection(); } - get(property, args, fallback) { - return Promise.resolve(this.mozL10n.get(property, args, fallback)); + async get(property, args, fallback) { + return this.mozL10n.get(property, args, fallback); } - translate(element) { + async translate(element) { this.mozL10n.translate(element); - return Promise.resolve(); } } diff --git a/web/genericl10n.js b/web/genericl10n.js index e2da69a605270..c722c289ce943 100644 --- a/web/genericl10n.js +++ b/web/genericl10n.js @@ -27,28 +27,24 @@ class GenericL10n { }); } - getLanguage() { - return this._ready.then((l10n) => { - return l10n.getLanguage(); - }); + async getLanguage() { + const l10n = await this._ready; + return l10n.getLanguage(); } - getDirection() { - return this._ready.then((l10n) => { - return l10n.getDirection(); - }); + async getDirection() { + const l10n = await this._ready; + return l10n.getDirection(); } - get(property, args, fallback) { - return this._ready.then((l10n) => { - return l10n.get(property, args, fallback); - }); + async get(property, args, fallback) { + const l10n = await this._ready; + return l10n.get(property, args, fallback); } - translate(element) { - return this._ready.then((l10n) => { - return l10n.translate(element); - }); + async translate(element) { + const l10n = await this._ready; + return l10n.translate(element); } } diff --git a/web/interfaces.js b/web/interfaces.js index 2e8b52d1d3024..62e4c2ce88c52 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -163,12 +163,12 @@ class IL10n { /** * @returns {Promise} - Resolves to the current locale. */ - getLanguage() {} + async getLanguage() {} /** * @returns {Promise} - Resolves to 'rtl' or 'ltr'. */ - getDirection() {} + async getDirection() {} /** * Translates text identified by the key and adds/formats data using the args @@ -179,12 +179,12 @@ class IL10n { * @param {string} fallback * @returns {Promise} */ - get(key, args, fallback) { } + async get(key, args, fallback) { } /** * Translates HTML element. * @param {HTMLElement} element * @returns {Promise} */ - translate(element) { } + async translate(element) { } } diff --git a/web/ui_utils.js b/web/ui_utils.js index d87b78c153764..d0e59d58de56f 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -56,21 +56,19 @@ function formatL10nValue(text, args) { * @implements {IL10n} */ let NullL10n = { - getLanguage() { - return Promise.resolve('en-us'); + async getLanguage() { + return 'en-us'; }, - getDirection() { - return Promise.resolve('ltr'); + async getDirection() { + return 'ltr'; }, - get(property, args, fallback) { - return Promise.resolve(formatL10nValue(fallback, args)); + async get(property, args, fallback) { + return formatL10nValue(fallback, args); }, - translate(element) { - return Promise.resolve(); - }, + async translate(element) { }, }; /** From 64e70fc16ffb102524215d02b5b3ebabddbacc09 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 30 Jul 2018 16:39:06 +0200 Subject: [PATCH 2/6] Refactor the `OverlayManager` to utilize `async` methods rather than manually returning `Promise`s --- web/overlay_manager.js | 107 ++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 59 deletions(-) diff --git a/web/overlay_manager.js b/web/overlay_manager.js index 11a927412c94a..4612440761ad8 100644 --- a/web/overlay_manager.js +++ b/web/overlay_manager.js @@ -37,22 +37,20 @@ class OverlayManager { * @returns {Promise} A promise that is resolved when the overlay has been * registered. */ - register(name, element, callerCloseMethod = null, canForceClose = false) { - return new Promise((resolve) => { - let container; - if (!name || !element || !(container = element.parentNode)) { - throw new Error('Not enough parameters.'); - } else if (this._overlays[name]) { - throw new Error('The overlay is already registered.'); - } - this._overlays[name] = { - element, - container, - callerCloseMethod, - canForceClose, - }; - resolve(); - }); + async register(name, element, callerCloseMethod = null, + canForceClose = false) { + let container; + if (!name || !element || !(container = element.parentNode)) { + throw new Error('Not enough parameters.'); + } else if (this._overlays[name]) { + throw new Error('The overlay is already registered.'); + } + this._overlays[name] = { + element, + container, + callerCloseMethod, + canForceClose, + }; } /** @@ -60,16 +58,13 @@ class OverlayManager { * @returns {Promise} A promise that is resolved when the overlay has been * unregistered. */ - unregister(name) { - return new Promise((resolve) => { - if (!this._overlays[name]) { - throw new Error('The overlay does not exist.'); - } else if (this._active === name) { - throw new Error('The overlay cannot be removed while it is active.'); - } - delete this._overlays[name]; - resolve(); - }); + async unregister(name) { + if (!this._overlays[name]) { + throw new Error('The overlay does not exist.'); + } else if (this._active === name) { + throw new Error('The overlay cannot be removed while it is active.'); + } + delete this._overlays[name]; } /** @@ -77,26 +72,23 @@ class OverlayManager { * @returns {Promise} A promise that is resolved when the overlay has been * opened. */ - open(name) { - return new Promise((resolve) => { - if (!this._overlays[name]) { - throw new Error('The overlay does not exist.'); - } else if (this._active) { - if (this._overlays[name].canForceClose) { - this._closeThroughCaller(); - } else if (this._active === name) { - throw new Error('The overlay is already active.'); - } else { - throw new Error('Another overlay is currently active.'); - } + async open(name) { + if (!this._overlays[name]) { + throw new Error('The overlay does not exist.'); + } else if (this._active) { + if (this._overlays[name].canForceClose) { + this._closeThroughCaller(); + } else if (this._active === name) { + throw new Error('The overlay is already active.'); + } else { + throw new Error('Another overlay is currently active.'); } - this._active = name; - this._overlays[this._active].element.classList.remove('hidden'); - this._overlays[this._active].container.classList.remove('hidden'); + } + this._active = name; + this._overlays[this._active].element.classList.remove('hidden'); + this._overlays[this._active].container.classList.remove('hidden'); - window.addEventListener('keydown', this._keyDownBound); - resolve(); - }); + window.addEventListener('keydown', this._keyDownBound); } /** @@ -104,22 +96,19 @@ class OverlayManager { * @returns {Promise} A promise that is resolved when the overlay has been * closed. */ - close(name) { - return new Promise((resolve) => { - if (!this._overlays[name]) { - throw new Error('The overlay does not exist.'); - } else if (!this._active) { - throw new Error('The overlay is currently not active.'); - } else if (this._active !== name) { - throw new Error('Another overlay is currently active.'); - } - this._overlays[this._active].container.classList.add('hidden'); - this._overlays[this._active].element.classList.add('hidden'); - this._active = null; + async close(name) { + if (!this._overlays[name]) { + throw new Error('The overlay does not exist.'); + } else if (!this._active) { + throw new Error('The overlay is currently not active.'); + } else if (this._active !== name) { + throw new Error('Another overlay is currently active.'); + } + this._overlays[this._active].container.classList.add('hidden'); + this._overlays[this._active].element.classList.add('hidden'); + this._active = null; - window.removeEventListener('keydown', this._keyDownBound); - resolve(); - }); + window.removeEventListener('keydown', this._keyDownBound); } /** From 233b3274bf7973aa06a674dddef585f4313f3742 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 30 Jul 2018 16:48:16 +0200 Subject: [PATCH 3/6] Refactor the `Preferences` classes to utilize `async` methods rather than manually returning `Promise`s --- web/chromecom.js | 4 +- web/firefoxcom.js | 4 +- web/genericcom.js | 14 ++----- web/preferences.js | 92 +++++++++++++++++++++++----------------------- 4 files changed, 53 insertions(+), 61 deletions(-) diff --git a/web/chromecom.js b/web/chromecom.js index da39d2b527cc7..48eecb6ccc35e 100644 --- a/web/chromecom.js +++ b/web/chromecom.js @@ -300,7 +300,7 @@ function setReferer(url, callback) { let storageArea = chrome.storage.sync || chrome.storage.local; class ChromePreferences extends BasePreferences { - _writeToStorage(prefObj) { + async _writeToStorage(prefObj) { return new Promise((resolve) => { if (prefObj === this.defaults) { let keysToRemove = Object.keys(this.defaults); @@ -317,7 +317,7 @@ class ChromePreferences extends BasePreferences { }); } - _readFromStorage(prefObj) { + async _readFromStorage(prefObj) { return new Promise((resolve) => { let getPreferences = (defaultPrefs) => { if (chrome.runtime.lastError) { diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 2aea61b0f0809..9100102e7693c 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -124,13 +124,13 @@ class DownloadManager { } class FirefoxPreferences extends BasePreferences { - _writeToStorage(prefObj) { + async _writeToStorage(prefObj) { return new Promise(function(resolve) { FirefoxCom.request('setPreferences', prefObj, resolve); }); } - _readFromStorage(prefObj) { + async _readFromStorage(prefObj) { return new Promise(function(resolve) { FirefoxCom.request('getPreferences', prefObj, function(prefStr) { let readPrefs = JSON.parse(prefStr); diff --git a/web/genericcom.js b/web/genericcom.js index 89ffd90e39eba..794bfbfbdfc32 100644 --- a/web/genericcom.js +++ b/web/genericcom.js @@ -26,18 +26,12 @@ if (typeof PDFJSDev !== 'undefined' && !PDFJSDev.test('GENERIC')) { let GenericCom = {}; class GenericPreferences extends BasePreferences { - _writeToStorage(prefObj) { - return new Promise(function(resolve) { - localStorage.setItem('pdfjs.preferences', JSON.stringify(prefObj)); - resolve(); - }); + async _writeToStorage(prefObj) { + localStorage.setItem('pdfjs.preferences', JSON.stringify(prefObj)); } - _readFromStorage(prefObj) { - return new Promise(function(resolve) { - let readPrefs = JSON.parse(localStorage.getItem('pdfjs.preferences')); - resolve(readPrefs); - }); + async _readFromStorage(prefObj) { + return JSON.parse(localStorage.getItem('pdfjs.preferences')); } } diff --git a/web/preferences.js b/web/preferences.js index a5fa67388279a..7cc9f3a153291 100644 --- a/web/preferences.js +++ b/web/preferences.js @@ -83,8 +83,8 @@ class BasePreferences { * @return {Promise} A promise that is resolved when the preference values * have been written. */ - _writeToStorage(prefObj) { - return Promise.reject(new Error('Not implemented: _writeToStorage')); + async _writeToStorage(prefObj) { + throw new Error('Not implemented: _writeToStorage'); } /** @@ -93,8 +93,8 @@ class BasePreferences { * @return {Promise} A promise that is resolved with an {Object} containing * the preferences that have been read. */ - _readFromStorage(prefObj) { - return Promise.reject(new Error('Not implemented: _readFromStorage')); + async _readFromStorage(prefObj) { + throw new Error('Not implemented: _readFromStorage'); } /** @@ -102,11 +102,10 @@ class BasePreferences { * @return {Promise} A promise that is resolved when the preference values * have been reset. */ - reset() { - return this._initializedPromise.then(() => { - this.prefs = Object.assign(Object.create(null), this.defaults); - return this._writeToStorage(this.defaults); - }); + async reset() { + await this._initializedPromise; + this.prefs = Object.assign(Object.create(null), this.defaults); + return this._writeToStorage(this.defaults); } /** @@ -116,31 +115,32 @@ class BasePreferences { * @return {Promise} A promise that is resolved when the value has been set, * provided that the preference exists and the types match. */ - set(name, value) { - return this._initializedPromise.then(() => { - if (this.defaults[name] === undefined) { - throw new Error(`Set preference: "${name}" is undefined.`); - } else if (value === undefined) { - throw new Error('Set preference: no value is specified.'); - } - let valueType = typeof value; - let defaultType = typeof this.defaults[name]; + async set(name, value) { + await this._initializedPromise; + let defaultValue = this.defaults[name]; - if (valueType !== defaultType) { - if (valueType === 'number' && defaultType === 'string') { - value = value.toString(); - } else { - throw new Error(`Set preference: "${value}" is a ${valueType}, ` + - `expected a ${defaultType}.`); - } + if (defaultValue === undefined) { + throw new Error(`Set preference: "${name}" is undefined.`); + } else if (value === undefined) { + throw new Error('Set preference: no value is specified.'); + } + let valueType = typeof value; + let defaultType = typeof defaultValue; + + if (valueType !== defaultType) { + if (valueType === 'number' && defaultType === 'string') { + value = value.toString(); } else { - if (valueType === 'number' && !Number.isInteger(value)) { - throw new Error(`Set preference: "${value}" must be an integer.`); - } + throw new Error(`Set preference: "${value}" is a ${valueType}, ` + + `expected a ${defaultType}.`); } - this.prefs[name] = value; - return this._writeToStorage(this.prefs); - }); + } else { + if (valueType === 'number' && !Number.isInteger(value)) { + throw new Error(`Set preference: "${value}" must be an integer.`); + } + } + this.prefs[name] = value; + return this._writeToStorage(this.prefs); } /** @@ -149,21 +149,20 @@ class BasePreferences { * @return {Promise} A promise that is resolved with a {boolean|number|string} * containing the value of the preference. */ - get(name) { - return this._initializedPromise.then(() => { - let defaultValue = this.defaults[name]; + async get(name) { + await this._initializedPromise; + let defaultValue = this.defaults[name]; - if (defaultValue === undefined) { - throw new Error(`Get preference: "${name}" is undefined.`); - } else { - let prefValue = this.prefs[name]; + if (defaultValue === undefined) { + throw new Error(`Get preference: "${name}" is undefined.`); + } else { + let prefValue = this.prefs[name]; - if (prefValue !== undefined) { - return prefValue; - } + if (prefValue !== undefined) { + return prefValue; } - return defaultValue; - }); + } + return defaultValue; } /** @@ -171,10 +170,9 @@ class BasePreferences { * @return {Promise} A promise that is resolved with an {Object} containing * the values of all preferences. */ - getAll() { - return this._initializedPromise.then(() => { - return Object.assign(Object.create(null), this.defaults, this.prefs); - }); + async getAll() { + await this._initializedPromise; + return Object.assign(Object.create(null), this.defaults, this.prefs); } } From a60963f88224235ff6e8847a019dfd93da203738 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 30 Jul 2018 17:07:21 +0200 Subject: [PATCH 4/6] Refactor the `ViewHistory` to utilize `async` methods rather than manually returning `Promise`s --- web/view_history.js | 82 ++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 46 deletions(-) diff --git a/web/view_history.js b/web/view_history.js index c50d8064c9456..5c5857b7e5b95 100644 --- a/web/view_history.js +++ b/web/view_history.js @@ -54,64 +54,54 @@ class ViewHistory { }); } - _writeToStorage() { - return new Promise((resolve) => { - let databaseStr = JSON.stringify(this.database); + async _writeToStorage() { + let databaseStr = JSON.stringify(this.database); - if (typeof PDFJSDev !== 'undefined' && - PDFJSDev.test('FIREFOX || MOZCENTRAL')) { - sessionStorage.setItem('pdfjs.history', databaseStr); - } else { - localStorage.setItem('pdfjs.history', databaseStr); - } - resolve(); - }); + if (typeof PDFJSDev !== 'undefined' && + PDFJSDev.test('FIREFOX || MOZCENTRAL')) { + sessionStorage.setItem('pdfjs.history', databaseStr); + return; + } + localStorage.setItem('pdfjs.history', databaseStr); } - _readFromStorage() { - return new Promise(function(resolve) { - if (typeof PDFJSDev !== 'undefined' && - PDFJSDev.test('FIREFOX || MOZCENTRAL')) { - resolve(sessionStorage.getItem('pdfjs.history')); - } else { - resolve(localStorage.getItem('pdfjs.history')); - } - }); + async _readFromStorage() { + if (typeof PDFJSDev !== 'undefined' && + PDFJSDev.test('FIREFOX || MOZCENTRAL')) { + return sessionStorage.getItem('pdfjs.history'); + } + return localStorage.getItem('pdfjs.history'); } - set(name, val) { - return this._initializedPromise.then(() => { - this.file[name] = val; - return this._writeToStorage(); - }); + async set(name, val) { + await this._initializedPromise; + this.file[name] = val; + return this._writeToStorage(); } - setMultiple(properties) { - return this._initializedPromise.then(() => { - for (let name in properties) { - this.file[name] = properties[name]; - } - return this._writeToStorage(); - }); + async setMultiple(properties) { + await this._initializedPromise; + for (let name in properties) { + this.file[name] = properties[name]; + } + return this._writeToStorage(); } - get(name, defaultValue) { - return this._initializedPromise.then(() => { - let val = this.file[name]; - return val !== undefined ? val : defaultValue; - }); + async get(name, defaultValue) { + await this._initializedPromise; + let val = this.file[name]; + return val !== undefined ? val : defaultValue; } - getMultiple(properties) { - return this._initializedPromise.then(() => { - let values = Object.create(null); + async getMultiple(properties) { + await this._initializedPromise; + let values = Object.create(null); - for (let name in properties) { - let val = this.file[name]; - values[name] = val !== undefined ? val : properties[name]; - } - return values; - }); + for (let name in properties) { + let val = this.file[name]; + values[name] = val !== undefined ? val : properties[name]; + } + return values; } } From 3eba7ea2673828865081d0f188477351803235f5 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 30 Jul 2018 17:41:39 +0200 Subject: [PATCH 5/6] Refactor a number of methods in `PDFViewerApplication` to be `async` rather than manually returning `Promise`s *Ignoring whitespace changes is probably necessary, in order for the diff to be readable.* --- web/app.js | 463 ++++++++++++++++++++++++++--------------------------- 1 file changed, 227 insertions(+), 236 deletions(-) diff --git a/web/app.js b/web/app.js index 4304c4cb9ae11..a4f3b581b4997 100644 --- a/web/app.js +++ b/web/app.js @@ -133,44 +133,42 @@ let PDFViewerApplication = { contentDispositionFilename: null, // Called once when the document is loaded. - initialize(appConfig) { + async initialize(appConfig) { this.preferences = this.externalServices.createPreferences(); this.appConfig = appConfig; - return this._readPreferences().then(() => { - return this._parseHashParameters(); - }).then(() => { - return this._initializeL10n(); - }).then(() => { - if (this.isViewerEmbedded && - AppOptions.get('externalLinkTarget') === LinkTarget.NONE) { - // Prevent external links from "replacing" the viewer, - // when it's embedded in e.g. an