Merge pull request #20615 from Snuffleupagus/transport-onProgress

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

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/display_utils.js

@@ -799,7 +799,7 @@ class CSSConstants {
 }
 
 function applyOpacity(r, g, b, opacity) {
-  opacity = Math.min(Math.max(opacity ?? 1, 0), 1);
+  opacity = MathClamp(opacity ?? 1, 0, 1);
   const white = 255 * (1 - opacity);
   r = Math.round(r * opacity + white);
   g = Math.round(g * opacity + white);

src/display/fetch_stream.js

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

src/display/node_stream.js

@@ -119,11 +119,8 @@ class PDFNodeStreamReader extends BasePDFStreamReader {
     if (done) {
       return { value, done };
     }
-    this._loaded += value.length;
+    this._loaded += value.byteLength;
-    this.onProgress?.({
+    this._callOnProgress();
-      loaded: this._loaded,
-      total: this._contentLength,
-    });
 
     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.

src/shared/util.js

@@ -1235,9 +1235,12 @@ function MathClamp(v, min, max) {
   return Math.min(Math.max(v, min), max);
 }
 
-// TODO: Remove this once the `javascript.options.experimental.math_sumprecise`
+// TODO: Remove this once `Math.sumPrecise` is generally available.
-//       preference is removed from Firefox.
+if (
-if (typeof Math.sumPrecise !== "function") {
+  (typeof PDFJSDev === "undefined" ||
+    PDFJSDev.test("SKIP_BABEL && !MOZCENTRAL")) &&
+  typeof Math.sumPrecise !== "function"
+) {
   // Note that this isn't a "proper" polyfill, but since we're only using it to
   // replace `Array.prototype.reduce()` invocations it should be fine.
   Math.sumPrecise = function (numbers) {

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,12 +5194,16 @@ 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);
 
@@ -5206,6 +5211,12 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
       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,6 +5254,11 @@ 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);
 
@@ -5249,6 +5266,12 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
       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,6 +5309,11 @@ 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);
 
@@ -5292,6 +5321,12 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
         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

@@ -570,15 +570,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();