Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the transfers computation into the AnnotationStorage class #16620

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions src/display/annotation_storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import { objectFromMap, unreachable } from "../shared/util.js";
import { AnnotationEditor } from "./editor/editor.js";
import { MurmurHash3_64 } from "../shared/murmurhash3.js";

const SerializableEmpty = Object.freeze({
map: null,
hash: "",
transfers: undefined,
});

/**
* Key/value storage for annotation data in forms.
*/
Expand Down Expand Up @@ -171,34 +177,27 @@ class AnnotationStorage {
*/
get serializable() {
if (this.#storage.size === 0) {
return null;
return SerializableEmpty;
}
const clone = new Map();

const map = new Map(),
hash = new MurmurHash3_64(),
transfers = [];
for (const [key, val] of this.#storage) {
const serialized =
val instanceof AnnotationEditor ? val.serialize() : val;
if (serialized) {
clone.set(key, serialized);
}
}
return clone;
}
map.set(key, serialized);

/**
* PLEASE NOTE: Only intended for usage within the API itself.
* @ignore
*/
static getHash(map) {
if (!map) {
return "";
}
const hash = new MurmurHash3_64();
hash.update(`${key}:${JSON.stringify(serialized)}`);

for (const [key, val] of map) {
hash.update(`${key}:${JSON.stringify(val)}`);
if (serialized.bitmap) {
transfers.push(serialized.bitmap);
}
}
}
return hash.hexdigest();
return map.size > 0
? { map, hash: hash.hexdigest(), transfers }
: SerializableEmpty;
}
}

