From 6dc5dd194f26c64ba14b1252055be808e8c4a59b Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 17 Jun 2021 18:01:10 +0200 Subject: [PATCH 1/2] Remove the internal `PDFScriptingManager._pageEventsReady` boolean (PR 13074 follow-up) With the introduction of `PDFScriptingManager._closeCapability` in PR 13074, the pre-existing `PDFScriptingManager._pageEventsReady` boolean essentially became redundant. Given that you always want to avoid tracking closely related state *separately*, since it's easy to introduce subtle bugs that way, we should just remove `PDFScriptingManager._pageEventsReady` now. Obviously I *should* have done this already back in PR 13074, sorry about the churn here! --- web/pdf_scripting_manager.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/web/pdf_scripting_manager.js b/web/pdf_scripting_manager.js index aff5926a0..c828e5ef0 100644 --- a/web/pdf_scripting_manager.js +++ b/web/pdf_scripting_manager.js @@ -46,7 +46,6 @@ class PDFScriptingManager { this._scripting = null; this._mouseState = Object.create(null); - this._pageEventsReady = false; this._ready = false; this._eventBus = eventBus; @@ -308,7 +307,6 @@ class PDFScriptingManager { return; } } - delete detail.id; delete detail.siblings; @@ -333,10 +331,8 @@ class PDFScriptingManager { if (initialize) { this._closeCapability = createPromiseCapability(); - - this._pageEventsReady = true; } - if (!this._pageEventsReady) { + if (!this._closeCapability) { return; // Scripting isn't fully initialized yet. } const pageView = this._pdfViewer.getPageView(/* index = */ pageNumber - 1); @@ -373,7 +369,7 @@ class PDFScriptingManager { const pdfDocument = this._pdfDocument, visitedPages = this._visitedPages; - if (!this._pageEventsReady) { + if (!this._closeCapability) { return; // Scripting isn't fully initialized yet. } if (this._pageOpenPending.has(pageNumber)) { @@ -481,7 +477,6 @@ class PDFScriptingManager { this._scripting = null; delete this._mouseState.isDown; - this._pageEventsReady = false; this._ready = false; this._destroyCapability?.resolve(); From 5db7a3cc886bea3cad1186516e72e679abecbe9d Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 17 Jun 2021 18:42:15 +0200 Subject: [PATCH 2/2] Ensure that `PDFScriptingManager.setDocument` handles failure when initializing the scripting-factory This way, we'll immediately clean-up in exactly the same way as the other failure code-paths in the `PDFScriptingManager.setDocument` method. --- web/pdf_scripting_manager.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/web/pdf_scripting_manager.js b/web/pdf_scripting_manager.js index c828e5ef0..85ac55cd4 100644 --- a/web/pdf_scripting_manager.js +++ b/web/pdf_scripting_manager.js @@ -96,7 +96,14 @@ class PDFScriptingManager { if (pdfDocument !== this._pdfDocument) { return; // The document was closed while the data resolved. } - this._scripting = this._createScripting(); + try { + this._scripting = this._createScripting(); + } catch (error) { + console.error(`PDFScriptingManager.setDocument: "${error?.message}".`); + + await this._destroyScripting(); + return; + } this._internalEvents.set("updatefromsandbox", event => { if (event?.source !== window) {