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

@@ -25,7 +25,7 @@ function getUrlProp(val) {
     return null; // The 'url' is unused with `PDFDataRangeTransport`.
   }
   if (val instanceof URL) {
-    return val.href;
+    return val;
   }
   if (typeof val === "string") {
     if (
@@ -33,13 +33,18 @@ function getUrlProp(val) {
       PDFJSDev.test("GENERIC") &&
       isNodeJS
     ) {
-      return val; // Use the url as-is in Node.js environments.
+      if (/^[a-z][a-z0-9\-+.]+:/i.test(val)) {
+        return new URL(val);
+      }
+      // eslint-disable-next-line no-undef
+      const url = process.getBuiltinModule("url");
+      return new URL(url.pathToFileURL(val));
     }
 
     // The full path is required in the 'url' field.
     const url = URL.parse(val, window.location);
     if (url) {
-      return url.href;
+      return url;
     }
   }
   throw new Error(

src/display/display_utils.js

@@ -464,7 +464,7 @@ function isValidFetchUrl(url, baseUrl) {
   }
   const res = baseUrl ? URL.parse(url, baseUrl) : URL.parse(url);
   // The Fetch API only supports the http/https protocols, and not file/ftp.
-  return res?.protocol === "http:" || res?.protocol === "https:";
+  return /https?:/.test(res?.protocol ?? "");
 }
 
 /**
@@ -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

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { AbortException, warn } from "../shared/util.js";
+import { AbortException, assert, warn } from "../shared/util.js";
 import {
   BasePDFStream,
   BasePDFStreamRangeReader,
@@ -67,8 +67,13 @@ class PDFFetchStream extends BasePDFStream {
 
   constructor(source) {
     super(source, PDFFetchStreamReader, PDFFetchStreamRangeReader);
-    this.isHttp = /^https?:/i.test(source.url);
-    this.headers = createHeaders(this.isHttp, source.httpHeaders);
+    const { httpHeaders, url } = source;
+
+    assert(
+      /https?:/.test(url.protocol),
+      "PDFFetchStream only supports http(s):// URLs."
+    );
+    this.headers = createHeaders(/* isHttp = */ true, httpHeaders);
   }
 }
 
@@ -106,7 +111,7 @@ class PDFFetchStreamReader extends BasePDFStreamReader {
         const { allowRangeRequests, suggestedLength } =
           validateRangeRequestCapabilities({
             responseHeaders,
-            isHttp: stream.isHttp,
+            isHttp: true,
             rangeChunkSize,
             disableRange,
           });
@@ -135,10 +140,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/network.js

@@ -48,9 +48,11 @@ class PDFNetworkStream extends BasePDFStream {
 
   constructor(source) {
     super(source, PDFNetworkStreamReader, PDFNetworkStreamRangeReader);
-    this.url = source.url;
-    this.isHttp = /^https?:/i.test(this.url);
-    this.headers = createHeaders(this.isHttp, source.httpHeaders);
+    const { httpHeaders, url } = source;
+
+    this.url = url;
+    this.isHttp = /https?:/.test(url.protocol);
+    this.headers = createHeaders(this.isHttp, httpHeaders);
   }
 
   /**

src/display/network_utils.js

@@ -101,9 +101,9 @@ function extractFilenameFromHeader(responseHeaders) {
 
 function createResponseError(status, url) {
   return new ResponseException(
-    `Unexpected server response (${status}) while retrieving PDF "${url}".`,
+    `Unexpected server response (${status}) while retrieving PDF "${url.href}".`,
     status,
-    /* missing = */ status === 404 || (status === 0 && url.startsWith("file:"))
+    /* missing = */ status === 404 || (status === 0 && url.protocol === "file:")
   );
 }
 

src/display/node_stream.js

@@ -28,16 +28,6 @@ if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
   );
 }
 