Expand All @@ -208,12 +207,21 @@ class AnnotationStorage {
* contents. (Necessary since printing is triggered synchronously in browsers.)
*/
class PrintAnnotationStorage extends AnnotationStorage {
#serializable = null;
#serializable;

constructor(parent) {
super();
const { map, hash, transfers } = parent.serializable;
// Create a *copy* of the data, since Objects are passed by reference in JS.
this.#serializable = structuredClone(parent.serializable);
const clone = structuredClone(
map,
(typeof PDFJSDev === "undefined" ||
PDFJSDev.test("SKIP_BABEL || TESTING")) &&
transfers
? { transfer: transfers }
: null
);
this.#serializable = { map: clone, hash, transfers };
}

/**
Expand All @@ -233,4 +241,4 @@ class PrintAnnotationStorage extends AnnotationStorage {
}
}

export { AnnotationStorage, PrintAnnotationStorage };
export { AnnotationStorage, PrintAnnotationStorage, SerializableEmpty };
42 changes: 15 additions & 27 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
import {
AnnotationStorage,
PrintAnnotationStorage,
SerializableEmpty,
} from "./annotation_storage.js";
import {
deprecated,
Expand Down Expand Up @@ -1811,30 +1812,26 @@ class PDFPageProxy {
/**
* @private
*/
_pumpOperatorList({ renderingIntent, cacheKey, annotationStorageMap }) {
_pumpOperatorList({
renderingIntent,
cacheKey,
annotationStorageSerializable,
}) {
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
assert(
Number.isInteger(renderingIntent) && renderingIntent > 0,
'_pumpOperatorList: Expected valid "renderingIntent" argument.'
);
}

const transfers = [];
if (annotationStorageMap) {
for (const annotation of annotationStorageMap.values()) {
if (annotation.bitmap) {
transfers.push(annotation.bitmap);
}
}
}
const { map, transfers } = annotationStorageSerializable;

const readableStream = this._transport.messageHandler.sendWithStream(
"GetOperatorList",
{
pageIndex: this._pageIndex,
intent: renderingIntent,
cacheKey,
annotationStorage: annotationStorageMap,
annotationStorage: map,
},
transfers
);
Expand Down Expand Up @@ -2459,7 +2456,7 @@ class WorkerTransport {
isOpList = false
) {
let renderingIntent = RenderingIntentFlag.DISPLAY; // Default value.
let annotationMap = null;
let annotationStorageSerializable = SerializableEmpty;

switch (intent) {
case "any":
Expand Down Expand Up @@ -2492,7 +2489,7 @@ class WorkerTransport {
? printAnnotationStorage
: this.annotationStorage;

annotationMap = annotationStorage.serializable;
annotationStorageSerializable = annotationStorage.serializable;
break;
default:
warn(`getRenderingIntent - invalid annotationMode: ${annotationMode}`);
Expand All @@ -2504,10 +2501,8 @@ class WorkerTransport {

return {
renderingIntent,
cacheKey: `${renderingIntent}_${AnnotationStorage.getHash(
annotationMap
)}`,
annotationStorageMap: annotationMap,
cacheKey: `${renderingIntent}_${annotationStorageSerializable.hash}`,
annotationStorageSerializable,
};
}

Expand Down Expand Up @@ -2915,22 +2910,15 @@ class WorkerTransport {
"please use the getData-method instead."
);
}
const annotationStorage = this.annotationStorage.serializable;
const transfers = [];
if (annotationStorage) {
for (const annotation of annotationStorage.values()) {
if (annotation.bitmap) {
transfers.push(annotation.bitmap);
}
}
}
const { map, transfers } = this.annotationStorage.serializable;

return this.messageHandler
.sendWithPromise(
"SaveDocument",
{
isPureXfa: !!this._htmlForXfa,
numPages: this._numPages,
annotationStorage,
annotationStorage: map,
filename: this._fullReader?.filename ?? null,
},
transfers
Expand Down
26 changes: 12 additions & 14 deletions test/integration/freetext_editor_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -688,13 +688,12 @@ describe("FreeText Editor", () => {
}

const serialize = proprName =>
page.evaluate(
name =>
[
...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.values(),
].map(x => x[name]),
proprName
);
page.evaluate(name => {
const { map } =
window.PDFViewerApplication.pdfDocument.annotationStorage
.serializable;
return map ? Array.from(map.values(), x => x[name]) : [];
}, proprName);

expect(await serialize("value"))
.withContext(`In ${browserName}`)
Expand Down Expand Up @@ -805,13 +804,12 @@ describe("FreeText Editor", () => {
}

const serialize = proprName =>
page.evaluate(
name =>
[
...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.values(),
].map(x => x[name]),
proprName
);
page.evaluate(name => {
const { map } =
window.PDFViewerApplication.pdfDocument.annotationStorage
.serializable;
return map ? Array.from(map.values(), x => x[name]) : [];
}, proprName);

const rects = (await serialize("rect")).map(rect =>
rect.slice(0, 2).map(x => Math.floor(x))
Expand Down
8 changes: 5 additions & 3 deletions test/integration/test_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,11 @@ const mockClipboard = async pages => {
exports.mockClipboard = mockClipboard;

const getSerialized = page =>
page.evaluate(() => [
...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.values(),
]);
page.evaluate(() => {
const { map } =
window.PDFViewerApplication.pdfDocument.annotationStorage.serializable;
return map ? [...map.values()] : [];
});
exports.getSerialized = getSerialized;

function getEditors(page, kind) {
Expand Down
9 changes: 2 additions & 7 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ import {
RenderingCancelledException,
StatTimer,
} from "../../src/display/display_utils.js";
import { AnnotationStorage } from "../../src/display/annotation_storage.js";
import { AutoPrintRegExp } from "../../web/ui_utils.js";
import { GlobalImageCache } from "../../src/core/image_utils.js";
import { GlobalWorkerOptions } from "../../src/display/worker_options.js";
Expand Down Expand Up @@ -3525,12 +3524,8 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`)
// Update the contents of the form-field again.
annotationStorage.setValue("22R", { value: "Printing again..." });

const annotationHash = AnnotationStorage.getHash(
annotationStorage.serializable
);
const printAnnotationHash = AnnotationStorage.getHash(
printAnnotationStorage.serializable
);
const { hash: annotationHash } = annotationStorage.serializable;
const { hash: printAnnotationHash } = printAnnotationStorage.serializable;
// Sanity check to ensure that the print-storage didn't change,
// after the form-field was updated.
expect(printAnnotationHash).not.toEqual(annotationHash);
Expand Down