Merge pull request #12703 from Snuffleupagus/GenericScripting-rm-scriptElement

Ensure that the `pdf.sandbox.js` is removed from the DOM on destroy, and unbreak the Chromium-extension (PR 12695 follow-up)

src/display/display_utils.js

@@ -530,14 +530,20 @@ function isValidFetchUrl(url, baseUrl) {
 
 /**
  * @param {string} src
+ * @param {boolean} [removeScriptElement]
  * @returns {Promise<void>}
  */
-function loadScript(src) {
+function loadScript(src, removeScriptElement = false) {
   return new Promise((resolve, reject) => {
     const script = document.createElement("script");
     script.src = src;
 
-    script.onload = resolve;
+    script.onload = function (evt) {
+      if (removeScriptElement) {
+        script.remove();
+      }
+      resolve(evt);
+    };
     script.onerror = function () {
       reject(new Error(`Cannot load script at: ${script.src}`));
     };

web/app.js

@@ -168,6 +168,10 @@ class DefaultExternalServices {
     throw new Error("Not implemented: createL10n");
   }
 
+  static createScripting() {
+    throw new Error("Not implemented: createScripting");
+  }
+
   static get supportsIntegratedFind() {
     return shadow(this, "supportsIntegratedFind", false);
   }
@@ -186,10 +190,6 @@ class DefaultExternalServices {
   static get isInAutomation() {
     return shadow(this, "isInAutomation", false);
   }
-
-  static get scripting() {
-    throw new Error("Not implemented: scripting");
-  }
 }
 
 const PDFViewerApplication = {
@@ -763,6 +763,39 @@ const PDFViewerApplication = {
     document.title = title;
   },
 
+  /**
+   * @private
+   */
+  _cancelIdleCallbacks() {
+    if (!this._idleCallbacks.size) {
+      return;
+    }
+    for (const callback of this._idleCallbacks) {
+      window.cancelIdleCallback(callback);
+    }
+    this._idleCallbacks.clear();
+  },
+
+  /**
+   * @private
+   */
+  async _destroyScriptingInstance() {
+    if (!this._scriptingInstance) {
+      return;
+    }
+    const { scripting, events } = this._scriptingInstance;
+    try {
+      await scripting.destroySandbox();
+    } catch (ex) {}
+
+    for (const [name, listener] of events) {
+      window.removeEventListener(name, listener);
+    }
+    events.clear();
+
+    this._scriptingInstance = null;
+  },
+
   /**
    * Closes opened PDF document.
    * @returns {Promise} - Returns the promise, which is resolved when all
@@ -775,8 +808,9 @@ const PDFViewerApplication = {
     if (!this.pdfLoadingTask) {
       return undefined;
     }
+    const promises = [];
 
-    const promise = this.pdfLoadingTask.destroy();
+    promises.push(this.pdfLoadingTask.destroy());
     this.pdfLoadingTask = null;
 
     if (this.pdfDocument) {
@@ -799,22 +833,9 @@ const PDFViewerApplication = {
     this._contentLength = null;
     this.triggerDelayedFallback = null;
     this._saveInProgress = false;
-    for (const callback of this._idleCallbacks) {
-      window.cancelIdleCallback(callback);
-    }
-    this._idleCallbacks.clear();
 
-    if (this._scriptingInstance) {
+    this._cancelIdleCallbacks();
-      const { scripting, events } = this._scriptingInstance;
+    promises.push(this._destroyScriptingInstance());
-      try {
-        scripting.destroySandbox();
-      } catch (ex) {}
-
-      for (const [name, listener] of events) {
-        window.removeEventListener(name, listener);
-      }
-      this._scriptingInstance = null;
-    }
 
     this.pdfSidebar.reset();
     this.pdfOutlineViewer.reset();
@@ -833,7 +854,7 @@ const PDFViewerApplication = {
     if (typeof PDFBug !== "undefined") {
       PDFBug.cleanup();
     }
-    return promise;
+    return Promise.all(promises);
   },
 
   /**
@@ -1435,7 +1456,7 @@ const PDFViewerApplication = {
       // or the document was closed while the data resolved.
       return;
     }
-    const { scripting } = this.externalServices;
+    const scripting = this.externalServices.createScripting();
     // Store a reference to the current scripting-instance, to allow destruction
     // of the sandbox and removal of the event listeners at document closing.
     this._scriptingInstance = { scripting, events: new Map() };

web/app_options.js

@@ -265,6 +265,12 @@ if (
         : "../build/pdf.sandbox.js",
     kind: OptionKind.VIEWER,
   };
+} else if (PDFJSDev.test("CHROME")) {
+  defaultOptions.sandboxBundleSrc = {
+    /** @type {string} */
+    value: "../build/pdf.sandbox.js",
+    kind: OptionKind.VIEWER,
+  };
 }
 
 const userOptions = Object.create(null);

web/chromecom.js

@@ -19,6 +19,7 @@ import { AppOptions } from "./app_options.js";
 import { BasePreferences } from "./preferences.js";
 import { DownloadManager } from "./download_manager.js";
 import { GenericL10n } from "./genericl10n.js";
+import { GenericScripting } from "./generic_scripting.js";
 
 if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("CHROME")) {
   throw new Error(
@@ -428,6 +429,10 @@ class ChromeExternalServices extends DefaultExternalServices {
   static createL10n(options) {
     return new GenericL10n(navigator.language);
   }
+
+  static createScripting() {
+    return new GenericScripting();
+  }
 }
 PDFViewerApplication.externalServices = ChromeExternalServices;
 

web/firefoxcom.js

@@ -360,7 +360,7 @@ class FirefoxExternalServices extends DefaultExternalServices {
     return new MozL10n(mozL10n);
   }
 
-  static get scripting() {
+  static createScripting() {
     return FirefoxScripting;
   }
 

web/generic_scripting.js

@@ -0,0 +1,45 @@
+/* Copyright 2020 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { AppOptions } from "./app_options.js";
+import { loadScript } from "pdfjs-lib";
+
+class GenericScripting {
+  constructor() {
+    this._ready = loadScript(
+      AppOptions.get("sandboxBundleSrc"),
+      /* removeScriptElement = */ true
+    ).then(() => {
+      return window.pdfjsSandbox.QuickJSSandbox();
+    });
+  }
+
+  async createSandbox(data) {
+    const sandbox = await this._ready;
+    sandbox.create(data);
+  }
+
+  async dispatchEventInSandbox(event) {
+    const sandbox = await this._ready;
+    sandbox.dispatchEvent(event);
+  }
+
+  async destroySandbox() {
+    const sandbox = await this._ready;
+    sandbox.nukeSandbox();
+  }
+}
+
+export { GenericScripting };

web/genericcom.js

@@ -14,11 +14,10 @@
  */
 
 import { DefaultExternalServices, PDFViewerApplication } from "./app.js";
-import { AppOptions } from "./app_options.js";
 import { BasePreferences } from "./preferences.js";
 import { DownloadManager } from "./download_manager.js";
 import { GenericL10n } from "./genericl10n.js";
-import { loadScript } from "pdfjs-lib";
+import { GenericScripting } from "./generic_scripting.js";
 
 if (typeof PDFJSDev !== "undefined" && !PDFJSDev.test("GENERIC")) {
   throw new Error(
@@ -39,29 +38,6 @@ class GenericPreferences extends BasePreferences {
   }
 }
 
-class GenericScripting {
-  constructor() {
-    this._ready = loadScript(AppOptions.get("sandboxBundleSrc")).then(() => {
-      return window.pdfjsSandbox.QuickJSSandbox();
-    });
-  }
-
-  async createSandbox(data) {
-    const sandbox = await this._ready;
-    sandbox.create(data);
-  }
-
-  async dispatchEventInSandbox(event) {
-    const sandbox = await this._ready;
-    sandbox.dispatchEvent(event);
-  }
-
-  async destroySandbox() {
-    const sandbox = await this._ready;
-    sandbox.nukeSandbox();
-  }
-}
-
 class GenericExternalServices extends DefaultExternalServices {
   static createDownloadManager(options) {
     return new DownloadManager();
@@ -75,7 +51,7 @@ class GenericExternalServices extends DefaultExternalServices {
     return new GenericL10n(locale);
   }
 
-  static get scripting() {
+  static createScripting() {
     return new GenericScripting();
   }
 }