-const urlRegex = /^[a-z][a-z0-9\-+.]+:/i;
-
-function parseUrlOrPath(sourceUrl) {
-  if (urlRegex.test(sourceUrl)) {
-    return new URL(sourceUrl);
-  }
-  const url = process.getBuiltinModule("url");
-  return new URL(url.pathToFileURL(sourceUrl));
-}
-
 function getReadableStream(readStream) {
   const { Readable } = process.getBuiltinModule("stream");
 
@@ -68,9 +58,10 @@ function getArrayBuffer(val) {
 class PDFNodeStream extends BasePDFStream {
   constructor(source) {
     super(source, PDFNodeStreamReader, PDFNodeStreamRangeReader);
-    this.url = parseUrlOrPath(source.url);
+    const { url } = source;
+
     assert(
-      this.url.protocol === "file:",
+      url.protocol === "file:",
       "PDFNodeStream only supports file:// URLs."
     );
   }
@@ -81,14 +72,13 @@ class PDFNodeStreamReader extends BasePDFStreamReader {
 
   constructor(stream) {
     super(stream);
-    const { disableRange, disableStream, length, rangeChunkSize } =
+    const { disableRange, disableStream, length, rangeChunkSize, url } =
       stream._source;
 
     this._contentLength = length;
     this._isStreamingSupported = !disableStream;
     this._isRangeSupported = !disableRange;
 
-    const url = stream.url;
     const fs = process.getBuiltinModule("fs");
     fs.promises
       .lstat(url)
@@ -117,7 +107,7 @@ class PDFNodeStreamReader extends BasePDFStreamReader {
       })
       .catch(error => {
         if (error.code === "ENOENT") {
-          error = createResponseError(/* status = */ 0, url.href);
+          error = createResponseError(/* status = */ 0, url);
         }
         this._headersCapability.reject(error);
       });
@@ -129,11 +119,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 };
   }
@@ -150,8 +137,8 @@ class PDFNodeStreamRangeReader extends BasePDFStreamRangeReader {
 
   constructor(stream, begin, end) {
     super(stream, begin, end);
+    const { url } = stream._source;
 
-    const url = stream.url;
     const fs = process.getBuiltinModule("fs");
     try {
       const readStream = fs.createReadStream(url, {

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`
-//       preference is removed from Firefox.
-if (typeof Math.sumPrecise !== "function") {
+// TODO: Remove this once `Math.sumPrecise` is generally available.
+if (
+  (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);

test/unit/common_pdfstream_tests.js

@@ -24,7 +24,7 @@ async function testCrossOriginRedirects({
   redirectIfRange,
   testRangeReader,
 }) {
-  const basicApiUrl = TestPdfsServer.resolveURL("basicapi.pdf").href;
+  const basicApiUrl = TestPdfsServer.resolveURL("basicapi.pdf");
   const basicApiFileLength = 105779;
 
   const rangeSize = 32768;
@@ -83,7 +83,7 @@ function getCrossOriginUrlWithRedirects(testserverUrl, redirectIfRange) {
   if (redirectIfRange) {
     url.searchParams.set("redirectIfRange", "1");
   }
-  return url.href;
+  return url;
 }
 
 export { testCrossOriginRedirects };

test/unit/fetch_stream_spec.js

@@ -20,7 +20,7 @@ import { TestPdfsServer } from "./test_utils.js";
 
 describe("fetch_stream", function () {
   function getPdfUrl() {
-    return TestPdfsServer.resolveURL("tracemonkey.pdf").href;
+    return TestPdfsServer.resolveURL("tracemonkey.pdf");
   }
   const pdfLength = 1016315;
 

test/unit/network_spec.js

@@ -19,7 +19,7 @@ import { testCrossOriginRedirects } from "./common_pdfstream_tests.js";
 import { TestPdfsServer } from "./test_utils.js";
 
 describe("network", function () {
-  const pdf1 = new URL("../pdfs/tracemonkey.pdf", window.location).href;
+  const pdf1 = new URL("../pdfs/tracemonkey.pdf", window.location);
   const pdf1Length = 1016315;
 
   it("read without stream and range", async function () {
@@ -124,9 +124,12 @@ describe("network", function () {
     }
 
     async function readRanges(mode) {
+      const pdfUrl = new URL(pdf1);
+      pdfUrl.searchParams.set("test-network-break-ranges", mode);
+
       const rangeSize = 32768;
       const stream = new PDFNetworkStream({
-        url: `${pdf1}?test-network-break-ranges=${mode}`,
+        url: pdfUrl,
         length: pdf1Length,
         rangeChunkSize: rangeSize,
         disableStream: true,

test/unit/network_utils_spec.js

@@ -372,22 +372,22 @@ describe("network_utils", function () {
 
       expect(error instanceof ResponseException).toEqual(true);
       expect(error.message).toEqual(
-        `Unexpected server response (${status}) while retrieving PDF "${url}".`
+        `Unexpected server response (${status}) while retrieving PDF "${url.href}".`
       );
       expect(error.status).toEqual(status);
       expect(error.missing).toEqual(missing);
     }
 
     it("handles missing PDF file responses", function () {
-      testCreateResponseError("https://foo.com/bar.pdf", 404, true);
+      testCreateResponseError(new URL("https://foo.com/bar.pdf"), 404, true);
 
-      testCreateResponseError("file://foo.pdf", 0, true);
+      testCreateResponseError(new URL("file://foo.pdf"), 0, true);
     });
 
     it("handles unexpected responses", function () {
-      testCreateResponseError("https://foo.com/bar.pdf", 302, false);
+      testCreateResponseError(new URL("https://foo.com/bar.pdf"), 302, false);
 
-      testCreateResponseError("https://foo.com/bar.pdf", 0, false);
+      testCreateResponseError(new URL("https://foo.com/bar.pdf"), 0, false);
     });
   });
 });

test/unit/node_stream_spec.js

@@ -26,7 +26,7 @@ if (!isNodeJS) {
 describe("node_stream", function () {
   const url = process.getBuiltinModule("url");
   const cwdURL = url.pathToFileURL(process.cwd()) + "/";
-  const pdf = new URL("./test/pdfs/tracemonkey.pdf", cwdURL).href;
+  const pdf = new URL("./test/pdfs/tracemonkey.pdf", cwdURL);
   const pdfLength = 1016315;
 
   it("read filesystem pdf files", async function () {
@@ -119,7 +119,7 @@ describe("node_stream", function () {
   });
 
   it("read filesystem pdf files (smaller than two range requests)", async function () {
-    const smallPdf = new URL("./test/pdfs/empty.pdf", cwdURL).href;
+    const smallPdf = new URL("./test/pdfs/empty.pdf", cwdURL);
     const smallLength = 4920;
 
     const stream = new PDFNodeStream({

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();