Skip to content

Commit 326df1f

Browse files
committed
Update the deepCompare helper function to handle Refs and Names correctly
Note that `Ref`s and `Name`s are cached globally[1], since that helps reduce object creation (a lot) during parsing. That cache will be cleared after a period of inactivity in the viewer[2], which is why those primitives cannot *safely* be compared with just `===`/`!==` and also (partially) why abstractions such as `RefSet`/`RefSetCache` are necessary. Currently `deepCompare` doesn't handle `Ref`s and `Name`s correctly, which may lead to future *intermittent* bugs in any code using the `deepCompare` helper function. --- [1] This applies to `Cmd` as well, however that doesn't matter in the context of this patch. [2] Currently, and for more than a decade, set to 30 seconds.
1 parent 702d60a commit 326df1f

2 files changed

Lines changed: 52 additions & 2 deletions

File tree

src/core/core_utils.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {
2323
Util,
2424
warn,
2525
} from "../shared/util.js";
26-
import { Dict, isName, Ref, RefSet } from "./primitives.js";
26+
import { Dict, isName, isRefsEqual, Name, Ref, RefSet } from "./primitives.js";
2727
import { BaseStream } from "./base_stream.js";
2828

2929
const PDF_VERSION_REGEXP = /^[1-9]\.\d$/;
@@ -210,6 +210,13 @@ function deepCompare(a, b) {
210210
if (a === b) {
211211
return true;
212212
}
213+
if (a instanceof Ref && b instanceof Ref) {
214+
return isRefsEqual(a, b);
215+
}
216+
if (a instanceof Name && b instanceof Name) {
217+
return a.name === b.name;
218+
}
219+
213220
if (a instanceof Dict && b instanceof Dict) {
214221
if (a.size !== b.size) {
215222
return false;

test/unit/core_utils_spec.js

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,12 @@ import {
3131
toRomanNumerals,
3232
validateCSSFont,
3333
} from "../../src/core/core_utils.js";
34-
import { Dict, Ref } from "../../src/core/primitives.js";
34+
import {
35+
clearPrimitiveCaches,
36+
Dict,
37+
Name,
38+
Ref,
39+
} from "../../src/core/primitives.js";
3540
import { XRefMock } from "./test_utils.js";
3641

3742
describe("core_utils", function () {
@@ -498,6 +503,20 @@ describe("core_utils", function () {
498503
expect(deepCompare(a, b)).toBeTrue();
499504
});
500505

506+
it("should return true for Dicts with same Ref values, after clearing cached Refs", function () {
507+
const refA = Ref.get(10, 0);
508+
clearPrimitiveCaches();
509+
const refB = Ref.get(10, 0);
510+
// Ensure that Ref-objects are not identical, after clearing the cache.
511+
expect(refA).not.toBe(refB);
512+
513+
const a = new Dict();
514+
a.set("Foo", refA);
515+
const b = new Dict();
516+
b.set("Foo", refB);
517+
expect(deepCompare(a, b)).toBeTrue();
518+
});
519+
501520
it("should return false for Dicts with different Ref values", function () {
502521
const a = new Dict();
503522
a.set("Foo", Ref.get(10, 0));
@@ -555,6 +574,30 @@ describe("core_utils", function () {
555574
it("should return false for arrays with different values", function () {
556575
expect(deepCompare([Ref.get(1, 0)], [Ref.get(2, 0)])).toBeFalse();
557576
});
577+
578+
it("should return true for equal Names", function () {
579+
const name1 = Name.get("name"),
580+
name2 = Name.get("name");
581+
expect(name1).toBe(name2); // Names are cached.
582+
583+
expect(deepCompare(name1, name2)).toBeTrue();
584+
});
585+
586+
it("should return false for different Names", function () {
587+
const name1 = Name.get("name"),
588+
name2 = Name.get("otherName");
589+
expect(deepCompare(name1, name2)).toBeFalse();
590+
});
591+
592+
it("should return true for equal Names, after clearing cached Names", function () {
593+
const name1 = Name.get("name");
594+
clearPrimitiveCaches();
595+
const name2 = Name.get("name");
596+
// Ensure that Name-objects are not identical, after clearing the cache.
597+
expect(name1).not.toBe(name2);
598+
599+
expect(deepCompare(name1, name2)).toBeTrue();
600+
});
558601
});
559602

560603
describe("getSizeInBytes", function () {

0 commit comments

Comments
 (0)