From ab672f0b778c898664edea8c45e0d6ab26a84900 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 17 May 2025 15:23:55 +0200 Subject: [PATCH 1/2] Replace `PDFWorker.fromPort` with a generic `PDFWorker.create` method This allows us to simply invoke `PDFWorker.create` unconditionally from the `getDocument` function, without having to manually check if a global `workerPort` is available first. --- src/display/api.js | 33 +++++++++++++++++++-------------- test/unit/api_spec.js | 2 +- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 83e79b2dd..81527051e 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -38,14 +38,15 @@ import { PrintAnnotationStorage, SerializableEmpty, } from "./annotation_storage.js"; -import { FontFaceObject, FontLoader } from "./font_loader.js"; import { + deprecated, isDataScheme, isValidFetchUrl, PageViewport, RenderingCancelledException, StatTimer, } from "./display_utils.js"; +import { FontFaceObject, FontLoader } from "./font_loader.js"; import { MessageHandler, wrapReason } from "../shared/message_handler.js"; import { NodeCanvasFactory, @@ -383,15 +384,12 @@ function getDocument(src = {}) { }; if (!worker) { - const workerParams = { - verbosity, - port: GlobalWorkerOptions.workerPort, - }; // Worker was not provided -- creating and owning our own. If message port // is specified in global worker options, using it. - worker = workerParams.port - ? PDFWorker.fromPort(workerParams) - : new PDFWorker(workerParams); + worker = PDFWorker.create({ + verbosity, + port: GlobalWorkerOptions.workerPort, + }); task._worker = worker; } @@ -2131,6 +2129,16 @@ class PDFWorker { new Blob([wrapper], { type: "text/javascript" }) ); }; + + this.fromPort = params => { + deprecated( + "`PDFWorker.fromPort` - please use `PDFWorker.create` instead." + ); + if (!params?.port) { + throw new Error("PDFWorker.fromPort - invalid method signature."); + } + return this.create(params); + }; } if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { @@ -2364,15 +2372,12 @@ class PDFWorker { * @param {PDFWorkerParameters} params - The worker initialization parameters. * @returns {PDFWorker} */ - static fromPort(params) { - if (!params?.port) { - throw new Error("PDFWorker.fromPort - invalid method signature."); - } - const cachedPort = this.#workerPorts.get(params.port); + static create(params) { + const cachedPort = this.#workerPorts.get(params?.port); if (cachedPort) { if (cachedPort._pendingDestroy) { throw new Error( - "PDFWorker.fromPort - the worker is being destroyed.\n" + + "PDFWorker.create - the worker is being destroyed.\n" + "Please remember to await `PDFDocumentLoadingTask.destroy()`-calls." ); } diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index e9c84f7f3..6399961ea 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -1082,7 +1082,7 @@ describe("api", function () { getDocument(tracemonkeyGetDocumentParams); }).toThrow( new Error( - "PDFWorker.fromPort - the worker is being destroyed.\n" + + "PDFWorker.create - the worker is being destroyed.\n" + "Please remember to await `PDFDocumentLoadingTask.destroy()`-calls." ) ); From fc697b36020d7bbafd63bc1c7b239229c1c04d60 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 17 May 2025 15:41:04 +0200 Subject: [PATCH 2/2] Utilize private fields and methods more in the `PDFWorker` class This replaces, wherever possible, the old semi-private fields and methods with actually private ones. --- src/display/api.js | 88 ++++++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 38 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 81527051e..61b326891 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2091,6 +2091,14 @@ class LoopbackPort { * @param {PDFWorkerParameters} params - The worker initialization parameters. */ class PDFWorker { + #capability = Promise.withResolvers(); + + #messageHandler = null; + + #port = null; + + #webWorker = null; + static #fakeWorkerId = 0; static #isWorkerDisabled = false; @@ -2158,20 +2166,24 @@ class PDFWorker { this.destroyed = false; this.verbosity = verbosity; - this._readyCapability = Promise.withResolvers(); - this._port = null; - this._webWorker = null; - this._messageHandler = null; - if (port) { if (PDFWorker.#workerPorts.has(port)) { throw new Error("Cannot use more than one PDFWorker per port."); } PDFWorker.#workerPorts.set(port, this); - this._initializeFromPort(port); - return; + this.#initializeFromPort(port); + } else { + this.#initialize(); + } + + if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) { + // For testing purposes. + Object.defineProperty(this, "_webWorker", { + get() { + return this.#webWorker; + }, + }); } - this._initialize(); } /** @@ -2179,13 +2191,13 @@ class PDFWorker { * @type {Promise} */ get promise() { - return this._readyCapability.promise; + return this.#capability.promise; } #resolve() { - this._readyCapability.resolve(); + this.#capability.resolve(); // Send global setting, e.g. verbosity level. - this._messageHandler.send("configure", { + this.#messageHandler.send("configure", { verbosity: this.verbosity, }); } @@ -2195,7 +2207,7 @@ class PDFWorker { * @type {Worker} */ get port() { - return this._port; + return this.#port; } /** @@ -2203,20 +2215,20 @@ class PDFWorker { * @type {MessageHandler} */ get messageHandler() { - return this._messageHandler; + return this.#messageHandler; } - _initializeFromPort(port) { - this._port = port; - this._messageHandler = new MessageHandler("main", "worker", port); - this._messageHandler.on("ready", function () { + #initializeFromPort(port) { + this.#port = port; + this.#messageHandler = new MessageHandler("main", "worker", port); + this.#messageHandler.on("ready", () => { // Ignoring "ready" event -- MessageHandler should already be initialized // and ready to accept messages. }); this.#resolve(); } - _initialize() { + #initialize() { // If worker support isn't disabled explicit and the browser has worker // support, create a new web worker and test if it/the browser fulfills // all requirements to run parts of pdf.js in a web worker. @@ -2226,7 +2238,7 @@ class PDFWorker { PDFWorker.#isWorkerDisabled || PDFWorker.#mainThreadWorkerMessageHandler ) { - this._setupFakeWorker(); + this.#setupFakeWorker(); return; } let { workerSrc } = PDFWorker; @@ -2251,11 +2263,11 @@ class PDFWorker { messageHandler.destroy(); worker.terminate(); if (this.destroyed) { - this._readyCapability.reject(new Error("Worker was destroyed")); + this.#capability.reject(new Error("Worker was destroyed")); } else { // Fall back to fake worker if the termination is caused by an // error (e.g. NetworkError / SecurityError). - this._setupFakeWorker(); + this.#setupFakeWorker(); } }; @@ -2263,7 +2275,7 @@ class PDFWorker { worker.addEventListener( "error", () => { - if (!this._webWorker) { + if (!this.#webWorker) { // Worker failed to initialize due to an error. Clean up and fall // back to the fake worker. terminateEarly(); @@ -2278,9 +2290,9 @@ class PDFWorker { terminateEarly(); return; } - this._messageHandler = messageHandler; - this._port = worker; - this._webWorker = worker; + this.#messageHandler = messageHandler; + this.#port = worker; + this.#webWorker = worker; this.#resolve(); }); @@ -2295,7 +2307,7 @@ class PDFWorker { sendTest(); } catch { // We need fallback to a faked worker. - this._setupFakeWorker(); + this.#setupFakeWorker(); } }); @@ -2315,10 +2327,10 @@ class PDFWorker { } // Either workers are not supported or have thrown an exception. // Thus, we fallback to a faked worker. - this._setupFakeWorker(); + this.#setupFakeWorker(); } - _setupFakeWorker() { + #setupFakeWorker() { if (!PDFWorker.#isWorkerDisabled) { warn("Setting up fake worker."); PDFWorker.#isWorkerDisabled = true; @@ -2327,11 +2339,11 @@ class PDFWorker { PDFWorker._setupFakeWorkerGlobal .then(WorkerMessageHandler => { if (this.destroyed) { - this._readyCapability.reject(new Error("Worker was destroyed")); + this.#capability.reject(new Error("Worker was destroyed")); return; } const port = new LoopbackPort(); - this._port = port; + this.#port = port; // All fake workers use the same port, making id unique. const id = `fake${PDFWorker.#fakeWorkerId++}`; @@ -2341,11 +2353,11 @@ class PDFWorker { const workerHandler = new MessageHandler(id + "_worker", id, port); WorkerMessageHandler.setup(workerHandler, port); - this._messageHandler = new MessageHandler(id, id + "_worker", port); + this.#messageHandler = new MessageHandler(id, id + "_worker", port); this.#resolve(); }) .catch(reason => { - this._readyCapability.reject( + this.#capability.reject( new Error(`Setting up fake worker failed: "${reason.message}".`) ); }); @@ -2358,14 +2370,14 @@ class PDFWorker { this.destroyed = true; // We need to terminate only web worker created resource. - this._webWorker?.terminate(); - this._webWorker = null; + this.#webWorker?.terminate(); + this.#webWorker = null; - PDFWorker.#workerPorts.delete(this._port); - this._port = null; + PDFWorker.#workerPorts.delete(this.#port); + this.#port = null; - this._messageHandler?.destroy(); - this._messageHandler = null; + this.#messageHandler?.destroy(); + this.#messageHandler = null; } /**