Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Re-factor the PDFScriptingManager._destroyScripting method (PR 13042 follow-up)

Browse Source 
*Please note:* Given the pre-existing issues raised in PR 13056, which seem to block immediate progress there, this patch extracts some *overall* improvements of the scripting/sandbox destruction in `PDFScriptingManager`.

As can be seen in `BaseViewer.setDocument`, it's currently necessary to *manually* delay the `PDFScriptingManager`-destruction in order for things to work correctly. This is, in hindsight, obviously an *extremely poor* design choice on my part; sorry about the churn here!

In order to improve things overall, the `PDFScriptingManager._destroyScripting`-method is re-factored to wait for the relevant events to be dispatched *before* sandbox-destruction occurs.
To avoid the scripting/sandbox-destruction hanging indefinitely, we utilize a timeout to force-destroy the sandbox after a short time (currently set to 1 second).
This commit is contained in:
Jonas Jenwald 2021-03-10 12:31:45 +01:00
parent 5e3af62d58
commit 52a598915f
2 changed files with 25 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
web/base_viewer.js
View File

@ -470,10 +470,7 @@ class BaseViewer {
this.findController.setDocument(null); this.findController.setDocument(null);
} }
if (this._scriptingManager) { if (this._scriptingManager) {
// Defer this slightly, to allow the "PageClose" event to be handled. this._scriptingManager.setDocument(null);
Promise.resolve().then(() => {
this._scriptingManager.setDocument(null);
});
} }
} }

29
web/pdf_scripting_manager.js
View File

@ -31,7 +31,7 @@ import { RenderingStates } from "./pdf_rendering_queue.js";
class PDFScriptingManager { class PDFScriptingManager {
/** /**
* @param {PDFScriptingManager} options * @param {PDFScriptingManagerOptions} options
*/ */
constructor({ constructor({
eventBus, eventBus,
@ -41,6 +41,7 @@ class PDFScriptingManager {
}) { }) {
this._pdfDocument = null; this._pdfDocument = null;
this._pdfViewer = null; this._pdfViewer = null;
this._closeCapability = null;
this._destroyCapability = null; this._destroyCapability = null;
this._scripting = null; this._scripting = null;
@ -124,8 +125,10 @@ class PDFScriptingManager {
} }
this._dispatchPageOpen(pageNumber); this._dispatchPageOpen(pageNumber);
}); });
this._internalEvents.set("pagesdestroy", event => { this._internalEvents.set("pagesdestroy", async event => {
this._dispatchPageClose(this._pdfViewer.currentPageNumber); await this._dispatchPageClose(this._pdfViewer.currentPageNumber);
this._closeCapability?.resolve();
}); });
this._domEvents.set("mousedown", event => { this._domEvents.set("mousedown", event => {
@ -307,6 +310,8 @@ class PDFScriptingManager {
visitedPages = this._visitedPages; visitedPages = this._visitedPages;
if (initialize) { if (initialize) {
this._closeCapability = createPromiseCapability();
this._pageEventsReady = true; this._pageEventsReady = true;
} }
if (!this._pageEventsReady) { if (!this._pageEventsReady) {
@ -417,12 +422,26 @@ class PDFScriptingManager {
* @private * @private
*/ */
async _destroyScripting() { async _destroyScripting() {
this._pdfDocument = null; // Ensure that it's *always* reset synchronously.
if (!this._scripting) { if (!this._scripting) {
this._pdfDocument = null;
this._destroyCapability?.resolve(); this._destroyCapability?.resolve();
return; return;
} }
if (this._closeCapability) {
await Promise.race([
this._closeCapability.promise,
new Promise(resolve => {
// Avoid the scripting/sandbox-destruction hanging indefinitely.
setTimeout(resolve, 1000);
}),
]).catch(reason => {
// Ignore any errors, to ensure that the sandbox is always destroyed.
});
this._closeCapability = null;
}
this._pdfDocument = null;
try { try {
await this._scripting.destroySandbox(); await this._scripting.destroySandbox();
} catch (ex) {} } catch (ex) {}