From 0b65d4cacd24403b50f42f3a2052149aa339e6b6 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Mon, 3 Aug 2020 13:58:58 +0200
Subject: [PATCH] Re-factor the dispatching of "attachmentsloaded" events, when
 the PDF document contains no "regular" attachments

Since the attachment fetching/parsing is already asynchronous, possibly delaying the dispatching of an "attachmentsloaded" event should thus not be a problem in general.
Note that in some cases, i.e. PDF documents with no "regular" attachments and only FileAttachment annotations, we'll thus no longer dispatch an "attachmentsloaded" event when `attachmentsCount === 0` and instead wait for the FileAttachment parsing to finish. (The use of a timeout still guarantees that an "attachmentsloaded" event is *eventually* dispatched though.)

This patch *considerably* simplifies the "attachmentsloaded" event handler, in `PDFSidebar`, since the details are now abstracted away from the consumer.

Finally, add a check in `_appendAttachment` to ensure (indirectly) that the FileAttachment annotation is actually relevant for the active document.
---
 web/pdf_attachment_viewer.js | 34 +++++++++++++++++++++++++++++++---
 web/pdf_sidebar.js           | 30 ++++++++----------------------
 2 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/web/pdf_attachment_viewer.js b/web/pdf_attachment_viewer.js
index 2210dfde4..0ba8715ff 100644
--- a/web/pdf_attachment_viewer.js
+++ b/web/pdf_attachment_viewer.js
@@ -55,10 +55,14 @@ class PDFAttachmentViewer {
     this.container.textContent = "";
 
     if (!keepRenderedCapability) {
-      // NOTE: The *only* situation in which the `_renderedCapability` should
-      //       not be replaced is when appending file attachment annotations.
+      // The only situation in which the `_renderedCapability` should *not* be
+      // replaced is when appending FileAttachment annotations.
       this._renderedCapability = createPromiseCapability();
     }
+    if (this._pendingDispatchEvent) {
+      clearTimeout(this._pendingDispatchEvent);
+    }
+    this._pendingDispatchEvent = null;
   }
 
   /**
@@ -67,6 +71,25 @@ class PDFAttachmentViewer {
   _dispatchEvent(attachmentsCount) {
     this._renderedCapability.resolve();
 
+    if (this._pendingDispatchEvent) {
+      clearTimeout(this._pendingDispatchEvent);
+      this._pendingDispatchEvent = null;
+    }
+    if (attachmentsCount === 0) {
+      // Delay the event when no "regular" attachments exist, to allow time for
+      // parsing of any FileAttachment annotations that may be present on the
+      // *initially* rendered page; this reduces the likelihood of temporarily
+      // disabling the attachmentsView when the `PDFSidebar` handles the event.
+      this._pendingDispatchEvent = setTimeout(() => {
+        this.eventBus.dispatch("attachmentsloaded", {
+          source: this,
+          attachmentsCount: 0,
+        });
+        this._pendingDispatchEvent = null;
+      });
+      return;
+    }
+
     this.eventBus.dispatch("attachmentsloaded", {
       source: this,
       attachmentsCount,
@@ -175,7 +198,12 @@ class PDFAttachmentViewer {
    * @private
    */
   _appendAttachment({ id, filename, content }) {
-    this._renderedCapability.promise.then(() => {
+    const renderedPromise = this._renderedCapability.promise;
+
+    renderedPromise.then(() => {
+      if (renderedPromise !== this._renderedCapability.promise) {
+        return; // The FileAttachment annotation belongs to a previous document.
+      }
       let attachments = this.attachments;
 
       if (!attachments) {
diff --git a/web/pdf_sidebar.js b/web/pdf_sidebar.js
index 9122d5d3c..512fbc850 100644
--- a/web/pdf_sidebar.js
+++ b/web/pdf_sidebar.js
@@ -445,31 +445,17 @@ class PDFSidebar {
     });
 
     this.eventBus._on("attachmentsloaded", evt => {
-      if (evt.attachmentsCount) {
-        this.attachmentsButton.disabled = false;
+      const attachmentsCount = evt.attachmentsCount;
 
+      this.attachmentsButton.disabled = !attachmentsCount;
+
+      if (attachmentsCount) {
         this._showUINotification(SidebarView.ATTACHMENTS);
-        return;
+      } else if (this.active === SidebarView.ATTACHMENTS) {
+        // If the attachments view was opened during document load, switch away
+        // from it if it turns out that the document has no attachments.
+        this.switchView(SidebarView.THUMBS);
       }
-
-      // Attempt to avoid temporarily disabling, and switching away from, the
-      // attachment view for documents that do not contain proper attachments
-      // but *only* FileAttachment annotations. Hence we defer those operations
-      // slightly to allow time for parsing any FileAttachment annotations that
-      // may be present on the *initially* rendered page of the document.
-      Promise.resolve().then(() => {
-        if (this.attachmentsView.hasChildNodes()) {
-          // FileAttachment annotations were appended to the attachment view.
-          return;
-        }
-        this.attachmentsButton.disabled = true;
-
-        if (this.active === SidebarView.ATTACHMENTS) {
-          // If the attachment view was opened during document load, switch away
-          // from it if it turns out that the document has no attachments.
-          this.switchView(SidebarView.THUMBS);
-        }
-      });
     });
 
     // Update the thumbnailViewer, if visible, when exiting presentation mode.