From 923bd52cdb94e0a0afa5a3d8261f64ad0e8e7e84 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Fri, 25 Mar 2022 14:10:22 +0100
Subject: [PATCH] Re-factor the `OverlayManager` class to use a `WeakMap`
 internally

This way we're able to store the `<dialog>` elements directly, which removes the need to use manually specified name-strings thus simplifying both the `OverlayManager` itself and its calling code.
---
 web/chromecom.js               | 16 ++++++------
 web/overlay_manager.js         | 45 +++++++++++++++++-----------------
 web/password_prompt.js         | 14 +++--------
 web/pdf_document_properties.js | 17 ++++---------
 web/pdf_print_service.js       | 20 +++++++--------
 web/viewer.js                  |  2 --
 6 files changed, 48 insertions(+), 66 deletions(-)

diff --git a/web/chromecom.js b/web/chromecom.js
index 61c2cb636..64b5ee042 100644
--- a/web/chromecom.js
+++ b/web/chromecom.js
@@ -162,13 +162,11 @@ function requestAccessToLocalFile(fileUrl, overlayManager, callback) {
       reloadIfRuntimeIsUnavailable();
     });
   }
-  if (!chromeFileAccessOverlayPromise) {
-    chromeFileAccessOverlayPromise = overlayManager.register(
-      "chromeFileAccessDialog",
-      dialog,
-      /* canForceClose = */ true
-    );
-  }
+  chromeFileAccessOverlayPromise ||= overlayManager.register(
+    dialog,
+    /* canForceClose = */ true
+  );
+
   chromeFileAccessOverlayPromise.then(function () {
     const iconPath = chrome.runtime.getManifest().icons[48];
     document.getElementById("chrome-pdfjs-logo-bg").style.backgroundImage =
@@ -227,11 +225,11 @@ function requestAccessToLocalFile(fileUrl, overlayManager, callback) {
           originalUrl = "file:///fakepath/to/" + encodeURIComponent(file.name);
         }
         callback(URL.createObjectURL(file), file.size, originalUrl);
-        overlayManager.close("chromeFileAccessOverlay");
+        overlayManager.close(dialog);
       }
     };
 
-    overlayManager.open("chromeFileAccessDialog");
+    overlayManager.open(dialog);
   });
 }
 
