From 447915af9da0c2eae8764fb2f5bb4bb4a396a259 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 9 Oct 2022 11:48:50 +0200 Subject: [PATCH 1/3] Stop sending the unused `source.url` property in "GetDocRequest" It seems that this property became *effectively* unused already in PR 8617, however we missed removing it as part of the clean-up in PR 10376. --- src/display/api.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/display/api.js b/src/display/api.js index b7914fe10..a4e278a5b 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -512,7 +512,6 @@ async function _fetchDocument(worker, source, pdfDataRangeTransport, docId) { // Only send the required properties, and *not* the entire object. source: { data: source.data, - url: source.url, password: source.password, disableAutoFetch: source.disableAutoFetch, rangeChunkSize: source.rangeChunkSize, From c84b717773a4280b2a9ff228560a6959638e5d6b Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 9 Oct 2022 11:15:09 +0200 Subject: [PATCH 2/3] Group the `evaluatorOptions` on the main-thread, when sending "GetDocRequest" Rather than sending all of these parameters individually and then grouping them together on the worker-thread, we can simply handle that in the API instead. --- src/core/worker.js | 14 +------------- src/display/api.js | 24 +++++++++++++----------- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/core/worker.js b/src/core/worker.js index 0749e1541..caba9149b 100644 --- a/src/core/worker.js +++ b/src/core/worker.js @@ -405,19 +405,7 @@ class WorkerMessageHandler { ensureNotTerminated(); - const evaluatorOptions = { - maxImageSize: data.maxImageSize, - disableFontFace: data.disableFontFace, - ignoreErrors: data.ignoreErrors, - isEvalSupported: data.isEvalSupported, - isOffscreenCanvasSupported: data.isOffscreenCanvasSupported, - fontExtraProperties: data.fontExtraProperties, - useSystemFonts: data.useSystemFonts, - cMapUrl: data.cMapUrl, - standardFontDataUrl: data.standardFontDataUrl, - }; - - getPdfManager(data, evaluatorOptions, data.enableXfa) + getPdfManager(data, data.evaluatorOptions, data.enableXfa) .then(function (newPdfManager) { if (terminated) { // We were in a process of setting up the manager, but it got diff --git a/src/display/api.js b/src/display/api.js index a4e278a5b..85b61d4b2 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -517,19 +517,21 @@ async function _fetchDocument(worker, source, pdfDataRangeTransport, docId) { rangeChunkSize: source.rangeChunkSize, length: source.length, }, - maxImageSize: source.maxImageSize, - disableFontFace: source.disableFontFace, docBaseUrl: source.docBaseUrl, - ignoreErrors: source.ignoreErrors, - isEvalSupported: source.isEvalSupported, - isOffscreenCanvasSupported: source.isOffscreenCanvasSupported, - fontExtraProperties: source.fontExtraProperties, enableXfa: source.enableXfa, - useSystemFonts: source.useSystemFonts, - cMapUrl: source.useWorkerFetch ? source.cMapUrl : null, - standardFontDataUrl: source.useWorkerFetch - ? source.standardFontDataUrl - : null, + evaluatorOptions: { + maxImageSize: source.maxImageSize, + disableFontFace: source.disableFontFace, + ignoreErrors: source.ignoreErrors, + isEvalSupported: source.isEvalSupported, + isOffscreenCanvasSupported: source.isOffscreenCanvasSupported, + fontExtraProperties: source.fontExtraProperties, + useSystemFonts: source.useSystemFonts, + cMapUrl: source.useWorkerFetch ? source.cMapUrl : null, + standardFontDataUrl: source.useWorkerFetch + ? source.standardFontDataUrl + : null, + }, } ); From 8a4f6aca979fc29af4c53ea5fd2ffd85d2cc505c Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 9 Oct 2022 11:30:24 +0200 Subject: [PATCH 3/3] Stop using the `source`-object when sending "GetDocRequest" Looking at the code on the worker-thread, there doesn't appear to be any particular reason for placing *some* of the properties in a `source`-object when sending them with "GetDocRequest". As is often the case the explanation for this structure is rather "for historical reasons", since originally we simply sent the `source`-object as-is. Doing that was obviously a bad idea, for a couple of reasons: - It makes it less clear what is/isn't actually needed on the worker-thread. - Sending unused properties will unnecessarily increase memory usage. - The `source`-object may contain unclonable data, which would break the library. --- src/core/worker.js | 41 +++++++++++++++++++++++------------------ src/display/api.js | 14 ++++++-------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/core/worker.js b/src/core/worker.js index caba9149b..8673f92d4 100644 --- a/src/core/worker.js +++ b/src/core/worker.js @@ -97,7 +97,7 @@ class WorkerMessageHandler { const WorkerTasks = []; const verbosity = getVerbosityLevel(); - const apiVersion = docParams.apiVersion; + const { docId, apiVersion } = docParams; const workerVersion = typeof PDFJSDev !== "undefined" && !PDFJSDev.test("TESTING") ? PDFJSDev.eval("BUNDLE_VERSION") @@ -142,10 +142,7 @@ class WorkerMessageHandler { throw new Error(partialMsg + "please update to a supported browser."); } } - - const docId = docParams.docId; - const docBaseUrl = docParams.docBaseUrl; - const workerHandlerName = docParams.docId + "_worker"; + const workerHandlerName = docId + "_worker"; let handler = new MessageHandler(workerHandlerName, docId, port); function ensureNotTerminated() { @@ -204,17 +201,25 @@ class WorkerMessageHandler { return { numPages, fingerprints, htmlForXfa }; } - function getPdfManager(data, evaluatorOptions, enableXfa) { + function getPdfManager({ + data, + password, + disableAutoFetch, + rangeChunkSize, + length, + docBaseUrl, + enableXfa, + evaluatorOptions, + }) { const pdfManagerCapability = createPromiseCapability(); let newPdfManager; - const source = data.source; - if (source.data) { + if (data) { try { newPdfManager = new LocalPdfManager( docId, - source.data, - source.password, + data, + password, handler, evaluatorOptions, enableXfa, @@ -242,19 +247,19 @@ class WorkerMessageHandler { if (!fullRequest.isRangeSupported) { return; } - // We don't need auto-fetch when streaming is enabled. - const disableAutoFetch = - source.disableAutoFetch || fullRequest.isStreamingSupported; + disableAutoFetch = + disableAutoFetch || fullRequest.isStreamingSupported; + newPdfManager = new NetworkPdfManager( docId, pdfStream, { msgHandler: handler, - password: source.password, + password, length: fullRequest.contentLength, disableAutoFetch, - rangeChunkSize: source.rangeChunkSize, + rangeChunkSize, }, evaluatorOptions, enableXfa, @@ -279,7 +284,7 @@ class WorkerMessageHandler { let loaded = 0; const flushChunks = function () { const pdfFile = arraysToBytes(cachedChunks); - if (source.length && pdfFile.length !== source.length) { + if (length && pdfFile.length !== length) { warn("reported HTTP length is different from actual"); } // the data is array, instantiating directly from it @@ -287,7 +292,7 @@ class WorkerMessageHandler { newPdfManager = new LocalPdfManager( docId, pdfFile, - source.password, + password, handler, evaluatorOptions, enableXfa, @@ -405,7 +410,7 @@ class WorkerMessageHandler { ensureNotTerminated(); - getPdfManager(data, data.evaluatorOptions, data.enableXfa) + getPdfManager(data) .then(function (newPdfManager) { if (terminated) { // We were in a process of setting up the manager, but it got diff --git a/src/display/api.js b/src/display/api.js index 85b61d4b2..0fff1a79c 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -503,20 +503,18 @@ async function _fetchDocument(worker, source, pdfDataRangeTransport, docId) { } const workerId = await worker.messageHandler.sendWithPromise( "GetDocRequest", + // Only send the required properties, and *not* the entire `source` object. { docId, apiVersion: typeof PDFJSDev !== "undefined" && !PDFJSDev.test("TESTING") ? PDFJSDev.eval("BUNDLE_VERSION") : null, - // Only send the required properties, and *not* the entire object. - source: { - data: source.data, - password: source.password, - disableAutoFetch: source.disableAutoFetch, - rangeChunkSize: source.rangeChunkSize, - length: source.length, - }, + data: source.data, + password: source.password, + disableAutoFetch: source.disableAutoFetch, + rangeChunkSize: source.rangeChunkSize, + length: source.length, docBaseUrl: source.docBaseUrl, enableXfa: source.enableXfa, evaluatorOptions: {