From 39113baa3302ea2bff27ec1f128b3f3741f7bfa2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 29 Jun 2023 08:43:02 +0200 Subject: [PATCH] Move the `transfers` computation into the `AnnotationStorage` class Rather than having to *manually* determine the potential `transfers` at various spots in the API, we can let the `AnnotationStorage.serializable` getter include this. To further simplify things, we can also let the `serializable` getter compute and include the `hash`-string as well. --- src/display/annotation_storage.js | 56 ++++++++++++++---------- src/display/api.js | 42 +++++++----------- test/integration/freetext_editor_spec.js | 26 +++++------ test/integration/test_utils.js | 8 ++-- test/unit/api_spec.js | 9 +--- 5 files changed, 66 insertions(+), 75 deletions(-) diff --git a/src/display/annotation_storage.js b/src/display/annotation_storage.js index 4fe1f478a..559638478 100644 --- a/src/display/annotation_storage.js +++ b/src/display/annotation_storage.js @@ -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. */ @@ -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); + map.set(key, serialized); + + hash.update(`${key}:${JSON.stringify(serialized)}`); + + if (serialized.bitmap) { + transfers.push(serialized.bitmap); + } } } - return clone; - } - - /** - * PLEASE NOTE: Only intended for usage within the API itself. - * @ignore - */ - static getHash(map) { - if (!map) { - return ""; - } - const hash = new MurmurHash3_64(); - - for (const [key, val] of map) { - hash.update(`${key}:${JSON.stringify(val)}`); - } - return hash.hexdigest(); + return map.size > 0 + ? { map, hash: hash.hexdigest(), transfers } + : SerializableEmpty; } } @@ -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 }; } /** @@ -233,4 +241,4 @@ class PrintAnnotationStorage extends AnnotationStorage { } } -export { AnnotationStorage, PrintAnnotationStorage }; +export { AnnotationStorage, PrintAnnotationStorage, SerializableEmpty }; diff --git a/src/display/api.js b/src/display/api.js index 002e164df..98e96115e 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -41,6 +41,7 @@ import { import { AnnotationStorage, PrintAnnotationStorage, + SerializableEmpty, } from "./annotation_storage.js"; import { deprecated, @@ -1811,22 +1812,18 @@ 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", @@ -1834,7 +1831,7 @@ class PDFPageProxy { pageIndex: this._pageIndex, intent: renderingIntent, cacheKey, - annotationStorage: annotationStorageMap, + annotationStorage: map, }, transfers ); @@ -2459,7 +2456,7 @@ class WorkerTransport { isOpList = false ) { let renderingIntent = RenderingIntentFlag.DISPLAY; // Default value. - let annotationMap = null; + let annotationStorageSerializable = SerializableEmpty; switch (intent) { case "any": @@ -2492,7 +2489,7 @@ class WorkerTransport { ? printAnnotationStorage : this.annotationStorage; - annotationMap = annotationStorage.serializable; + annotationStorageSerializable = annotationStorage.serializable; break; default: warn(`getRenderingIntent - invalid annotationMode: ${annotationMode}`); @@ -2504,10 +2501,8 @@ class WorkerTransport { return { renderingIntent, - cacheKey: `${renderingIntent}_${AnnotationStorage.getHash( - annotationMap - )}`, - annotationStorageMap: annotationMap, + cacheKey: `${renderingIntent}_${annotationStorageSerializable.hash}`, + annotationStorageSerializable, }; } @@ -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 diff --git a/test/integration/freetext_editor_spec.js b/test/integration/freetext_editor_spec.js index df0013268..fd38ac800 100644 --- a/test/integration/freetext_editor_spec.js +++ b/test/integration/freetext_editor_spec.js @@ -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}`) @@ -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)) diff --git a/test/integration/test_utils.js b/test/integration/test_utils.js index 69414a910..696aaf5f9 100644 --- a/test/integration/test_utils.js +++ b/test/integration/test_utils.js @@ -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) { diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index b9e027ad3..6f209b29d 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -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"; @@ -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);