From 5dd25b6e80d4f71677d5a47d437922e2bb18efa8 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 26 Jan 2024 14:31:42 +0100 Subject: [PATCH] Re-factor `DefaultExternalServices` into a regular class, without static methods The `DefaultExternalServices` code, which is used to provide build-specific functionality, is very old. This results in a pattern where we first initialize `PDFViewerApplication.externalServices` and then *override* it for the different builds. By converting `DefaultExternalServices` into a "regular" class, and leveraging import maps, we can directly initialize the correct instance depending on the build. --- gulpfile.mjs | 4 ++++ test/unit/unit_test.html | 1 + web/app.js | 39 +++------------------------------ web/chromecom.js | 14 ++++++------ web/external_services.js | 46 +++++++++++++++++++++++++++++++++++++++ web/firefoxcom.js | 24 ++++++++++---------- web/genericcom.js | 11 +++++----- web/viewer-geckoview.html | 1 + web/viewer.html | 1 + 9 files changed, 80 insertions(+), 61 deletions(-) create mode 100644 web/external_services.js diff --git a/gulpfile.mjs b/gulpfile.mjs index 2923f79357535..966225af19d23 100644 --- a/gulpfile.mjs +++ b/gulpfile.mjs @@ -271,6 +271,7 @@ function createWebpackConfig( "web-annotation_editor_params": "web/annotation_editor_params.js", "web-com": "", "web-download_manager": "", + "web-external_services": "", "web-l10n_utils": "web/stubs.js", "web-pdf_attachment_viewer": "web/pdf_attachment_viewer.js", "web-pdf_cursor_tools": "web/pdf_cursor_tools.js", @@ -292,6 +293,7 @@ function createWebpackConfig( viewerAlias["web-com"] = "web/chromecom.js"; viewerAlias["web-download_manager"] = "web/download_manager.js"; + viewerAlias["web-external_services"] = "web/chromecom.js"; viewerAlias["web-preferences"] = "web/chromecom.js"; viewerAlias["web-print_service"] = "web/pdf_print_service.js"; } else if (bundleDefines.GENERIC) { @@ -305,6 +307,7 @@ function createWebpackConfig( viewerAlias["web-com"] = "web/genericcom.js"; viewerAlias["web-download_manager"] = "web/download_manager.js"; + viewerAlias["web-external_services"] = "web/genericcom.js"; viewerAlias["web-l10n_utils"] = "web/l10n_utils.js"; viewerAlias["web-preferences"] = "web/genericcom.js"; viewerAlias["web-print_service"] = "web/pdf_print_service.js"; @@ -320,6 +323,7 @@ function createWebpackConfig( } viewerAlias["web-com"] = "web/firefoxcom.js"; viewerAlias["web-download_manager"] = "web/firefoxcom.js"; + viewerAlias["web-external_services"] = "web/firefoxcom.js"; viewerAlias["web-preferences"] = "web/firefoxcom.js"; viewerAlias["web-print_service"] = "web/firefox_print_service.js"; } diff --git a/test/unit/unit_test.html b/test/unit/unit_test.html index 4066ba2a7ac37..add51eb7c1676 100644 --- a/test/unit/unit_test.html +++ b/test/unit/unit_test.html @@ -29,6 +29,7 @@ "web-annotation_editor_params": "../../web/annotation_editor_params.js", "web-com": "../../web/genericcom.js", "web-download_manager": "../../web/download_manager.js", + "web-external_services": "../../web/genericcom.js", "web-l10n_utils": "../../web/l10n_utils.js", "web-pdf_attachment_viewer": "../../web/pdf_attachment_viewer.js", "web-pdf_cursor_tools": "../../web/pdf_cursor_tools.js", diff --git a/web/app.js b/web/app.js index 1d50adfea5ec2..3997cb87eef03 100644 --- a/web/app.js +++ b/web/app.js @@ -57,6 +57,7 @@ import { LinkTarget, PDFLinkService } from "./pdf_link_service.js"; import { AltTextManager } from "web-alt_text_manager"; import { AnnotationEditorParams } from "web-annotation_editor_params"; import { DownloadManager } from "web-download_manager"; +import { ExternalServices } from "web-external_services"; import { OverlayManager } from "./overlay_manager.js"; import { PasswordPrompt } from "./password_prompt.js"; import { PDFAttachmentViewer } from "web-pdf_attachment_viewer"; @@ -87,36 +88,6 @@ const ViewOnLoad = { INITIAL: 1, }; -class DefaultExternalServices { - constructor() { - throw new Error("Cannot initialize DefaultExternalServices."); - } - - static updateFindControlState(data) {} - - static updateFindMatchesCount(data) {} - - static initPassiveLoading(callbacks) {} - - static reportTelemetry(data) {} - - static async createL10n() { - throw new Error("Not implemented: createL10n"); - } - - static createScripting() { - throw new Error("Not implemented: createScripting"); - } - - static updateEditorStates(data) { - throw new Error("Not implemented: updateEditorStates"); - } - - static getNimbusExperimentData() { - return shadow(this, "getNimbusExperimentData", Promise.resolve(null)); - } -} - const PDFViewerApplication = { initialBookmark: document.location.hash.substring(1), _initializedCapability: new PromiseCapability(), @@ -174,7 +145,7 @@ const PDFViewerApplication = { url: "", baseUrl: "", _downloadUrl: "", - externalServices: DefaultExternalServices, + externalServices: new ExternalServices(), _boundEvents: Object.create(null), documentInfo: null, metadata: null, @@ -3217,8 +3188,4 @@ const PDFPrintServiceFactory = { }, }; -export { - DefaultExternalServices, - PDFPrintServiceFactory, - PDFViewerApplication, -}; +export { PDFPrintServiceFactory, PDFViewerApplication }; diff --git a/web/chromecom.js b/web/chromecom.js index e8f6b7952a3ac..6ba500509c05e 100644 --- a/web/chromecom.js +++ b/web/chromecom.js @@ -14,11 +14,12 @@ */ /* globals chrome */ -import { DefaultExternalServices, PDFViewerApplication } from "./app.js"; import { AppOptions } from "./app_options.js"; +import { BaseExternalServices } from "./external_services.js"; import { BasePreferences } from "./preferences.js"; import { GenericL10n } from "./genericl10n.js"; import { GenericScripting } from "./generic_scripting.js"; +import { PDFViewerApplication } from "./app.js"; if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("CHROME")) { throw new Error( @@ -414,8 +415,8 @@ class Preferences extends BasePreferences { } } -class ChromeExternalServices extends DefaultExternalServices { - static initPassiveLoading(callbacks) { +class ExternalServices extends BaseExternalServices { + initPassiveLoading(callbacks) { // defaultUrl is set in viewer.js ChromeCom.resolvePDFFile( AppOptions.get("defaultUrl"), @@ -426,14 +427,13 @@ class ChromeExternalServices extends DefaultExternalServices { ); } - static async createL10n() { + async createL10n() { return new GenericL10n(navigator.language); } - static createScripting() { + createScripting() { return new GenericScripting(AppOptions.get("sandboxBundleSrc")); } } -PDFViewerApplication.externalServices = ChromeExternalServices; -export { ChromeCom, Preferences }; +export { ChromeCom, ExternalServices, Preferences }; diff --git a/web/external_services.js b/web/external_services.js new file mode 100644 index 0000000000000..1fa42cd43dafa --- /dev/null +++ b/web/external_services.js @@ -0,0 +1,46 @@ +/* Copyright 2024 Mozilla Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class BaseExternalServices { + constructor() { + if (this.constructor === BaseExternalServices) { + throw new Error("Cannot initialize BaseExternalServices."); + } + } + + updateFindControlState(data) {} + + updateFindMatchesCount(data) {} + + initPassiveLoading(callbacks) {} + + reportTelemetry(data) {} + + async createL10n() { + throw new Error("Not implemented: createL10n"); + } + + createScripting() { + throw new Error("Not implemented: createScripting"); + } + + updateEditorStates(data) { + throw new Error("Not implemented: updateEditorStates"); + } + + async getNimbusExperimentData() {} +} + +export { BaseExternalServices }; diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 2c2966b442b01..06b5f1b194db0 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -13,11 +13,12 @@ * limitations under the License. */ -import { DefaultExternalServices, PDFViewerApplication } from "./app.js"; import { isPdfFile, PDFDataRangeTransport } from "pdfjs-lib"; +import { BaseExternalServices } from "./external_services.js"; import { BasePreferences } from "./preferences.js"; import { DEFAULT_SCALE_VALUE } from "./ui_utils.js"; import { L10n } from "./l10n.js"; +import { PDFViewerApplication } from "./app.js"; if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) { throw new Error( @@ -309,16 +310,16 @@ class FirefoxScripting { } } -class FirefoxExternalServices extends DefaultExternalServices { - static updateFindControlState(data) { +class ExternalServices extends BaseExternalServices { + updateFindControlState(data) { FirefoxCom.request("updateFindControlState", data); } - static updateFindMatchesCount(data) { + updateFindMatchesCount(data) { FirefoxCom.request("updateFindMatchesCount", data); } - static initPassiveLoading(callbacks) { + initPassiveLoading(callbacks) { let pdfDataRangeTransport; window.addEventListener("message", function windowMessage(e) { @@ -378,15 +379,15 @@ class FirefoxExternalServices extends DefaultExternalServices { FirefoxCom.request("initPassiveLoading", null); } - static reportTelemetry(data) { + reportTelemetry(data) { FirefoxCom.request("reportTelemetry", JSON.stringify(data)); } - static updateEditorStates(data) { + updateEditorStates(data) { FirefoxCom.request("updateEditorStates", data); } - static async createL10n() { + async createL10n() { const [localeProperties] = await Promise.all([ FirefoxCom.requestAsync("getLocaleProperties", null), document.l10n.ready, @@ -394,11 +395,11 @@ class FirefoxExternalServices extends DefaultExternalServices { return new L10n(localeProperties, document.l10n); } - static createScripting() { + createScripting() { return FirefoxScripting; } - static async getNimbusExperimentData() { + async getNimbusExperimentData() { const nimbusData = await FirefoxCom.requestAsync( "getNimbusExperimentData", null @@ -406,6 +407,5 @@ class FirefoxExternalServices extends DefaultExternalServices { return nimbusData && JSON.parse(nimbusData); } } -PDFViewerApplication.externalServices = FirefoxExternalServices; -export { DownloadManager, FirefoxCom, Preferences }; +export { DownloadManager, ExternalServices, FirefoxCom, Preferences }; diff --git a/web/genericcom.js b/web/genericcom.js index 1ad633f5ee672..f4c48f4783680 100644 --- a/web/genericcom.js +++ b/web/genericcom.js @@ -13,8 +13,8 @@ * limitations under the License. */ -import { DefaultExternalServices, PDFViewerApplication } from "./app.js"; import { AppOptions } from "./app_options.js"; +import { BaseExternalServices } from "./external_services.js"; import { BasePreferences } from "./preferences.js"; import { GenericL10n } from "./genericl10n.js"; import { GenericScripting } from "./generic_scripting.js"; @@ -37,15 +37,14 @@ class Preferences extends BasePreferences { } } -class GenericExternalServices extends DefaultExternalServices { - static async createL10n() { +class ExternalServices extends BaseExternalServices { + async createL10n() { return new GenericL10n(AppOptions.get("locale")); } - static createScripting() { + createScripting() { return new GenericScripting(AppOptions.get("sandboxBundleSrc")); } } -PDFViewerApplication.externalServices = GenericExternalServices; -export { GenericCom, Preferences }; +export { ExternalServices, GenericCom, Preferences }; diff --git a/web/viewer-geckoview.html b/web/viewer-geckoview.html index 1f72425e6e208..af03111b45e82 100644 --- a/web/viewer-geckoview.html +++ b/web/viewer-geckoview.html @@ -62,6 +62,7 @@ "web-annotation_editor_params": "./stubs-geckoview.js", "web-com": "./genericcom.js", "web-download_manager": "./download_manager.js", + "web-external_services": "./genericcom.js", "web-l10n_utils": "./l10n_utils.js", "web-pdf_attachment_viewer": "./stubs-geckoview.js", "web-pdf_cursor_tools": "./stubs-geckoview.js", diff --git a/web/viewer.html b/web/viewer.html index fa0c55c1d74c2..d0fa501a6e7c7 100644 --- a/web/viewer.html +++ b/web/viewer.html @@ -71,6 +71,7 @@ "web-annotation_editor_params": "./annotation_editor_params.js", "web-com": "./genericcom.js", "web-download_manager": "./download_manager.js", + "web-external_services": "./genericcom.js", "web-l10n_utils": "./l10n_utils.js", "web-pdf_attachment_viewer": "./pdf_attachment_viewer.js", "web-pdf_cursor_tools": "./pdf_cursor_tools.js",