Report loading progress "automatically" when using the PDFDataTransportStream class, and remove the PDFDataRangeTransport.prototype.onDataProgress method

This is consistent with the other `BasePDFStream` implementations, and simplifies the API surface of the `PDFDataRangeTransport` class (note the changes in the viewer).
Given that the `onDataProgress` method was changed to a no-op this won't affect third-party users, assuming there even are any since this code was written specifically for the Firefox PDF Viewer.

Author: Snuffleupagus

src/display/api.js

@@ -639,8 +639,6 @@ class PDFDataRangeTransport {
 
   #progressiveReadListeners = [];
 
-  #progressListeners = [];
-
   #rangeListeners = [];
 
   /**
@@ -659,6 +657,18 @@ class PDFDataRangeTransport {
     this.initialData = initialData;
     this.progressiveDone = progressiveDone;
     this.contentDispositionFilename = contentDispositionFilename;
+
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
+      Object.defineProperty(this, "onDataProgress", {
+        value: () => {
+          deprecated(
+            "`PDFDataRangeTransport.prototype.onDataProgress` - method was " +
+              "removed, since loading progress is now reported automatically " +
+              "through the `PDFDataTransportStream` class (and related code)."
+          );
+        },
+      });
+    }
   }
 
   /**
@@ -668,13 +678,6 @@ class PDFDataRangeTransport {
     this.#rangeListeners.push(listener);
   }
 
-  /**
-   * @param {function} listener
-   */
-  addProgressListener(listener) {
-    this.#progressListeners.push(listener);
-  }
-
   /**
    * @param {function} listener
    */
@@ -699,18 +702,6 @@ class PDFDataRangeTransport {
     }
   }
 
-  /**
-   * @param {number} loaded
-   * @param {number|undefined} total
-   */
-  onDataProgress(loaded, total) {
-    this.#capability.promise.then(() => {
-      for (const listener of this.#progressListeners) {
-        listener(loaded, total);
-      }
-    });
-  }
-
   /**
    * @param {Uint8Array|null} chunk
    */

src/display/fetch_stream.js

@@ -135,10 +135,7 @@ class PDFFetchStreamReader extends BasePDFStreamReader {
       return { value, done };
     }
     this._loaded += value.byteLength;
-    this.onProgress?.({
-      loaded: this._loaded,
-      total: this._contentLength,
-    });
+    this._callOnProgress();
 
     return { value: getArrayBuffer(value), done: false };
   }

src/display/node_stream.js

@@ -129,11 +129,8 @@ class PDFNodeStreamReader extends BasePDFStreamReader {
     if (done) {
       return { value, done };
     }
-    this._loaded += value.length;
-    this.onProgress?.({
-      loaded: this._loaded,
-      total: this._contentLength,
-    });
+    this._loaded += value.byteLength;
+    this._callOnProgress();
 
     return { value: getArrayBuffer(value), done: false };
   }

src/display/transport_stream.js

@@ -53,12 +53,6 @@ class PDFDataTransportStream extends BasePDFStream {
       this.#onReceiveData(begin, chunk);
     });
 
-    pdfDataRangeTransport.addProgressListener((loaded, total) => {
-      if (total !== undefined) {
-        this._fullReader?.onProgress?.({ loaded, total });
-      }
-    });
-
     pdfDataRangeTransport.addProgressiveReadListener(chunk => {
       this.#onReceiveData(/* begin = */ undefined, chunk);
     });
@@ -144,6 +138,16 @@ class PDFDataTransportStreamReader extends BasePDFStreamReader {
       this._filename = contentDispositionFilename;
     }
     this._headersCapability.resolve();
