Merge pull request #19778 from Snuffleupagus/replace-getAll

[api-minor] Replace various `getAll` methods with iterators

eslint.config.mjs

@@ -127,6 +127,7 @@ export default [
       "perfectionist/sort-named-exports": "error",
       "unicorn/no-abusive-eslint-disable": "error",
       "unicorn/no-array-push-push": "error",
+      "unicorn/no-array-reduce": ["error", { allowSimpleOperations: true }],
       "unicorn/no-console-spaces": "error",
       "unicorn/no-instanceof-builtins": "error",
       "unicorn/no-invalid-remove-event-listener": "error",

src/core/annotation.js

@@ -25,7 +25,6 @@ import {
   BASELINE_FACTOR,
   FeatureTest,
   getModificationDate,
-  IDENTITY_MATRIX,
   info,
   isArrayEqual,
   LINE_DESCENT_FACTOR,
@@ -44,6 +43,7 @@ import {
   getInheritableProperty,
   getParentToUpdate,
   getRotationMatrix,
+  IDENTITY_MATRIX,
   isNumberArray,
   lookupMatrix,
   lookupNormalRect,

src/core/core_utils.js

@@ -30,6 +30,8 @@ const PDF_VERSION_REGEXP = /^[1-9]\.\d$/;
 const MAX_INT_32 = 2 ** 31 - 1;
 const MIN_INT_32 = -(2 ** 31);
 
+const IDENTITY_MATRIX = [1, 0, 0, 1, 0, 0];
+
 function getLookupTableFactory(initializer) {
   let lookup;
   return function () {
@@ -722,6 +724,7 @@ export {
   getParentToUpdate,
   getRotationMatrix,
   getSizeInBytes,
+  IDENTITY_MATRIX,
   isAscii,
   isBooleanArray,
   isNumberArray,

src/core/evaluator.js

@@ -19,7 +19,6 @@ import {
   DrawOPS,
   FONT_IDENTITY_MATRIX,
   FormatError,
-  IDENTITY_MATRIX,
   info,
   isArrayEqual,
   normalizeUnicode,
@@ -36,6 +35,7 @@ import { compileType3Glyph, FontFlags } from "./fonts_utils.js";
 import { ErrorFont, Font } from "./fonts.js";
 import {
   fetchBinaryData,
+  IDENTITY_MATRIX,
   isNumberArray,
   lookupMatrix,
   lookupNormalRect,
@@ -72,7 +72,6 @@ import { BaseStream } from "./base_stream.js";
 import { bidi } from "./bidi.js";
 import { ColorSpace } from "./colorspace.js";
 import { ColorSpaceUtils } from "./colorspace_utils.js";
-import { DecodeStream } from "./decode_stream.js";
 import { getFontSubstitution } from "./font_substitutions.js";
 import { getGlyphsUnicode } from "./glyphlist.js";
 import { getMetrics } from "./metrics.js";
@@ -571,7 +570,10 @@ class PartialEvaluator {
     localImageCache,
     localColorSpaceCache,
   }) {
-    const dict = image.dict;
+    const { maxImageSize, ignoreErrors, isOffscreenCanvasSupported } =
+      this.options;
+
+    const { dict } = image;
     const imageRef = dict.objId;
     const w = dict.get("W", "Width");
     const h = dict.get("H", "Height");
@@ -580,15 +582,14 @@ class PartialEvaluator {
       warn("Image dimensions are missing, or not numbers.");
       return;
     }
-    const maxImageSize = this.options.maxImageSize;
     if (maxImageSize !== -1 && w * h > maxImageSize) {
       const msg = "Image exceeded maximum allowed size and was removed.";
 
-      if (this.options.ignoreErrors) {
+      if (!ignoreErrors) {
-        warn(msg);
+        throw new Error(msg);
-        return;
       }
-      throw new Error(msg);
+      warn(msg);
+      return;
     }
 
     let optionalContent;
@@ -607,52 +608,10 @@ class PartialEvaluator {
       // data can't be done here. Instead of creating a
       // complete PDFImage, only read the information needed
       // for later.
-      const interpolate = dict.get("I", "Interpolate");
-      const bitStrideLength = (w + 7) >> 3;
-      const imgArray = image.getBytes(bitStrideLength * h);
-      const decode = dict.getArray("D", "Decode");
-
-      if (this.parsingType3Font) {
-        // NOTE: Compared to other image resources we don't bother caching
-        // Type3-glyph image masks, since we've not come across any cases
-        // where that actually helps.
-        // In Type3-glyphs image masks are "always" inline resources,
-        // they're usually fairly small and aren't being re-used either.
-
-        imgData = PDFImage.createRawMask({
-          imgArray,
-          width: w,
-          height: h,
-          imageIsFromDecodeStream: image instanceof DecodeStream,
-          inverseDecode: decode?.[0] > 0,
-          interpolate,
-        });
-        args = compileType3Glyph(imgData);
-
-        if (args) {
-          operatorList.addImageOps(OPS.constructPath, args, optionalContent);
-          return;
-        }
-        warn("Cannot compile Type3 glyph.");
-
-        // If compilation failed, or was disabled, fallback to using an inline
-        // image mask; this case should be extremely rare.
-        operatorList.addImageOps(
-          OPS.paintImageMaskXObject,
-          [imgData],
-          optionalContent
-        );
-        return;
-      }
-
       imgData = await PDFImage.createMask({
-        imgArray,
+        image,
-        width: w,
+        isOffscreenCanvasSupported:
-        height: h,
+          isOffscreenCanvasSupported && !this.parsingType3Font,
-        imageIsFromDecodeStream: image instanceof DecodeStream,
-        inverseDecode: decode?.[0] > 0,
-        interpolate,
-        isOffscreenCanvasSupported: this.options.isOffscreenCanvasSupported,
       });
 
       if (imgData.isSingleOpaquePixel) {
@@ -677,6 +636,36 @@ class PartialEvaluator {
         return;
       }
 
+      if (this.parsingType3Font) {
+        // NOTE: Compared to other image resources we don't bother caching
+        // Type3-glyph image masks, since we've not come across any cases
+        // where that actually helps.
+        // In Type3-glyphs image masks are "always" inline resources,
+        // they're usually fairly small and aren't being re-used either.
+        if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
+          assert(
+            imgData.data instanceof Uint8Array,
+            "Type3 glyph image mask must be a TypedArray."
+          );
+        }
+        args = compileType3Glyph(imgData);
+
+        if (args) {
+          operatorList.addImageOps(OPS.constructPath, args, optionalContent);
+          return;
+        }
+        warn("Cannot compile Type3 glyph.");
+
+        // If compilation failed, or was disabled, fallback to using an inline
+        // image mask; this case should be extremely rare.
+        operatorList.addImageOps(
+          OPS.paintImageMaskXObject,
+          [imgData],
+          optionalContent
+        );
+        return;
+      }
+
       const objId = `mask_${this.idFactory.createObjId()}`;
       operatorList.addDependency(objId);
 
@@ -736,7 +725,7 @@ class PartialEvaluator {
       } catch (reason) {
         const msg = `Unable to decode inline image: "${reason}".`;
 
-        if (!this.options.ignoreErrors) {
+        if (!ignoreErrors) {
           throw new Error(msg);
         }
         warn(msg);
@@ -819,8 +808,7 @@ class PartialEvaluator {
       .then(async imageObj => {
         imgData = await imageObj.createImageData(
           /* forceRGBA = */ false,
-          /* isOffscreenCanvasSupported = */ this.options
+          isOffscreenCanvasSupported
-            .isOffscreenCanvasSupported
         );
         imgData.dataLen = imgData.bitmap
           ? imgData.width * imgData.height * 4

src/core/image.js

@@ -348,58 +348,18 @@ class PDFImage {
     });
   }
 
-  static createRawMask({
+  static async createMask({ image, isOffscreenCanvasSupported = false }) {
-    imgArray,
+    const { dict } = image;
-    width,
+    const width = dict.get("W", "Width");
-    height,
+    const height = dict.get("H", "Height");
-    imageIsFromDecodeStream,
+
-    inverseDecode,
+    const interpolate = dict.get("I", "Interpolate");
-    interpolate,
+    const decode = dict.getArray("D", "Decode");
-  }) {
+    const inverseDecode = decode?.[0] > 0;
-    // |imgArray| might not contain full data for every pixel of the mask, so
-    // we need to distinguish between |computedLength| and |actualLength|.
-    // In particular, if inverseDecode is true, then the array we return must
-    // have a length of |computedLength|.
 
     const computedLength = ((width + 7) >> 3) * height;
-    const actualLength = imgArray.byteLength;
+    const imgArray = image.getBytes(computedLength);
-    const haveFullData = computedLength === actualLength;
-    let data, i;
 
-    if (imageIsFromDecodeStream && (!inverseDecode || haveFullData)) {
-      // imgArray came from a DecodeStream and its data is in an appropriate
-      // form, so we can just transfer it.
-      data = imgArray;
-    } else if (!inverseDecode) {
-      data = new Uint8Array(imgArray);
-    } else {
-      data = new Uint8Array(computedLength);
-      data.set(imgArray);
-      data.fill(0xff, actualLength);
-    }
-
-    // If necessary, invert the original mask data (but not any extra we might
-    // have added above). It's safe to modify the array -- whether it's the
-    // original or a copy, we're about to transfer it anyway, so nothing else
-    // in this thread can be relying on its contents.
-    if (inverseDecode) {
-      for (i = 0; i < actualLength; i++) {
-        data[i] ^= 0xff;
-      }
-    }
-
-    return { data, width, height, interpolate };
-  }
-
-  static async createMask({
-    imgArray,
-    width,
-    height,
-    imageIsFromDecodeStream,
-    inverseDecode,
-    interpolate,
-    isOffscreenCanvasSupported = false,
-  }) {
     const isSingleOpaquePixel =
       width === 1 &&
       height === 1 &&
@@ -452,17 +412,40 @@ class PDFImage {
         bitmap,
       };
     }
-
+    // Fallback to get the data almost as they're and they'll be decoded
-    // Get the data almost as they're and they'll be decoded
     // just before being drawn.
-    return this.createRawMask({
+
-      imgArray,
+    // |imgArray| might not contain full data for every pixel of the mask, so
-      width,
+    // we need to distinguish between |computedLength| and |actualLength|.
-      height,
+    // In particular, if inverseDecode is true, then the array we return must
-      inverseDecode,
+    // have a length of |computedLength|.
-      imageIsFromDecodeStream,
+    const actualLength = imgArray.byteLength;
-      interpolate,
+    const haveFullData = computedLength === actualLength;
-    });
+    let data;
+
+    if (image instanceof DecodeStream && (!inverseDecode || haveFullData)) {
+      // imgArray came from a DecodeStream and its data is in an appropriate
+      // form, so we can just transfer it.
+      data = imgArray;
+    } else if (!inverseDecode) {
+      data = new Uint8Array(imgArray);
+    } else {
+      data = new Uint8Array(computedLength);
+      data.set(imgArray);
+      data.fill(0xff, actualLength);
+    }
+
+    // If necessary, invert the original mask data (but not any extra we might
+    // have added above). It's safe to modify the array -- whether it's the
+    // original or a copy, we're about to transfer it anyway, so nothing else
+    // in this thread can be relying on its contents.
+    if (inverseDecode) {
+      for (let i = 0; i < actualLength; i++) {
+        data[i] ^= 0xff;
+      }
+    }
+
+    return { data, width, height, interpolate };
   }
 
   get drawWidth() {

src/core/pattern.js

@@ -16,7 +16,6 @@
 import {
   assert,
   FormatError,
-  IDENTITY_MATRIX,
   info,
   MathClamp,
   unreachable,
@@ -24,6 +23,7 @@ import {
   warn,
 } from "../shared/util.js";
 import {
+  IDENTITY_MATRIX,
   isBooleanArray,
   isNumberArray,
   lookupMatrix,

src/display/annotation_storage.js

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { objectFromMap, shadow, unreachable } from "../shared/util.js";
+import { shadow, unreachable } from "../shared/util.js";
 import { AnnotationEditor } from "./editor/editor.js";
 import { MurmurHash3_64 } from "../shared/murmurhash3.js";
 
@@ -41,6 +41,17 @@ class AnnotationStorage {
     this.onSetModified = null;
     this.onResetModified = null;
     this.onAnnotationEditor = null;
+
+    if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
+      // For testing purposes.
+      Object.defineProperty(this, "_setValues", {
+        value: obj => {
+          for (const [key, val] of Object.entries(obj)) {
+            this.setValue(key, val);
+          }
+        },
+      });
+    }
   }
 
   /**
@@ -128,22 +139,6 @@ class AnnotationStorage {
     return this.#storage.has(key);
   }
 
-  /**
-   * @returns {Object | null}
-   */
-  getAll() {
-    return this.#storage.size > 0 ? objectFromMap(this.#storage) : null;
-  }
-
-  /**
-   * @param {Object} obj
-   */
-  setAll(obj) {
-    for (const [key, val] of Object.entries(obj)) {
-      this.setValue(key, val);
-    }
-  }
-
   get size() {
     return this.#storage.size;
   }
@@ -278,6 +273,10 @@ class AnnotationStorage {
       hash: ids.join(","),
     });
   }
+
+  [Symbol.iterator]() {
+    return this.#storage.entries();
+  }
 }
 
 /**

src/display/metadata.js

@@ -13,15 +13,13 @@
  * limitations under the License.
  */
 
-import { objectFromMap } from "../shared/util.js";
-
 class Metadata {
-  #metadataMap;
+  #map;
 
   #data;
 
   constructor({ parsedData, rawData }) {
-    this.#metadataMap = parsedData;
+    this.#map = parsedData;
     this.#data = rawData;
   }
 
@@ -30,15 +28,11 @@ class Metadata {
   }
 
   get(name) {
-    return this.#metadataMap.get(name) ?? null;
+    return this.#map.get(name) ?? null;
   }
 
-  getAll() {
+  [Symbol.iterator]() {
-    return objectFromMap(this.#metadataMap);
+    return this.#map.entries();
-  }
-
-  has(name) {
-    return this.#metadataMap.has(name);
   }
 }
 

src/display/optional_content_config.js

@@ -15,7 +15,6 @@
 
 import {
   info,
-  objectFromMap,
   RenderingIntentFlag,
   unreachable,
   warn,
@@ -301,10 +300,6 @@ class OptionalContentConfig {
     return [...this.#groups.keys()];
   }
 
-  getGroups() {
-    return this.#groups.size > 0 ? objectFromMap(this.#groups) : null;
-  }
-
   getGroup(id) {
     return this.#groups.get(id) || null;
   }
@@ -320,6 +315,10 @@ class OptionalContentConfig {
     }
     return (this.#cachedGetHash = hash.hexdigest());
   }
+
+  [Symbol.iterator]() {
+    return this.#groups.entries();
+  }
 }
 
 export { OptionalContentConfig };

src/shared/util.js

@@ -25,7 +25,6 @@ const isNodeJS =
   !process.versions.nw &&
   !(process.versions.electron && process.type && process.type !== "browser");
 
-const IDENTITY_MATRIX = [1, 0, 0, 1, 0, 0];
 const FONT_IDENTITY_MATRIX = [0.001, 0, 0, 0.001, 0, 0];
 
 // Represent the percentage of the height of a single-line field over
@@ -578,16 +577,6 @@ function objectSize(obj) {
   return Object.keys(obj).length;
 }
 
-// Ensure that the returned Object has a `null` prototype; hence why
-// `Object.fromEntries(...)` is not used.
-function objectFromMap(map) {
-  const obj = Object.create(null);
-  for (const [key, value] of map) {
-    obj[key] = value;
-  }
-  return obj;
-}
-
 // Checks the endianness of the platform.
 function isLittleEndian() {
   const buffer8 = new Uint8Array(4);
@@ -1303,7 +1292,6 @@ export {
   getUuid,
   getVerbosityLevel,
   hexNumbers,
-  IDENTITY_MATRIX,
   ImageKind,
   info,
   InvalidPDFException,
@@ -1313,7 +1301,6 @@ export {
   LINE_FACTOR,
   MathClamp,
   normalizeUnicode,
-  objectFromMap,
   objectSize,
   OPS,
   PageActionEventType,

test/driver.js

@@ -738,7 +738,7 @@ class Driver {
                 await page.getAnnotations({ intent: "display" });
               }
             }
-            doc.annotationStorage.setAll(task.annotationStorage);
+            doc.annotationStorage._setValues(task.annotationStorage);
 
             const data = await doc.saveDocument();
             await loadingTask.destroy();
@@ -919,7 +919,7 @@ class Driver {
               pageColors = null;
 
             if (task.annotationStorage) {
-              task.pdfDoc.annotationStorage.setAll(task.annotationStorage);
+              task.pdfDoc.annotationStorage._setValues(task.annotationStorage);
             }
 
             let textLayerCanvas, annotationLayerCanvas, annotationLayerContext;

test/stats/statcmp.js

@@ -76,10 +76,7 @@ function pad(s, length, dir /* default: 'right' */) {
 }
 
 function mean(array) {
-  function add(a, b) {
+  return array.reduce((a, b) => a + b, 0) / array.length;
-    return a + b;
-  }
-  return array.reduce(add, 0) / array.length;
 }
 
 /* Comparator for row key sorting. */

test/unit/annotation_storage_spec.js

@@ -59,7 +59,7 @@ describe("AnnotationStorage", function () {
     it("should set a new value in the annotation storage", function () {
       const annotationStorage = new AnnotationStorage();
       annotationStorage.setValue("123A", { value: "an other string" });
-      const value = annotationStorage.getAll()["123A"].value;
+      const { value } = annotationStorage.getRawValue("123A");
       expect(value).toEqual("an other string");
     });
 

test/unit/metadata_spec.js

@@ -16,8 +16,6 @@
 import { Metadata } from "../../src/display/metadata.js";
 import { MetadataParser } from "../../src/core/metadata_parser.js";
 
-const emptyObj = Object.create(null);
-
 function createMetadata(data) {
   const metadataParser = new MetadataParser(data);
   return new Metadata(metadataParser.serializable);
@@ -33,13 +31,10 @@ describe("metadata", function () {
       "</rdf:Alt></dc:title></rdf:Description></rdf:RDF></x:xmpmeta>";
     const metadata = createMetadata(data);
 
-    expect(metadata.has("dc:title")).toBeTruthy();
-    expect(metadata.has("dc:qux")).toBeFalsy();
-
     expect(metadata.get("dc:title")).toEqual("Foo bar baz");
     expect(metadata.get("dc:qux")).toEqual(null);
 
-    expect(metadata.getAll()).toEqual({ "dc:title": "Foo bar baz" });
+    expect([...metadata]).toEqual([["dc:title", "Foo bar baz"]]);
   });
 
   it("should repair and handle invalid metadata", function () {
@@ -51,13 +46,10 @@ describe("metadata", function () {
       "</rdf:Description></rdf:RDF></x:xmpmeta>";
     const metadata = createMetadata(data);
 
-    expect(metadata.has("dc:title")).toBeTruthy();
-    expect(metadata.has("dc:qux")).toBeFalsy();
-
     expect(metadata.get("dc:title")).toEqual("PDF&");
     expect(metadata.get("dc:qux")).toEqual(null);
 
-    expect(metadata.getAll()).toEqual({ "dc:title": "PDF&" });
+    expect([...metadata]).toEqual([["dc:title", "PDF&"]]);
   });
 
   it("should repair and handle invalid metadata (bug 1424938)", function () {
@@ -94,19 +86,16 @@ describe("metadata", function () {
       "</x:xmpmeta>";
     const metadata = createMetadata(data);
 
-    expect(metadata.has("dc:title")).toBeTruthy();
-    expect(metadata.has("dc:qux")).toBeFalsy();
-
     expect(metadata.get("dc:title")).toEqual(
       "L'Odissee thématique logo Odisséé - décembre 2008.pub"
     );
     expect(metadata.get("dc:qux")).toEqual(null);
 
-    expect(metadata.getAll()).toEqual({
+    expect([...metadata].sort()).toEqual([
-      "dc:creator": ["ODIS"],
+      ["dc:creator", ["ODIS"]],
-      "dc:title": "L'Odissee thématique logo Odisséé - décembre 2008.pub",
+      ["dc:title", "L'Odissee thématique logo Odisséé - décembre 2008.pub"],
-      "xap:creatortool": "PDFCreator Version 0.9.6",
+      ["xap:creatortool", "PDFCreator Version 0.9.6"],
-    });
+    ]);
   });
 
   it("should gracefully handle incomplete tags (issue 8884)", function () {
@@ -137,7 +126,7 @@ describe("metadata", function () {
       '<?xpacket end="w"?>';
     const metadata = createMetadata(data);
 
-    expect(metadata.getAll()).toEqual(emptyObj);
+    expect([...metadata]).toEqual([]);
   });
 
   it('should gracefully handle "junk" before the actual metadata (issue 10395)', function () {
@@ -168,26 +157,23 @@ describe("metadata", function () {
       '</rdf:Description></rdf:RDF></x:xmpmeta><?xpacket end="w"?>';
     const metadata = createMetadata(data);
 
-    expect(metadata.has("dc:title")).toBeTruthy();
-    expect(metadata.has("dc:qux")).toBeFalsy();
-
     expect(metadata.get("dc:title")).toEqual("");
     expect(metadata.get("dc:qux")).toEqual(null);
 
-    expect(metadata.getAll()).toEqual({
+    expect([...metadata].sort()).toEqual([
-      "dc:creator": [""],
+      ["dc:creator", [""]],
-      "dc:description": "",
+      ["dc:description", ""],
-      "dc:format": "application/pdf",
+      ["dc:format", "application/pdf"],
-      "dc:subject": [],
+      ["dc:subject", []],
-      "dc:title": "",
+      ["dc:title", ""],
-      "pdf:keywords": "",
+      ["pdf:keywords", ""],
-      "pdf:pdfversion": "1.7",
+      ["pdf:pdfversion", "1.7"],
-      "pdf:producer": "PDFKit.NET 4.0.102.0",
+      ["pdf:producer", "PDFKit.NET 4.0.102.0"],
-      "xap:createdate": "2018-12-27T13:50:36-08:00",
+      ["xap:createdate", "2018-12-27T13:50:36-08:00"],
-      "xap:creatortool": "",
+      ["xap:creatortool", ""],
-      "xap:metadatadate": "2018-12-27T13:50:38-08:00",
+      ["xap:metadatadate", "2018-12-27T13:50:38-08:00"],
-      "xap:modifydate": "2018-12-27T13:50:38-08:00",
+      ["xap:modifydate", "2018-12-27T13:50:38-08:00"],
-    });
+    ]);
   });
 
   it('should correctly handle metadata containing "&apos" (issue 10407)', function () {
@@ -200,13 +186,10 @@ describe("metadata", function () {
       "</rdf:Alt></dc:title></rdf:Description></rdf:RDF></x:xmpmeta>";
     const metadata = createMetadata(data);
 
-    expect(metadata.has("dc:title")).toBeTruthy();
-    expect(metadata.has("dc:qux")).toBeFalsy();
-
     expect(metadata.get("dc:title")).toEqual("'Foo bar baz'");
     expect(metadata.get("dc:qux")).toEqual(null);
 
-    expect(metadata.getAll()).toEqual({ "dc:title": "'Foo bar baz'" });
+    expect([...metadata]).toEqual([["dc:title", "'Foo bar baz'"]]);
   });
 
   it("should gracefully handle unbalanced end tags (issue 10410)", function () {
@@ -229,7 +212,7 @@ describe("metadata", function () {
       '</rdf:RDF></x:xmpmeta><?xpacket end="w"?>';
     const metadata = createMetadata(data);
 
-    expect(metadata.getAll()).toEqual(emptyObj);
+    expect([...metadata]).toEqual([]);
   });
 
   it("should not be vulnerable to the billion laughs attack", function () {
@@ -258,12 +241,9 @@ describe("metadata", function () {
       "</rdf:RDF>";
     const metadata = createMetadata(data);
 
-    expect(metadata.has("dc:title")).toBeTruthy();
-    expect(metadata.has("dc:qux")).toBeFalsy();
-
     expect(metadata.get("dc:title")).toEqual("a&lol9;b");
     expect(metadata.get("dc:qux")).toEqual(null);
 
-    expect(metadata.getAll()).toEqual({ "dc:title": "a&lol9;b" });
+    expect([...metadata]).toEqual([["dc:title", "a&lol9;b"]]);
   });
 });