From ddebb0f954b0fea89136e706d23955123da71870 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 26 Nov 2023 12:18:08 +0100 Subject: [PATCH 1/3] [Firefox] Don't send the "initPassiveLoading" message synchronously The return value is not, nor has it ever been, used for anything and we should thus be able to just send the message. Note that the responses are already handled by the "message" event listener registered above. --- web/firefoxcom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 12687a231..ba3790e10 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -403,7 +403,7 @@ class FirefoxExternalServices extends DefaultExternalServices { break; } }); - FirefoxCom.requestSync("initPassiveLoading", null); + FirefoxCom.request("initPassiveLoading", null); } static reportTelemetry(data) { From 9ca504e538a04cd989c9d6c5828479d636e530e4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 26 Nov 2023 12:18:17 +0100 Subject: [PATCH 2/3] [Firefox] Don't send the "abortLoading" message synchronously Despite the comment, I believe that changing this should be fine for two separate reasons: - The platform code has an "unload" event listener, see [this code](https://searchfox.org/mozilla-central/rev/edb2612db13e89f1c44ab95b1e4d4366c16eb9fb/toolkit/components/pdfjs/content/PdfStreamConverter.sys.mjs#533-538), that invokes the same method. Hence we should still be guaranteed that the relevant platform method will run. - The `FirefoxComDataRangeTransport.abort` method is never actually invoked in the Firefox PDF Viewer. Note that the [`PDFDataRangeTransport.abort` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/src/display/api.js#L759) is only invoked via the [`PDFDataTransportStream.cancelAllRequests` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/src/display/transport_stream.js#L167-L175), which in turn is only invoked via the [`WorkerTransport.destroy` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/src/display/api.js#L2485-L2487). That method is invoked via the [`PDFDocumentLoadingTask.destroy` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/src/display/api.js#L630), which in the viewer is only invoked via the [`PDFViewerApplication.close` method](https://github.com/mozilla/pdf.js/blob/f4b396f6c88e951174879a52390fa89223326b36/web/app.js#L919) which is never actually called in the Firefox PDF Viewer. All-in-all, given the existing platform code *and* the current viewer-implementation it should thus be safe to not wait for the "abortLoading" message to complete. --- web/firefoxcom.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index ba3790e10..b31ea75cc 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -314,9 +314,9 @@ class FirefoxComDataRangeTransport extends PDFDataRangeTransport { FirefoxCom.request("requestDataRange", { begin, end }); } + // NOTE: This method is currently not invoked in the Firefox PDF Viewer. abort() { - // Sync call to ensure abort is really started. - FirefoxCom.requestSync("abortLoading", null); + FirefoxCom.request("abortLoading", null); } } From b03ce966051a85a5f0f2ceb267667e65beab5ee0 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 26 Nov 2023 12:18:23 +0100 Subject: [PATCH 3/3] [Firefox] Remove the `FirefoxCom.requestSync` method, since it's unused After the two previous commits, which removed the remaining call-sites, this method is no longer used and can thus be removed. As mentioned in the JSDocs for the now removed method, synchronous communication between the viewer and the platform code isn't really a good idea. Once this patch has landed in mozilla-central some additional clean-up of the platform code will also be possible. --- web/firefoxcom.js | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/web/firefoxcom.js b/web/firefoxcom.js index b31ea75cc..a411d2723 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -26,35 +26,6 @@ if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) { } class FirefoxCom { - /** - * Creates an event that the extension is listening for and will - * synchronously respond to. - * NOTE: It is recommended to use requestAsync() instead since one day we may - * not be able to synchronously reply. - * @param {string} action - The action to trigger. - * @param {Object|string} [data] - The data to send. - * @returns {*} The response. - */ - static requestSync(action, data) { - const request = document.createTextNode(""); - document.documentElement.append(request); - - const sender = new CustomEvent("pdf.js.message", { - bubbles: true, - cancelable: false, - detail: { - action, - data, - sync: true, - }, - }); - request.dispatchEvent(sender); - const response = sender.detail.response; - request.remove(); - - return response; - } - /** * Creates an event that the extension is listening for and will * asynchronously respond to. @@ -96,7 +67,6 @@ class FirefoxCom { detail: { action, data, - sync: false, responseExpected: !!callback, }, });