Merge pull request #20627 from Snuffleupagus/ChunkedStream-onReceiveData-rm-copy

Improve progress reporting in `ChunkedStreamManager`, and prevent unnecessary data copy in `ChunkedStream.prototype.onReceiveData`

src/core/chunked_stream.js

@@ -14,7 +14,7 @@
  */
 
 import { arrayBuffersToBytes, MissingDataException } from "./core_utils.js";
-import { assert } from "../shared/util.js";
+import { assert, MathClamp } from "../shared/util.js";
 import { Stream } from "./stream.js";
 
 class ChunkedStream extends Stream {
@@ -70,6 +70,12 @@ class ChunkedStream extends Stream {
       throw new Error(`Bad end offset: ${end}`);
     }
 
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
+      assert(
+        chunk instanceof ArrayBuffer,
+        "onReceiveData - expected an ArrayBuffer."
+      );
+    }
     this.bytes.set(new Uint8Array(chunk), begin);
     const beginChunk = Math.floor(begin / chunkSize);
     const endChunk = Math.floor((end - 1) / chunkSize) + 1;
@@ -85,6 +91,12 @@ class ChunkedStream extends Stream {
     let position = this.progressiveDataLength;
     const beginChunk = Math.floor(position / this.chunkSize);
 
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
+      assert(
+        data instanceof ArrayBuffer,
+        "onReceiveProgressiveData - expected an ArrayBuffer."
+      );
+    }
     this.bytes.set(new Uint8Array(data), position);
     position += data.byteLength;
     this.progressiveDataLength = position;
@@ -310,7 +322,7 @@ class ChunkedStreamManager {
       if (this.aborted) {
         return; // Ignoring any data after abort.
       }
-      this.onReceiveData({ chunk: data, begin });
+      this.onReceiveData({ chunk: data.buffer, begin });
     });
   }
 
@@ -511,7 +523,11 @@ class ChunkedStreamManager {
     }
 
     this.msgHandler.send("DocProgress", {
-      loaded: stream.numChunksLoaded * chunkSize,
+      loaded: MathClamp(
+        stream.numChunksLoaded * chunkSize,
+        stream.progressiveDataLength,
+        length
+      ),
       total: length,
     });
   }

src/core/evaluator.js

@@ -2538,7 +2538,7 @@ class PartialEvaluator {
 
     const preprocessor = new EvaluatorPreprocessor(stream, xref, stateManager);
 
-    let textState;
+    let textState, currentTextState;
 
     function pushWhitespace({
       width = 0,
@@ -2800,7 +2800,9 @@ class PartialEvaluator {
 
         // When the total height of the current chunk is negative
         // then we're writing from bottom to top.
-        const textOrientation = Math.sign(textContentItem.height);
+        const textOrientation = Math.sign(
+          textContentItem.height || textContentItem.totalHeight
+        );
         if (advanceY < textOrientation * textContentItem.negativeSpaceMax) {
           if (
             Math.abs(advanceX) >
@@ -2864,7 +2866,9 @@ class PartialEvaluator {
 
       // When the total width of the current chunk is negative
       // then we're writing from right to left.
-      const textOrientation = Math.sign(textContentItem.width);
+      const textOrientation = Math.sign(
+        textContentItem.width || textContentItem.totalWidth
+      );
       if (advanceX < textOrientation * textContentItem.negativeSpaceMax) {
         if (
           Math.abs(advanceY) >
@@ -2922,6 +2926,15 @@ class PartialEvaluator {
     }
 
     function buildTextContentItem({ chars, extraSpacing }) {
+      if (
+        currentTextState !== textState &&
+        (currentTextState.fontName !== textState.fontName ||
+          currentTextState.fontSize !== textState.fontSize)
+      ) {
+        flushTextContentItem();
+        currentTextState = textState.clone();
+      }
+
       const font = textState.font;
       if (!chars) {
         // Just move according to the space we have.
@@ -3177,8 +3190,8 @@ class PartialEvaluator {
           break;
         }
 
-        const previousState = textState;
         textState = stateManager.state;
+        currentTextState ||= textState.clone();
         const fn = operation.fn;
         args = operation.args;
 
@@ -3195,7 +3208,6 @@ class PartialEvaluator {
               break;
             }
 
-            flushTextContentItem();
             textState.fontName = fontNameArg;
             textState.fontSize = fontSizeArg;
             next(handleSetFont(fontNameArg, null));
@@ -3552,14 +3564,10 @@ class PartialEvaluator {
             }
             break;
           case OPS.restore:
-            if (
+            stateManager.restore();
-              previousState &&
+            break;
-              (previousState.font !== textState.font ||
+          case OPS.save:
-                previousState.fontSize !== textState.fontSize ||
+            stateManager.save();
-                previousState.fontName !== textState.fontName)
-            ) {
-              flushTextContentItem();
-            }
             break;
         } // switch
         if (textContent.items.length >= (sink?.desiredSize ?? 1)) {
@@ -5083,7 +5091,7 @@ class TextState {
   }
 
   clone() {
-    const clone = Object.create(this);
+    const clone = Object.assign(Object.create(this), this);
     clone.textMatrix = this.textMatrix.slice();
     clone.textLineMatrix = this.textLineMatrix.slice();
     clone.fontMatrix = this.fontMatrix.slice();

test/pdfs/.gitignore

@@ -871,3 +871,4 @@
 !page_with_number.pdf
 !page_with_number_and_link.pdf
 !Brotli-Prototype-FileA.pdf
+!bug2013793.pdf

test/unit/api_spec.js

@@ -4069,6 +4069,29 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
       await loadingTask.destroy();
     });
 
+    it("gets text content with some fake font changes (bug 2013793)", async function () {
+      const loadingTask = getDocument(buildGetDocumentParams("bug2013793.pdf"));
+      const pdfDoc = await loadingTask.promise;
+      const pdfPage = await pdfDoc.getPage(1);
+      const { items } = await pdfPage.getTextContent({
+        disableNormalization: true,
+      });
+      const text = mergeText(items);
+      expect(text)
+        .toEqual(`This is a great deal of nothing. The purpose is to help in identifying a bug when the PDF
+is read by Firefox. I want to know whether any of the two words in this paragraph run
+together. If they do, I will file a bug report. The problem seems to occur somewhere
+between the 240th and 260th character in the paragraph. I should have written that much
+by now. So, here’s to squashing bugs.
+This is a great deal of nothing. The purpose is to help in identifying a bug when the
+PDF is read by Firefox. I want to know whether any of the two words in this
+paragraph run together. If they do, I will file a bug report. The problem seems to
+occur somewhere between the 240th and 260th character in the paragraph. I should
+have written that much by now. So, here’s to squashing bugs.`);
+
+      await loadingTask.destroy();
+    });
+
     it("gets empty structure tree", async function () {
       const tree = await page.getStructTree();