diff --git a/web/overlay_manager.js b/web/overlay_manager.js
index d7d8ee2c8..233c9f594 100644
--- a/web/overlay_manager.js
+++ b/web/overlay_manager.js
@@ -14,7 +14,7 @@
  */
 
 class OverlayManager {
-  #overlays = Object.create(null);
+  #overlays = new WeakMap();
 
   #active = null;
 
@@ -23,20 +23,19 @@ class OverlayManager {
   }
 
   /**
-   * @param {string} name - The name of the overlay that is registered.
    * @param {HTMLDialogElement} dialog - The overlay's DOM element.
    * @param {boolean} [canForceClose] - Indicates if opening the overlay closes
    *                  an active overlay. The default is `false`.
    * @returns {Promise} A promise that is resolved when the overlay has been
    *                    registered.
    */
-  async register(name, dialog, canForceClose = false) {
-    if (!name || !dialog) {
+  async register(dialog, canForceClose = false) {
+    if (typeof dialog !== "object") {
       throw new Error("Not enough parameters.");
-    } else if (this.#overlays[name]) {
+    } else if (this.#overlays.has(dialog)) {
       throw new Error("The overlay is already registered.");
     }
-    this.#overlays[name] = { dialog, canForceClose };
+    this.#overlays.set(dialog, { canForceClose });
 
     dialog.addEventListener("cancel", evt => {
       this.#active = null;
@@ -44,54 +43,54 @@ class OverlayManager {
   }
 
   /**
-   * @param {string} name - The name of the overlay that is unregistered.
+   * @param {HTMLDialogElement} dialog - The overlay's DOM element.
    * @returns {Promise} A promise that is resolved when the overlay has been
    *                    unregistered.
    */
-  async unregister(name) {
-    if (!this.#overlays[name]) {
+  async unregister(dialog) {
+    if (!this.#overlays.has(dialog)) {
       throw new Error("The overlay does not exist.");
-    } else if (this.#active === name) {
+    } else if (this.#active === dialog) {
       throw new Error("The overlay cannot be removed while it is active.");
     }
-    delete this.#overlays[name];
+    this.#overlays.delete(dialog);
   }
 
   /**
-   * @param {string} name - The name of the overlay that should be opened.
+   * @param {HTMLDialogElement} dialog - The overlay's DOM element.
    * @returns {Promise} A promise that is resolved when the overlay has been
    *                    opened.
    */
-  async open(name) {
-    if (!this.#overlays[name]) {
+  async open(dialog) {
+    if (!this.#overlays.has(dialog)) {
       throw new Error("The overlay does not exist.");
     } else if (this.#active) {
-      if (this.#active === name) {
+      if (this.#active === dialog) {
         throw new Error("The overlay is already active.");
-      } else if (this.#overlays[name].canForceClose) {
+      } else if (this.#overlays.get(dialog).canForceClose) {
         await this.close();
       } else {
         throw new Error("Another overlay is currently active.");
       }
     }
-    this.#active = name;
-    this.#overlays[this.#active].dialog.showModal();
+    this.#active = dialog;
+    dialog.showModal();
   }
 
   /**
-   * @param {string} name - The name of the overlay that should be closed.
+   * @param {HTMLDialogElement} dialog - The overlay's DOM element.
    * @returns {Promise} A promise that is resolved when the overlay has been
    *                    closed.
    */
-  async close(name = this.#active) {
-    if (!this.#overlays[name]) {
+  async close(dialog = this.#active) {
+    if (!this.#overlays.has(dialog)) {
       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) {
+    } else if (this.#active !== dialog) {
       throw new Error("Another overlay is currently active.");
     }
-    this.#overlays[this.#active].dialog.close();
+    dialog.close();
     this.#active = null;
   }
 }
diff --git a/web/password_prompt.js b/web/password_prompt.js
index 1a8728553..2207b1a9d 100644
--- a/web/password_prompt.js
+++ b/web/password_prompt.js
@@ -17,7 +17,6 @@ import { PasswordResponses } from "pdfjs-lib";
 
 /**
  * @typedef {Object} PasswordPromptOptions
- * @property {string} dialogName - Name/identifier for the dialog.
  * @property {HTMLDialogElement} dialog - The overlay's DOM element.
  * @property {HTMLParagraphElement} label - Label containing instructions for
  *                                          entering the password.
@@ -41,7 +40,6 @@ class PasswordPrompt {
    *   an <iframe> or an <object>. The default value is `false`.
    */
   constructor(options, overlayManager, l10n, isViewerEmbedded = false) {
-    this.dialogName = options.dialogName;
     this.dialog = options.dialog;
     this.label = options.label;
     this.input = options.input;
@@ -60,17 +58,13 @@ class PasswordPrompt {
       }
     });
 
-    this.overlayManager.register(
-      this.dialogName,
-      this.dialog,
-      /* canForceClose = */ true
-    );
+    this.overlayManager.register(this.dialog, /* canForceClose = */ true);
 
     this.dialog.addEventListener("close", this.#cancel.bind(this));
   }
 
   async open() {
-    await this.overlayManager.open(this.dialogName);
+    await this.overlayManager.open(this.dialog);
 
     const passwordIncorrect =
       this.#reason === PasswordResponses.INCORRECT_PASSWORD;
@@ -84,8 +78,8 @@ class PasswordPrompt {
   }
 
   async close() {
-    if (this.overlayManager.active === this.dialogName) {
-      this.overlayManager.close(this.dialogName);
+    if (this.overlayManager.active === this.dialog) {
+      this.overlayManager.close(this.dialog);
     }
   }
 
diff --git a/web/pdf_document_properties.js b/web/pdf_document_properties.js
index 1b4f6804e..727013b58 100644
--- a/web/pdf_document_properties.js
+++ b/web/pdf_document_properties.js
@@ -45,7 +45,6 @@ function getPageName(size, isPortrait, pageNames) {
 
 /**
  * @typedef {Object} PDFDocumentPropertiesOptions
- * @property {string} dialogName - Name/identifier for the dialog.
  * @property {HTMLDialogElement} dialog - The overlay's DOM element.
  * @property {Object} fields - Names and elements of the overlay's fields.
  * @property {HTMLButtonElement} closeButton - Button for closing the overlay.
@@ -60,13 +59,7 @@ class PDFDocumentProperties {
    * @param {EventBus} eventBus - The application event bus.
    * @param {IL10n} l10n - Localization service.
    */
-  constructor(
-    { dialogName, dialog, fields, closeButton },
-    overlayManager,
-    eventBus,
-    l10n
-  ) {
-    this.dialogName = dialogName;
+  constructor({ dialog, fields, closeButton }, overlayManager, eventBus, l10n) {
     this.dialog = dialog;
     this.fields = fields;
     this.overlayManager = overlayManager;
@@ -76,7 +69,7 @@ class PDFDocumentProperties {
     // Bind the event listener for the Close button.
     closeButton.addEventListener("click", this.close.bind(this));
 
-    this.overlayManager.register(this.dialogName, this.dialog);
+    this.overlayManager.register(this.dialog);
 
     eventBus._on("pagechanging", evt => {
       this._currentPageNumber = evt.pageNumber;
@@ -96,7 +89,7 @@ class PDFDocumentProperties {
    */
   async open() {
     await Promise.all([
-      this.overlayManager.open(this.dialogName),
+      this.overlayManager.open(this.dialog),
       this._dataAvailableCapability.promise,
     ]);
     const currentPageNumber = this._currentPageNumber;
@@ -176,7 +169,7 @@ class PDFDocumentProperties {
    * Close the document properties overlay.
    */
   async close() {
-    this.overlayManager.close(this.dialogName);
+    this.overlayManager.close(this.dialog);
   }
 
   /**
@@ -224,7 +217,7 @@ class PDFDocumentProperties {
       }
       return;
     }
-    if (this.overlayManager.active !== this.dialogName) {
+    if (this.overlayManager.active !== this.dialog) {
       // Don't bother updating the dialog if has already been closed,
       // since it will be updated the next time `this.open` is called.
       return;
diff --git a/web/pdf_print_service.js b/web/pdf_print_service.js
index 050608ea4..f58cabd38 100644
--- a/web/pdf_print_service.js
+++ b/web/pdf_print_service.js
@@ -18,6 +18,7 @@ import { PDFPrintServiceFactory, PDFViewerApplication } from "./app.js";
 import { getXfaHtmlForPrinting } from "./print_utils.js";
 
 let activeService = null;
+let dialog = null;
 let overlayManager = null;
 
 // Renders the page to the canvas of the given print service, and returns
@@ -131,8 +132,8 @@ PDFPrintService.prototype = {
     this.scratchCanvas = null;
     activeService = null;
     ensureOverlay().then(function () {
-      if (overlayManager.active === "printServiceDialog") {
-        overlayManager.close("printServiceDialog");
+      if (overlayManager.active === dialog) {
+        overlayManager.close(dialog);
       }
     });
   },
@@ -229,7 +230,7 @@ window.print = function () {
   }
   ensureOverlay().then(function () {
     if (activeService) {
-      overlayManager.open("printServiceDialog");
+      overlayManager.open(dialog);
     }
   });
 
@@ -239,8 +240,8 @@ window.print = function () {
     if (!activeService) {
       console.error("Expected print service to be initialized.");
       ensureOverlay().then(function () {
-        if (overlayManager.active === "printServiceDialog") {
-          overlayManager.close("printServiceDialog");
+        if (overlayManager.active === dialog) {
+          overlayManager.close(dialog);
         }
       });
       return; // eslint-disable-line no-unsafe-finally
@@ -281,10 +282,10 @@ function abort() {
 }
 
 function renderProgress(index, total, l10n) {
-  const progressDialog = document.getElementById("printServiceDialog");
+  dialog ||= document.getElementById("printServiceDialog");
   const progress = Math.round((100 * index) / total);
-  const progressBar = progressDialog.querySelector("progress");
-  const progressPerc = progressDialog.querySelector(".relative-progress");
+  const progressBar = dialog.querySelector("progress");
+  const progressPerc = dialog.querySelector(".relative-progress");
   progressBar.value = progress;
   l10n.get("print_progress_percent", { progress }).then(msg => {
     progressPerc.textContent = msg;
@@ -336,10 +337,9 @@ function ensureOverlay() {
     if (!overlayManager) {
       throw new Error("The overlay manager has not yet been initialized.");
     }
-    const dialog = document.getElementById("printServiceDialog");
+    dialog ||= document.getElementById("printServiceDialog");
 
     overlayPromise = overlayManager.register(
-      "printServiceDialog",
       dialog,
       /* canForceClose = */ true
     );
diff --git a/web/viewer.js b/web/viewer.js
index b6b5b9b4c..6bc8b2ab7 100644
--- a/web/viewer.js
+++ b/web/viewer.js
@@ -161,7 +161,6 @@ function getViewerConfiguration() {
       findNextButton: document.getElementById("findNext"),
     },
     passwordOverlay: {
-      dialogName: "passwordDialog",
       dialog: document.getElementById("passwordDialog"),
       label: document.getElementById("passwordText"),
       input: document.getElementById("password"),
@@ -169,7 +168,6 @@ function getViewerConfiguration() {
       cancelButton: document.getElementById("passwordCancel"),
     },
     documentProperties: {
-      dialogName: "documentPropertiesDialog",
       dialog: document.getElementById("documentPropertiesDialog"),
       closeButton: document.getElementById("documentPropertiesClose"),
       fields: {