From d6612b34274e88e72b699d62093d25cc53ca91f2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 15 Jun 2024 13:10:54 +0200 Subject: [PATCH 1/5] Remove some now redundant validation in `getDocument` Given that we now check/validate all options properly this old code can be simplified. --- src/display/api.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 43954cabd..20d8226fc 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -229,7 +229,7 @@ const DefaultStandardFontDataFactory = * already populated with data, or a parameter object. * @returns {PDFDocumentLoadingTask} */ -function getDocument(src) { +function getDocument(src = {}) { if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { if (typeof src === "string" || src instanceof URL) { src = { url: src }; @@ -237,14 +237,6 @@ function getDocument(src) { src = { data: src }; } } - if (typeof src !== "object") { - throw new Error("Invalid parameter in getDocument, need parameter object."); - } - if (!src.url && !src.data && !src.range) { - throw new Error( - "Invalid parameter object: need either .data, .range or .url" - ); - } const task = new PDFDocumentLoadingTask(); const { docId } = task; @@ -423,6 +415,9 @@ function getDocument(src) { if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) { throw new Error("Not implemented: createPDFNetworkStream"); } + if (!url) { + throw new Error("getDocument - no `url` parameter provided."); + } const createPDFNetworkStream = params => { if ( typeof PDFJSDev !== "undefined" && From 0a36b667e4ddb5c5e9b3ba0589f9e0ba9905d02f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 15 Jun 2024 13:17:55 +0200 Subject: [PATCH 2/5] Use an early return in `PDFWorker.prototype._initialize` when workers are disabled This helps reduce overall indentation in the method, thus leading to slightly less code. Also, remove an old comment referring to Chrome 15 since that's no longer relevant now. --- src/display/api.js | 174 +++++++++++++++++++++++---------------------- 1 file changed, 88 insertions(+), 86 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 20d8226fc..ef2511fdb 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2124,103 +2124,105 @@ class PDFWorker { // 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. // Right now, the requirement is, that an Uint8Array is still an - // Uint8Array as it arrives on the worker. (Chrome added this with v.15.) + // Uint8Array as it arrives on the worker. if ( - !PDFWorkerUtil.isWorkerDisabled && - !PDFWorker.#mainThreadWorkerMessageHandler + PDFWorkerUtil.isWorkerDisabled || + PDFWorker.#mainThreadWorkerMessageHandler ) { - let { workerSrc } = PDFWorker; + this._setupFakeWorker(); + return; + } + let { workerSrc } = PDFWorker; - try { - // Wraps workerSrc path into blob URL, if the former does not belong - // to the same origin. - if ( - typeof PDFJSDev !== "undefined" && - PDFJSDev.test("GENERIC") && - !PDFWorkerUtil.isSameOrigin(window.location.href, workerSrc) - ) { - workerSrc = PDFWorkerUtil.createCDNWrapper( - new URL(workerSrc, window.location).href - ); + try { + // Wraps workerSrc path into blob URL, if the former does not belong + // to the same origin. + if ( + typeof PDFJSDev !== "undefined" && + PDFJSDev.test("GENERIC") && + !PDFWorkerUtil.isSameOrigin(window.location.href, workerSrc) + ) { + workerSrc = PDFWorkerUtil.createCDNWrapper( + new URL(workerSrc, window.location).href + ); + } + + const worker = new Worker(workerSrc, { type: "module" }); + const messageHandler = new MessageHandler("main", "worker", worker); + const terminateEarly = () => { + worker.removeEventListener("error", onWorkerError); + messageHandler.destroy(); + worker.terminate(); + if (this.destroyed) { + this._readyCapability.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(); } + }; - const worker = new Worker(workerSrc, { type: "module" }); - const messageHandler = new MessageHandler("main", "worker", worker); - const terminateEarly = () => { - worker.removeEventListener("error", onWorkerError); + const onWorkerError = () => { + if (!this._webWorker) { + // Worker failed to initialize due to an error. Clean up and fall + // back to the fake worker. + terminateEarly(); + } + }; + worker.addEventListener("error", onWorkerError); + + messageHandler.on("test", data => { + worker.removeEventListener("error", onWorkerError); + if (this.destroyed) { + terminateEarly(); + return; // worker was destroyed + } + if (data) { + this._messageHandler = messageHandler; + this._port = worker; + this._webWorker = worker; + + this._readyCapability.resolve(); + // Send global setting, e.g. verbosity level. + messageHandler.send("configure", { + verbosity: this.verbosity, + }); + } else { + this._setupFakeWorker(); messageHandler.destroy(); worker.terminate(); - if (this.destroyed) { - this._readyCapability.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(); - } - }; + } + }); - const onWorkerError = () => { - if (!this._webWorker) { - // Worker failed to initialize due to an error. Clean up and fall - // back to the fake worker. - terminateEarly(); - } - }; - worker.addEventListener("error", onWorkerError); + messageHandler.on("ready", data => { + worker.removeEventListener("error", onWorkerError); + if (this.destroyed) { + terminateEarly(); + return; // worker was destroyed + } + try { + sendTest(); + } catch { + // We need fallback to a faked worker. + this._setupFakeWorker(); + } + }); - messageHandler.on("test", data => { - worker.removeEventListener("error", onWorkerError); - if (this.destroyed) { - terminateEarly(); - return; // worker was destroyed - } - if (data) { - this._messageHandler = messageHandler; - this._port = worker; - this._webWorker = worker; + const sendTest = () => { + const testObj = new Uint8Array(); + // Ensure that we can use `postMessage` transfers. + messageHandler.send("test", testObj, [testObj.buffer]); + }; - this._readyCapability.resolve(); - // Send global setting, e.g. verbosity level. - messageHandler.send("configure", { - verbosity: this.verbosity, - }); - } else { - this._setupFakeWorker(); - messageHandler.destroy(); - worker.terminate(); - } - }); - - messageHandler.on("ready", data => { - worker.removeEventListener("error", onWorkerError); - if (this.destroyed) { - terminateEarly(); - return; // worker was destroyed - } - try { - sendTest(); - } catch { - // We need fallback to a faked worker. - this._setupFakeWorker(); - } - }); - - const sendTest = () => { - const testObj = new Uint8Array(); - // Ensure that we can use `postMessage` transfers. - messageHandler.send("test", testObj, [testObj.buffer]); - }; - - // It might take time for the worker to initialize. We will try to send - // the "test" message immediately, and once the "ready" message arrives. - // The worker shall process only the first received "test" message. - sendTest(); - return; - } catch { - info("The worker has been disabled."); - } + // It might take time for the worker to initialize. We will try to send + // the "test" message immediately, and once the "ready" message arrives. + // The worker shall process only the first received "test" message. + sendTest(); + return; + } catch { + info("The worker has been disabled."); } - // Either workers are disabled, not supported or have thrown an exception. + // Either workers are not supported or have thrown an exception. // Thus, we fallback to a faked worker. this._setupFakeWorker(); } From 8d4456172b795d61ff4dcfbf8d7cbad87e78856f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 15 Jun 2024 13:25:51 +0200 Subject: [PATCH 3/5] Reduce duplication when handling the "test" message from the worker The feature-testing on the worker-thread has been simplified in previous pull requests, which means that we can simplify this main-thread handler as well. --- src/display/api.js | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index ef2511fdb..82dacf519 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2173,32 +2173,26 @@ class PDFWorker { messageHandler.on("test", data => { worker.removeEventListener("error", onWorkerError); - if (this.destroyed) { + if (this.destroyed || !data) { terminateEarly(); - return; // worker was destroyed + return; } - if (data) { - this._messageHandler = messageHandler; - this._port = worker; - this._webWorker = worker; + this._messageHandler = messageHandler; + this._port = worker; + this._webWorker = worker; - this._readyCapability.resolve(); - // Send global setting, e.g. verbosity level. - messageHandler.send("configure", { - verbosity: this.verbosity, - }); - } else { - this._setupFakeWorker(); - messageHandler.destroy(); - worker.terminate(); - } + this._readyCapability.resolve(); + // Send global setting, e.g. verbosity level. + messageHandler.send("configure", { + verbosity: this.verbosity, + }); }); messageHandler.on("ready", data => { worker.removeEventListener("error", onWorkerError); if (this.destroyed) { terminateEarly(); - return; // worker was destroyed + return; } try { sendTest(); From 2d0e08f1c86e8ced259bf175e7ee48580dfd843c Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 15 Jun 2024 13:37:04 +0200 Subject: [PATCH 4/5] Introduce a helper method for resolving the `PDFWorker` promise This avoids having to repeat the same code multiple times, since besides resolving the promise we also need to send the "configure" message to the worker-thread. --- src/display/api.js | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 82dacf519..46acad652 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2086,6 +2086,14 @@ class PDFWorker { return this._readyCapability.promise; } + #resolve() { + this._readyCapability.resolve(); + // Send global setting, e.g. verbosity level. + this._messageHandler.send("configure", { + verbosity: this.verbosity, + }); + } + /** * The current `workerPort`, when it exists. * @type {Worker} @@ -2112,11 +2120,7 @@ class PDFWorker { // Ignoring "ready" event -- MessageHandler should already be initialized // and ready to accept messages. }); - this._readyCapability.resolve(); - // Send global setting, e.g. verbosity level. - this._messageHandler.send("configure", { - verbosity: this.verbosity, - }); + this.#resolve(); } _initialize() { @@ -2181,11 +2185,7 @@ class PDFWorker { this._port = worker; this._webWorker = worker; - this._readyCapability.resolve(); - // Send global setting, e.g. verbosity level. - messageHandler.send("configure", { - verbosity: this.verbosity, - }); + this.#resolve(); }); messageHandler.on("ready", data => { @@ -2244,13 +2244,8 @@ class PDFWorker { const workerHandler = new MessageHandler(id + "_worker", id, port); WorkerMessageHandler.setup(workerHandler, port); - const messageHandler = new MessageHandler(id, id + "_worker", port); - this._messageHandler = messageHandler; - this._readyCapability.resolve(); - // Send global setting, e.g. verbosity level. - messageHandler.send("configure", { - verbosity: this.verbosity, - }); + this._messageHandler = new MessageHandler(id, id + "_worker", port); + this.#resolve(); }) .catch(reason => { this._readyCapability.reject( From f3f88eecb447bd33d1c0040954c649f3f6565b50 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 15 Jun 2024 13:48:41 +0200 Subject: [PATCH 5/5] Use an `AbortController` to remove the temporary "error" handler for the worker --- src/display/api.js | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 46acad652..3b1ca159c 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2154,7 +2154,7 @@ class PDFWorker { const worker = new Worker(workerSrc, { type: "module" }); const messageHandler = new MessageHandler("main", "worker", worker); const terminateEarly = () => { - worker.removeEventListener("error", onWorkerError); + ac.abort(); messageHandler.destroy(); worker.terminate(); if (this.destroyed) { @@ -2166,17 +2166,21 @@ class PDFWorker { } }; - const onWorkerError = () => { - if (!this._webWorker) { - // Worker failed to initialize due to an error. Clean up and fall - // back to the fake worker. - terminateEarly(); - } - }; - worker.addEventListener("error", onWorkerError); + const ac = new AbortController(); + worker.addEventListener( + "error", + () => { + if (!this._webWorker) { + // Worker failed to initialize due to an error. Clean up and fall + // back to the fake worker. + terminateEarly(); + } + }, + { signal: ac.signal } + ); messageHandler.on("test", data => { - worker.removeEventListener("error", onWorkerError); + ac.abort(); if (this.destroyed || !data) { terminateEarly(); return; @@ -2189,7 +2193,7 @@ class PDFWorker { }); messageHandler.on("ready", data => { - worker.removeEventListener("error", onWorkerError); + ac.abort(); if (this.destroyed) { terminateEarly(); return;