From 674052d3fc6998371b8f152d09c152b73398cc6d Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Mon, 16 Oct 2023 16:20:46 +0200
Subject: [PATCH] Re-factor the blob-URL caching in
 `DownloadManager.openOrDownloadData`

Cache blob-URLs on the actual data, rather than DOM elements, to reduce potential duplicates (note the updated unit-test).
---
 src/display/annotation_layer.js | 7 +------
 test/unit/api_spec.js           | 5 +++++
 web/download_manager.js         | 8 ++++----
 web/firefoxcom.js               | 8 ++++----
 web/interfaces.js               | 4 ++--
 web/pdf_attachment_viewer.js    | 2 +-
 web/pdf_outline_viewer.js       | 1 -
 7 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js
index da2745da8..088d5014a 100644
--- a/src/display/annotation_layer.js
+++ b/src/display/annotation_layer.js
@@ -806,7 +806,6 @@ class LinkAnnotationElement extends AnnotationElement {
     link.href = this.linkService.getAnchorUrl("");
     link.onclick = () => {
       this.downloadManager?.openOrDownloadData(
-        this.container,
         attachment.content,
         attachment.filename,
         dest
@@ -2861,11 +2860,7 @@ class FileAttachmentAnnotationElement extends AnnotationElement {
    * Download the file attachment associated with this annotation.
    */
   #download() {
-    this.downloadManager?.openOrDownloadData(
-      this.container,
-      this.content,
-      this.filename
-    );
+    this.downloadManager?.openOrDownloadData(this.content, this.filename);
   }
 }
 
diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js
index 0396325c0..2db8a25dd 100644
--- a/test/unit/api_spec.js
+++ b/test/unit/api_spec.js
@@ -2883,6 +2883,11 @@ describe("api", function () {
 
       expect(attachmentDest).toEqual('[0,{"name":"Fit"}]');
 
+      // Check that the attachments, which are identical, aren't duplicated.
+      for (let i = 1, ii = annotations.length; i < ii; i++) {
+        expect(annotations[i].attachment).toBe(attachment);
+      }
+
       await loadingTask.destroy();
     });
 
diff --git a/web/download_manager.js b/web/download_manager.js
index 26d655bc1..48bf4fe80 100644
--- a/web/download_manager.js
+++ b/web/download_manager.js
@@ -67,7 +67,7 @@ class DownloadManager {
   /**
    * @returns {boolean} Indicating if the data was opened.
    */
-  openOrDownloadData(element, data, filename, dest = null) {
+  openOrDownloadData(data, filename, dest = null) {
     const isPdfData = isPdfFile(filename);
     const contentType = isPdfData ? "application/pdf" : "";
 
@@ -75,10 +75,10 @@ class DownloadManager {
       (typeof PDFJSDev === "undefined" || !PDFJSDev.test("COMPONENTS")) &&
       isPdfData
     ) {
-      let blobUrl = this.#openBlobUrls.get(element);
+      let blobUrl = this.#openBlobUrls.get(data);
       if (!blobUrl) {
         blobUrl = URL.createObjectURL(new Blob([data], { type: contentType }));
-        this.#openBlobUrls.set(element, blobUrl);
+        this.#openBlobUrls.set(data, blobUrl);
       }
       let viewerUrl;
       if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
@@ -105,7 +105,7 @@ class DownloadManager {
         // Release the `blobUrl`, since opening it failed, and fallback to
         // downloading the PDF file.
         URL.revokeObjectURL(blobUrl);
-        this.#openBlobUrls.delete(element);
+        this.#openBlobUrls.delete(data);
       }
     }
 
diff --git a/web/firefoxcom.js b/web/firefoxcom.js
index 438fbf9fd..baaf61066 100644
--- a/web/firefoxcom.js
+++ b/web/firefoxcom.js
@@ -132,15 +132,15 @@ class DownloadManager {
   /**
    * @returns {boolean} Indicating if the data was opened.
    */
-  openOrDownloadData(element, data, filename, dest = null) {
+  openOrDownloadData(data, filename, dest = null) {
     const isPdfData = isPdfFile(filename);
     const contentType = isPdfData ? "application/pdf" : "";
 
     if (isPdfData) {
-      let blobUrl = this.#openBlobUrls.get(element);
+      let blobUrl = this.#openBlobUrls.get(data);
       if (!blobUrl) {
         blobUrl = URL.createObjectURL(new Blob([data], { type: contentType }));
-        this.#openBlobUrls.set(element, blobUrl);
+        this.#openBlobUrls.set(data, blobUrl);
       }
       // Let Firefox's content handler catch the URL and display the PDF.
       let viewerUrl = blobUrl + "?filename=" + encodeURIComponent(filename);
@@ -156,7 +156,7 @@ class DownloadManager {
         // Release the `blobUrl`, since opening it failed, and fallback to
         // downloading the PDF file.
         URL.revokeObjectURL(blobUrl);
-        this.#openBlobUrls.delete(element);
+        this.#openBlobUrls.delete(data);
       }
     }
 
diff --git a/web/interfaces.js b/web/interfaces.js
index abf039040..3f7f277de 100644
--- a/web/interfaces.js
+++ b/web/interfaces.js
@@ -158,12 +158,12 @@ class IDownloadManager {
   downloadData(data, filename, contentType) {}
 
   /**
-   * @param {HTMLElement} element
    * @param {Uint8Array} data
    * @param {string} filename
+   * @param {string | null} [dest]
    * @returns {boolean} Indicating if the data was opened.
    */
-  openOrDownloadData(element, data, filename) {}
+  openOrDownloadData(data, filename, dest = null) {}
 
   /**
    * @param {Blob} blob
diff --git a/web/pdf_attachment_viewer.js b/web/pdf_attachment_viewer.js
index 49b27dd47..d99cfa985 100644
--- a/web/pdf_attachment_viewer.js
+++ b/web/pdf_attachment_viewer.js
@@ -91,7 +91,7 @@ class PDFAttachmentViewer extends BaseTreeViewer {
    */
   _bindLink(element, { content, filename }) {
     element.onclick = () => {
-      this.downloadManager.openOrDownloadData(element, content, filename);
+      this.downloadManager.openOrDownloadData(content, filename);
       return false;
     };
   }
diff --git a/web/pdf_outline_viewer.js b/web/pdf_outline_viewer.js
index d77222c8f..70c629a71 100644
--- a/web/pdf_outline_viewer.js
+++ b/web/pdf_outline_viewer.js
@@ -133,7 +133,6 @@ class PDFOutlineViewer extends BaseTreeViewer {
       element.href = linkService.getAnchorUrl("");
       element.onclick = () => {
         this.downloadManager.openOrDownloadData(
-          element,
           attachment.content,
           attachment.filename
         );