From 6381b5b08f69cc55cbc55564956550001cbec653 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 16 Jul 2020 11:57:50 +0200 Subject: [PATCH 1/3] Add a `size` getter, to `Dict` instances, to provide an easier way of checking the number of entries This removes the need to manually call `Dict.getKeys()` and check its length. --- src/core/document.js | 2 +- src/core/primitives.js | 4 ++++ test/unit/primitives_spec.js | 11 +++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/core/document.js b/src/core/document.js index 45b6abcfd1951..1e02d414c9f19 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -551,7 +551,7 @@ class PDFDocument { // Check if a Collection dictionary is present in the document. try { const collection = this.catalog.catDict.get("Collection"); - if (isDict(collection) && collection.getKeys().length > 0) { + if (isDict(collection) && collection.size > 0) { this.collection = collection; } } catch (ex) { diff --git a/src/core/primitives.js b/src/core/primitives.js index f9c7ecbdbe9cf..06d0e21d9cbec 100644 --- a/src/core/primitives.js +++ b/src/core/primitives.js @@ -85,6 +85,10 @@ var Dict = (function DictClosure() { this.xref = newXref; }, + get size() { + return Object.keys(this._map).length; + }, + // automatically dereferences Ref objects get(key1, key2, key3) { let value = this._map[key1]; diff --git a/test/unit/primitives_spec.js b/test/unit/primitives_spec.js index fa1c3ab9b1b3e..cbd3d01f5dfba 100644 --- a/test/unit/primitives_spec.js +++ b/test/unit/primitives_spec.js @@ -117,6 +117,17 @@ describe("primitives", function () { expect(dict.xref).toEqual(xref); }); + it("should return correct size", function () { + const dict = new Dict(null); + expect(dict.size).toEqual(0); + + dict.set("Type", Name.get("Page")); + expect(dict.size).toEqual(1); + + dict.set("Contents", Ref.get(10, 0)); + expect(dict.size).toEqual(2); + }); + it("should return invalid values for unknown keys", function () { checkInvalidHasValues(emptyDict); checkInvalidKeyValues(emptyDict); From ea8e432c45468b30c5dc2ec3768c5a0657b9f531 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 17 Jul 2020 12:57:34 +0200 Subject: [PATCH 2/3] Add a `getRawValues` method, to `Dict` instances, to provide an easier way of getting all *raw* values When the old `Dict.getAll()` method was removed, it was replaced with a `Dict.getKeys()` call and `Dict.get(...)` calls (in a loop). While this pattern obviously makes a lot of sense in many cases, there's some instances where we actually want the *raw* `Dict` values (i.e. `Ref`s where applicable). In those cases, `Dict.getRaw(...)` calls are instead used within the loop. However, by introducing a new `Dict.getRawValues()` method we can reduce the number of (strictly unnecessary) function calls by simply getting the *raw* `Dict` values directly. --- src/core/evaluator.js | 10 +++------- src/core/obj.js | 7 ++----- src/core/primitives.js | 5 +++++ test/unit/primitives_spec.js | 29 +++++++++++++++++++++++++++++ 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/core/evaluator.js b/src/core/evaluator.js index 1b2376b3c4d50..a67ad4ef58906 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -254,8 +254,7 @@ class PartialEvaluator { // First check the current resources for blend modes. var graphicStates = node.get("ExtGState"); if (graphicStates instanceof Dict) { - for (const key of graphicStates.getKeys()) { - let graphicState = graphicStates.getRaw(key); + for (let graphicState of graphicStates.getRawValues()) { if (graphicState instanceof Ref) { if (processed.has(graphicState)) { continue; // The ExtGState has already been processed. @@ -301,8 +300,7 @@ class PartialEvaluator { if (!(xObjects instanceof Dict)) { continue; } - for (const key of xObjects.getKeys()) { - var xObject = xObjects.getRaw(key); + for (let xObject of xObjects.getRawValues()) { if (xObject instanceof Ref) { if (processed.has(xObject)) { // The XObject has already been processed, and by avoiding a @@ -3078,9 +3076,7 @@ class PartialEvaluator { } else if (isRef(encoding)) { hash.update(encoding.toString()); } else if (isDict(encoding)) { - var keys = encoding.getKeys(); - for (var i = 0, ii = keys.length; i < ii; i++) { - var entry = encoding.getRaw(keys[i]); + for (const entry of encoding.getRawValues()) { if (isName(entry)) { hash.update(entry.name); } else if (isRef(entry)) { diff --git a/src/core/obj.js b/src/core/obj.js index fdaa1bd178aae..aa468c1e738e7 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -2207,16 +2207,13 @@ const ObjectLoader = (function () { function addChildren(node, nodesToVisit) { if (node instanceof Dict || isStream(node)) { const dict = node instanceof Dict ? node : node.dict; - const dictKeys = dict.getKeys(); - for (let i = 0, ii = dictKeys.length; i < ii; i++) { - const rawValue = dict.getRaw(dictKeys[i]); + for (const rawValue of dict.getRawValues()) { if (mayHaveChildren(rawValue)) { nodesToVisit.push(rawValue); } } } else if (Array.isArray(node)) { - for (let i = 0, ii = node.length; i < ii; i++) { - const value = node[i]; + for (const value of node) { if (mayHaveChildren(value)) { nodesToVisit.push(value); } diff --git a/src/core/primitives.js b/src/core/primitives.js index 06d0e21d9cbec..eb53ccb82c42d 100644 --- a/src/core/primitives.js +++ b/src/core/primitives.js @@ -144,6 +144,11 @@ var Dict = (function DictClosure() { return Object.keys(this._map); }, + // no dereferencing + getRawValues: function Dict_getRawValues() { + return Object.values(this._map); + }, + set: function Dict_set(key, value) { if ( (typeof PDFJSDev === "undefined" || diff --git a/test/unit/primitives_spec.js b/test/unit/primitives_spec.js index cbd3d01f5dfba..7f3e375b7984b 100644 --- a/test/unit/primitives_spec.js +++ b/test/unit/primitives_spec.js @@ -275,6 +275,35 @@ describe("primitives", function () { expect(keys.sort()).toEqual(expectedKeys); }); + it("should get all raw values", function () { + // Test direct objects: + const expectedRawValues1 = [testFontFile, testFontFile2, testFontFile3]; + const rawValues1 = dictWithManyKeys.getRawValues(); + + expect(rawValues1.sort()).toEqual(expectedRawValues1); + + // Test indirect objects: + const typeName = Name.get("Page"); + const resources = new Dict(null), + resourcesRef = Ref.get(5, 0); + const contents = new StringStream("data"), + contentsRef = Ref.get(10, 0); + const xref = new XRefMock([ + { ref: resourcesRef, data: resources }, + { ref: contentsRef, data: contents }, + ]); + + const dict = new Dict(xref); + dict.set("Type", typeName); + dict.set("Resources", resourcesRef); + dict.set("Contents", contentsRef); + + const expectedRawValues2 = [contentsRef, resourcesRef, typeName]; + const rawValues2 = dict.getRawValues(); + + expect(rawValues2.sort()).toEqual(expectedRawValues2); + }); + it("should create only one object for Dict.empty", function () { const firstDictEmpty = Dict.empty; const secondDictEmpty = Dict.empty; From 684a7b89ac27636735e4e92ebad25d98b81eaeeb Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 17 Jul 2020 13:50:02 +0200 Subject: [PATCH 3/3] Remove unnecessary duplication in the `addChildren` helper function (used by the `ObjectLoader`) Besides being fewer lines of code overall, this also avoids *one* `node instanceof Dict` check for both of the `Dict`/`Stream`-cases. --- src/core/obj.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/core/obj.js b/src/core/obj.js index aa468c1e738e7..fd1986e9d0771 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -2205,18 +2205,16 @@ const ObjectLoader = (function () { } function addChildren(node, nodesToVisit) { - if (node instanceof Dict || isStream(node)) { - const dict = node instanceof Dict ? node : node.dict; - for (const rawValue of dict.getRawValues()) { - if (mayHaveChildren(rawValue)) { - nodesToVisit.push(rawValue); - } - } - } else if (Array.isArray(node)) { - for (const value of node) { - if (mayHaveChildren(value)) { - nodesToVisit.push(value); - } + if (node instanceof Dict) { + node = node.getRawValues(); + } else if (isStream(node)) { + node = node.dict.getRawValues(); + } else if (!Array.isArray(node)) { + return; + } + for (const rawValue of node) { + if (mayHaveChildren(rawValue)) { + nodesToVisit.push(rawValue); } } }