From 327f2eb588e66fb7fb63e75eafa28c24c9c0bb9a Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Tue, 16 Oct 2018 13:24:02 +0200
Subject: [PATCH] Ensure that `onProgress` is always called when the entire PDF
 file has been loaded, regardless of how it was fetched (issue 10160)

*Please note:* I'm totally fine with this patch being rejected, and the issue closed as WONTFIX; however these changes should address the issue if that's desired.

From a conceptual point of view, reporting loading progress doesn't really make a lot of sense for PDF files opened by passing raw binary data directly to `getDocument` (since obviously *all* data was loaded).
This is compared to PDF files loaded via e.g. `XMLHttpRequest` or the Fetch API, where the entire PDF file isn't available from the start and knowing the loading progress makes total sense.

However I can certainly see why the current API could be considered inconsistent, which isn't great, since a registered `onProgress` callback will never be called for certain `getDocument` calls.
The simplest solution to this inconsistency thus seem to be to ensure that `onProgress` is always called when handling the `DataLoaded` message, since that will *always* be dispatched[1] from the worker-thread.

---
[1] Note that this isn't guaranteed to happen, since setting `disableAutoFetch = true` often prevents the *entire* file from ever loading. However, this isn't relevant for the issue at hand, and is a well-known consequence of using `disableAutoFetch = true`; note how the default viewer even has a specialized code-path for hiding the loadingBar.
---
 src/display/api.js    | 17 +++++++++++------
 test/unit/api_spec.js | 17 ++++++++++++++---
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/display/api.js b/src/display/api.js
index af6831942..deaf7d4af 100644
--- a/src/display/api.js
+++ b/src/display/api.js
@@ -1763,12 +1763,9 @@ class WorkerTransport {
       fullReader.headersReady.then(() => {
         // If stream or range are disabled, it's our only way to report
         // loading progress.
-        if (!fullReader.isStreamingSupported ||
-            !fullReader.isRangeSupported) {
-          if (this._lastProgress) {
-            if (loadingTask.onProgress) {
-              loadingTask.onProgress(this._lastProgress);
-            }
+        if (!fullReader.isStreamingSupported || !fullReader.isRangeSupported) {
+          if (this._lastProgress && loadingTask.onProgress) {
+            loadingTask.onProgress(this._lastProgress);
           }
           fullReader.onProgress = (evt) => {
             if (loadingTask.onProgress) {
@@ -1866,6 +1863,14 @@ class WorkerTransport {
     }, this);
 
     messageHandler.on('DataLoaded', function(data) {
+      // For consistency: Ensure that progress is always reported when the
+      // entire PDF file has been loaded, regardless of how it was fetched.
+      if (loadingTask.onProgress) {
+        loadingTask.onProgress({
+          loaded: data.length,
+          total: data.length,
+        });
+      }
       this.downloadInfoCapability.resolve(data);
     }, this);
 
diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js
index ebdc3d6d5..42a287263 100644
--- a/test/unit/api_spec.js
+++ b/test/unit/api_spec.js
@@ -142,9 +142,20 @@ describe('api', function() {
       // Sanity check to make sure that we fetched the entire PDF file.
       expect(typedArrayPdf.length).toEqual(basicApiFileLength);
 
-      var loadingTask = getDocument(typedArrayPdf);
-      loadingTask.promise.then(function(data) {
-        expect(data instanceof PDFDocumentProxy).toEqual(true);
+      const loadingTask = getDocument(typedArrayPdf);
+
+      const progressReportedCapability = createPromiseCapability();
+      loadingTask.onProgress = function(data) {
+        progressReportedCapability.resolve(data);
+      };
+
+      Promise.all([
+        loadingTask.promise,
+        progressReportedCapability.promise,
+      ]).then(function(data) {
+        expect(data[0] instanceof PDFDocumentProxy).toEqual(true);
+        expect(data[1].loaded / data[1].total).toEqual(1);
+
         loadingTask.destroy().then(done);
       }).catch(done.fail);
     });