Merge pull request #17588 from Snuffleupagus/DefaultExternalServices-lighten

Re-factor `DefaultExternalServices` into a regular class, without static methods

gulpfile.mjs

@@ -270,6 +270,8 @@ function createWebpackConfig(
     "web-alt_text_manager": "web/alt_text_manager.js",
     "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",
@@ -280,6 +282,7 @@ function createWebpackConfig(
     "web-pdf_presentation_mode": "web/pdf_presentation_mode.js",
     "web-pdf_sidebar": "web/pdf_sidebar.js",
     "web-pdf_thumbnail_viewer": "web/pdf_thumbnail_viewer.js",
+    "web-preferences": "",
     "web-print_service": "",
     "web-secondary_toolbar": "web/secondary_toolbar.js",
     "web-toolbar": "web/toolbar.js",
@@ -289,6 +292,9 @@ function createWebpackConfig(
     libraryAlias["display-network"] = "src/display/network.js";
 
     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) {
     // Aliases defined here must also be replicated in the paths section of
@@ -300,7 +306,10 @@ function createWebpackConfig(
     libraryAlias["display-node_utils"] = "src/display/node_utils.js";
 
     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";
   } else if (bundleDefines.MOZCENTRAL) {
     if (bundleDefines.GECKOVIEW) {
@@ -313,6 +322,9 @@ 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";
   }
   const alias = { ...basicAlias, ...libraryAlias, ...viewerAlias };

test/unit/unit_test.html

@@ -28,6 +28,8 @@
         "web-alt_text_manager": "../../web/alt_text_manager.js",
         "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",
@@ -38,6 +40,7 @@
         "web-pdf_presentation_mode": "../../web/pdf_presentation_mode.js",
         "web-pdf_sidebar": "../../web/pdf_sidebar.js",
         "web-pdf_thumbnail_viewer": "../../web/pdf_thumbnail_viewer.js",
+        "web-preferences": "../../web/genericcom.js",
         "web-print_service": "../../web/pdf_print_service.js",
         "web-secondary_toolbar": "../../web/secondary_toolbar.js",
         "web-toolbar": "../../web/toolbar.js"

web/app.js

@@ -56,6 +56,8 @@ import { AutomationEventBus, EventBus } from "./event_utils.js";
 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";
@@ -72,6 +74,7 @@ import { PDFScriptingManager } from "./pdf_scripting_manager.js";
 import { PDFSidebar } from "web-pdf_sidebar";
 import { PDFThumbnailViewer } from "web-pdf_thumbnail_viewer";
 import { PDFViewer } from "./pdf_viewer.js";
+import { Preferences } from "web-preferences";
 import { SecondaryToolbar } from "web-secondary_toolbar";
 import { Toolbar } from "web-toolbar";
 import { ViewHistory } from "./view_history.js";
@@ -85,44 +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 createDownloadManager() {
-    throw new Error("Not implemented: createDownloadManager");
-  }
-
-  static createPreferences() {
-    throw new Error("Not implemented: createPreferences");
-  }
-
-  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(),
@@ -180,7 +145,7 @@ const PDFViewerApplication = {
   url: "",
   baseUrl: "",
   _downloadUrl: "",
-  externalServices: DefaultExternalServices,
+  externalServices: new ExternalServices(),
   _boundEvents: Object.create(null),
   documentInfo: null,
   metadata: null,
@@ -390,8 +355,7 @@ const PDFViewerApplication = {
     });
     this.pdfLinkService = pdfLinkService;
 
-    const downloadManager = externalServices.createDownloadManager();
+    const downloadManager = (this.downloadManager = new DownloadManager());
-    this.downloadManager = downloadManager;
 
     const findController = new PDFFindController({
       linkService: pdfLinkService,
@@ -621,7 +585,7 @@ const PDFViewerApplication = {
   },
 
   async run(config) {
-    this.preferences = this.externalServices.createPreferences();
+    this.preferences = new Preferences();
     await this.initialize(config);
 
     const { appConfig, eventBus } = this;
@@ -3224,8 +3188,4 @@ const PDFPrintServiceFactory = {
   },
 };
 
-export {
+export { PDFPrintServiceFactory, PDFViewerApplication };
-  DefaultExternalServices,
-  PDFPrintServiceFactory,
-  PDFViewerApplication,
-};

web/chromecom.js

@@ -14,12 +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 { DownloadManager } from "./download_manager.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(
@@ -326,7 +326,7 @@ function setReferer(url, callback) {
 // chrome.storage.local to chrome.storage.sync when needed.
 const storageArea = chrome.storage.sync || chrome.storage.local;
 
-class ChromePreferences extends BasePreferences {
+class Preferences extends BasePreferences {
   async _writeToStorage(prefObj) {
     return new Promise(resolve => {
       if (prefObj === this.defaults) {
@@ -415,8 +415,8 @@ class ChromePreferences extends BasePreferences {
   }
 }
 
-class ChromeExternalServices extends DefaultExternalServices {
+class ExternalServices extends BaseExternalServices {
-  static initPassiveLoading(callbacks) {
+  initPassiveLoading(callbacks) {
     // defaultUrl is set in viewer.js
     ChromeCom.resolvePDFFile(
       AppOptions.get("defaultUrl"),
@@ -427,22 +427,13 @@ class ChromeExternalServices extends DefaultExternalServices {
     );
   }
 
-  static createDownloadManager() {
+  async createL10n() {
-    return new DownloadManager();
-  }
-
-  static createPreferences() {
-    return new ChromePreferences();
-  }
-
-  static async createL10n() {
     return new GenericL10n(navigator.language);
   }
 
-  static createScripting() {
+  createScripting() {
     return new GenericScripting(AppOptions.get("sandboxBundleSrc"));
   }
 }
-PDFViewerApplication.externalServices = ChromeExternalServices;
 
-export { ChromeCom };
+export { ChromeCom, ExternalServices, Preferences };

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 };

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(
@@ -147,7 +148,7 @@ class DownloadManager {
   }
 }
 
-class FirefoxPreferences extends BasePreferences {
+class Preferences extends BasePreferences {
   async _readFromStorage(prefObj) {
     return FirefoxCom.requestAsync("getPreferences", prefObj);
   }
@@ -309,16 +310,16 @@ class FirefoxScripting {
   }
 }
 
-class FirefoxExternalServices extends DefaultExternalServices {
+class ExternalServices extends BaseExternalServices {
-  static updateFindControlState(data) {
+  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,23 +379,15 @@ class FirefoxExternalServices extends DefaultExternalServices {
     FirefoxCom.request("initPassiveLoading", null);
   }
 
-  static reportTelemetry(data) {
+  reportTelemetry(data) {
     FirefoxCom.request("reportTelemetry", JSON.stringify(data));
   }
 
-  static createDownloadManager() {
+  updateEditorStates(data) {
-    return new DownloadManager();
-  }
-
-  static createPreferences() {
-    return new FirefoxPreferences();
-  }
-
-  static updateEditorStates(data) {
     FirefoxCom.request("updateEditorStates", data);
   }
 
-  static async createL10n() {
+  async createL10n() {
     const [localeProperties] = await Promise.all([
       FirefoxCom.requestAsync("getLocaleProperties", null),
       document.l10n.ready,
@@ -402,11 +395,14 @@ class FirefoxExternalServices extends DefaultExternalServices {
     return new L10n(localeProperties, document.l10n);
   }
 
-  static createScripting() {
+  createScripting() {
     return FirefoxScripting;
   }
 
-  static async getNimbusExperimentData() {
+  async getNimbusExperimentData() {
+    if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("GECKOVIEW")) {
+      return null;
+    }
     const nimbusData = await FirefoxCom.requestAsync(
       "getNimbusExperimentData",
       null
@@ -414,6 +410,5 @@ class FirefoxExternalServices extends DefaultExternalServices {
     return nimbusData && JSON.parse(nimbusData);
   }
 }
-PDFViewerApplication.externalServices = FirefoxExternalServices;
 
-export { DownloadManager, FirefoxCom };
+export { DownloadManager, ExternalServices, FirefoxCom, Preferences };

web/genericcom.js

@@ -13,10 +13,9 @@
  * 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 { DownloadManager } from "./download_manager.js";
 import { GenericL10n } from "./genericl10n.js";
 import { GenericScripting } from "./generic_scripting.js";
 
@@ -28,7 +27,7 @@ if (typeof PDFJSDev !== "undefined" && !PDFJSDev.test("GENERIC")) {
 
 const GenericCom = {};
 
-class GenericPreferences extends BasePreferences {
+class Preferences extends BasePreferences {
   async _writeToStorage(prefObj) {
     localStorage.setItem("pdfjs.preferences", JSON.stringify(prefObj));
   }
@@ -38,23 +37,14 @@ class GenericPreferences extends BasePreferences {
   }
 }
 
-class GenericExternalServices extends DefaultExternalServices {
+class ExternalServices extends BaseExternalServices {
-  static createDownloadManager() {
+  async createL10n() {
-    return new DownloadManager();
-  }
-
-  static createPreferences() {
-    return new GenericPreferences();
-  }
-
-  static async createL10n() {
     return new GenericL10n(AppOptions.get("locale"));
   }
 
-  static createScripting() {
+  createScripting() {
     return new GenericScripting(AppOptions.get("sandboxBundleSrc"));
   }
 }
-PDFViewerApplication.externalServices = GenericExternalServices;
 
-export { GenericCom };
+export { ExternalServices, GenericCom, Preferences };

web/viewer-geckoview.html

@@ -61,6 +61,8 @@ See https://github.com/adobe-type-tools/cmap-resources
           "web-alt_text_manager": "./stubs-geckoview.js",
           "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",
@@ -71,6 +73,7 @@ See https://github.com/adobe-type-tools/cmap-resources
           "web-pdf_presentation_mode": "./stubs-geckoview.js",
           "web-pdf_sidebar": "./stubs-geckoview.js",
           "web-pdf_thumbnail_viewer": "./stubs-geckoview.js",
+          "web-preferences": "./genericcom.js",
           "web-print_service": "./pdf_print_service.js",
           "web-secondary_toolbar": "./stubs-geckoview.js",
           "web-toolbar": "./toolbar-geckoview.js"

web/viewer.html

@@ -70,6 +70,8 @@ See https://github.com/adobe-type-tools/cmap-resources
           "web-alt_text_manager": "./alt_text_manager.js",
           "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",
@@ -80,6 +82,7 @@ See https://github.com/adobe-type-tools/cmap-resources
           "web-pdf_presentation_mode": "./pdf_presentation_mode.js",
           "web-pdf_sidebar": "./pdf_sidebar.js",
           "web-pdf_thumbnail_viewer": "./pdf_thumbnail_viewer.js",
+          "web-preferences": "./genericcom.js",
           "web-print_service": "./pdf_print_service.js",
           "web-secondary_toolbar": "./secondary_toolbar.js",
           "web-toolbar": "./toolbar.js"