+
+    // Report loading progress when there is `initialData`, and `_enqueue` has
+    // not been invoked, but with a small delay to give an `onProgress` callback
+    // a chance to be registered first.
+    const loaded = this._loaded;
+    Promise.resolve().then(() => {
+      if (loaded > 0 && this._loaded === loaded) {
+        this._callOnProgress();
+      }
+    });
   }
 
   _enqueue(chunk) {
@@ -157,6 +161,7 @@ class PDFDataTransportStreamReader extends BasePDFStreamReader {
       this._queuedChunks.push(chunk);
     }
     this._loaded += chunk.byteLength;
+    this._callOnProgress();
   }
 
   async read() {

src/shared/base_pdf_stream.js

@@ -128,6 +128,10 @@ class BasePDFStreamReader {
     this._stream = stream;
   }
 
+  _callOnProgress() {
+    this.onProgress?.({ loaded: this._loaded, total: this._contentLength });
+  }
+
   /**
    * Gets a promise that is resolved when the headers and other metadata of
    * the PDF data stream are available.

test/unit/api_spec.js

@@ -5180,6 +5180,7 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
     it("should fetch document info and page using ranges", async function () {
       const initialDataLength = 4000;
       const subArrays = [];
+      let initialProgress = null;
       let fetches = 0;
 
       const data = await dataPromise;
@@ -5193,19 +5194,29 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
           const chunk = new Uint8Array(data.subarray(begin, end));
           subArrays.push(chunk);
 
-          transport.onDataProgress(initialDataLength);
           transport.onDataRange(begin, chunk);
         });
       };
 
       const loadingTask = getDocument({ range: transport });
+      loadingTask.onProgress = evt => {
+        initialProgress = evt;
+        loadingTask.onProgress = null;
+      };
+
       const pdfDocument = await loadingTask.promise;
       expect(pdfDocument.numPages).toEqual(14);
 
       const pdfPage = await pdfDocument.getPage(10);
       expect(pdfPage.rotate).toEqual(0);
       expect(fetches).toBeGreaterThan(2);
 
+      expect(initialProgress).toEqual({
+        loaded: initialDataLength,
+        total: data.length,
+        percent: 0,
+      });
+
       // Check that the TypedArrays were transferred.
       for (const array of subArrays) {
         expect(array.length).toEqual(0);
@@ -5217,6 +5228,7 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
     it("should fetch document info and page using range and streaming", async function () {
       const initialDataLength = 4000;
       const subArrays = [];
+      let initialProgress = null;
       let fetches = 0;
 
       const data = await dataPromise;
@@ -5242,13 +5254,24 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
       };
 
       const loadingTask = getDocument({ range: transport });
+      loadingTask.onProgress = evt => {
+        initialProgress = evt;
+        loadingTask.onProgress = null;
+      };
+
       const pdfDocument = await loadingTask.promise;
       expect(pdfDocument.numPages).toEqual(14);
 
       const pdfPage = await pdfDocument.getPage(10);
       expect(pdfPage.rotate).toEqual(0);
       expect(fetches).toEqual(1);
 
+      expect(initialProgress).toEqual({
+        loaded: initialDataLength,
+        total: data.length,
+        percent: 0,
+      });
+
       await new Promise(resolve => {
         waitSome(resolve);
       });
@@ -5266,6 +5289,7 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
         "using complete initialData",
       async function () {
         const subArrays = [];
+        let initialProgress = null;
         let fetches = 0;
 
         const data = await dataPromise;
@@ -5285,13 +5309,24 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
           disableRange: true,
           range: transport,
         });
+        loadingTask.onProgress = evt => {
+          initialProgress = evt;
+          loadingTask.onProgress = null;
+        };
+
         const pdfDocument = await loadingTask.promise;
         expect(pdfDocument.numPages).toEqual(14);
 
         const pdfPage = await pdfDocument.getPage(10);
         expect(pdfPage.rotate).toEqual(0);
         expect(fetches).toEqual(0);
 
+        expect(initialProgress).toEqual({
+          loaded: data.length,
+          total: data.length,
+          percent: 100,
+        });
+
         // Check that the TypedArrays were transferred.
         for (const array of subArrays) {
           expect(array.length).toEqual(0);

web/firefoxcom.js

@@ -613,15 +613,8 @@ class ExternalServices extends BaseExternalServices {
         case "range":
           pdfDataRangeTransport.onDataRange(args.begin, args.chunk);
           break;
-        case "rangeProgress":
-          pdfDataRangeTransport.onDataProgress(args.loaded);
-          break;
         case "progressiveRead":
           pdfDataRangeTransport.onDataProgressiveRead(args.chunk);
-
-          // Don't forget to report loading progress as well, since otherwise
-          // the loadingBar won't update when `disableRange=true` is set.
-          pdfDataRangeTransport.onDataProgress(args.loaded, args.total);
           break;
         case "progressiveDone":
           pdfDataRangeTransport?.onDataProgressiveDone();