Re-factor DefaultExternalServices into a regular class, without sta…

…tic 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.
timvandermeij · Jan 27, 2024 · 5dd25b6 · 5dd25b6
1 parent d1080e7
commit 5dd25b6
Show file tree

Hide file tree

Showing 9 changed files with 80 additions and 61 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,34 +379,33 @@ 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,
     ]);
     return new L10n(localeProperties, document.l10n);
   }
 
-  static createScripting() {
+  createScripting() {
     return FirefoxScripting;
   }
 
-  static async getNimbusExperimentData() {
+  async getNimbusExperimentData() {
     const nimbusData = await FirefoxCom.requestAsync(
       "getNimbusExperimentData",
       null
     );
     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
@@ -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
@@ -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
@@ -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",