From afb57e35ef63f79de5296ef8bc73bf0e2f845116 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 | 4 +- test/integration/test_utils.js | 2 +- test/unit/api_spec.js | 9 +--- 5 files changed, 52 insertions(+), 61 deletions(-) diff --git a/src/display/annotation_storage.js b/src/display/annotation_storage.js index 4fe1f478a5bb32..d641c75bda7d1c 100644 --- a/src/display/annotation_storage.js +++ b/src/display/annotation_storage.js @@ -17,6 +17,25 @@ 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, +}); + +function getHashAndTransfers(map) { + const hash = new MurmurHash3_64(), + transfers = []; + for (const [key, val] of map) { + hash.update(`${key}:${JSON.stringify(val)}`); + + if (val.bitmap) { + transfers.push(val.bitmap); + } + } + return { hash: hash.hexdigest(), transfers }; +} + /** * Key/value storage for annotation data in forms. */ @@ -171,34 +190,20 @@ class AnnotationStorage { */ get serializable() { if (this.#storage.size === 0) { - return null; + return SerializableEmpty; } - const clone = new Map(); + const map = new Map(); 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); } } - 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, ...getHashAndTransfers(map) } + : SerializableEmpty; } } @@ -208,12 +213,15 @@ class AnnotationStorage { * contents. (Necessary since printing is triggered synchronously in browsers.) */ class PrintAnnotationStorage extends AnnotationStorage { - #serializable = null; + #serializable; - constructor(parent) { + constructor({ serializable }) { super(); // Create a *copy* of the data, since Objects are passed by reference in JS. - this.#serializable = structuredClone(parent.serializable); + const map = structuredClone(serializable.map); + this.#serializable = map + ? { map, ...getHashAndTransfers(map) } + : SerializableEmpty; } /** @@ -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 002e164df442b3..98e96115e2cd94 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 df00132687732f..22614c67c2547f 100644 --- a/test/integration/freetext_editor_spec.js +++ b/test/integration/freetext_editor_spec.js @@ -691,7 +691,7 @@ describe("FreeText Editor", () => { page.evaluate( name => [ - ...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.values(), + ...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.map.values(), ].map(x => x[name]), proprName ); @@ -808,7 +808,7 @@ describe("FreeText Editor", () => { page.evaluate( name => [ - ...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.values(), + ...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.map.values(), ].map(x => x[name]), proprName ); diff --git a/test/integration/test_utils.js b/test/integration/test_utils.js index 69414a91042903..e85cba93b06019 100644 --- a/test/integration/test_utils.js +++ b/test/integration/test_utils.js @@ -137,7 +137,7 @@ exports.mockClipboard = mockClipboard; const getSerialized = page => page.evaluate(() => [ - ...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.values(), + ...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.map.values(), ]); exports.getSerialized = getSerialized; diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index b9e027ad3acfd9..6f209b29db8b52 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);