From 52a598915f780b503efe0e4e35aba85edf02b6aa Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Wed, 10 Mar 2021 12:31:45 +0100
Subject: [PATCH] Re-factor the `PDFScriptingManager._destroyScripting` method
 (PR 13042 follow-up)

*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).
---
 web/base_viewer.js           |  5 +----
 web/pdf_scripting_manager.js | 29 ++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/web/base_viewer.js b/web/base_viewer.js
index bd8816740..c6d5cb557 100644
--- a/web/base_viewer.js
+++ b/web/base_viewer.js
@@ -470,10 +470,7 @@ class BaseViewer {
         this.findController.setDocument(null);
       }
       if (this._scriptingManager) {
-        // Defer this slightly, to allow the "PageClose" event to be handled.
-        Promise.resolve().then(() => {
-          this._scriptingManager.setDocument(null);
-        });
+        this._scriptingManager.setDocument(null);
       }
     }
 
diff --git a/web/pdf_scripting_manager.js b/web/pdf_scripting_manager.js
index ac5b45935..32c9d0e2b 100644
--- a/web/pdf_scripting_manager.js
+++ b/web/pdf_scripting_manager.js
@@ -31,7 +31,7 @@ import { RenderingStates } from "./pdf_rendering_queue.js";
 
 class PDFScriptingManager {
   /**
-   * @param {PDFScriptingManager} options
+   * @param {PDFScriptingManagerOptions} options
    */
   constructor({
     eventBus,
@@ -41,6 +41,7 @@ class PDFScriptingManager {
   }) {
     this._pdfDocument = null;
     this._pdfViewer = null;
+    this._closeCapability = null;
     this._destroyCapability = null;
 
     this._scripting = null;
@@ -124,8 +125,10 @@ class PDFScriptingManager {
       }
       this._dispatchPageOpen(pageNumber);
     });
-    this._internalEvents.set("pagesdestroy", event => {
-      this._dispatchPageClose(this._pdfViewer.currentPageNumber);
+    this._internalEvents.set("pagesdestroy", async event => {
+      await this._dispatchPageClose(this._pdfViewer.currentPageNumber);
+
+      this._closeCapability?.resolve();
     });
 
     this._domEvents.set("mousedown", event => {
@@ -307,6 +310,8 @@ class PDFScriptingManager {
       visitedPages = this._visitedPages;
 
     if (initialize) {
+      this._closeCapability = createPromiseCapability();
+
       this._pageEventsReady = true;
     }
     if (!this._pageEventsReady) {
@@ -417,12 +422,26 @@ class PDFScriptingManager {
    * @private
    */
   async _destroyScripting() {
-    this._pdfDocument = null; // Ensure that it's *always* reset synchronously.
-
     if (!this._scripting) {
+      this._pdfDocument = null;
+
       this._destroyCapability?.resolve();
       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 {
       await this._scripting.destroySandbox();
     } catch (ex) {}