Merge pull request #13172 from Snuffleupagus/cleanup-keepFonts

[api-minor] Add an option, in `PDFDocumentProxy.cleanup`, to allow fonts to remain attached to the DOM

Author: timvandermeij

src/display/api.js

@@ -921,10 +921,13 @@ class PDFDocumentProxy {
    * NOTE: Do not, under any circumstances, call this method when rendering is
    * currently ongoing since that may lead to rendering errors.
    *
+   * @param {boolean} [keepLoadedFonts] - Let fonts remain attached to the DOM.
+   *   NOTE: This will increase persistent memory usage, hence don't use this
+   *   option unless absolutely necessary. The default value is `false`.
    * @returns {Promise} A promise that is resolved when clean-up has finished.
    */
-  cleanup() {
-    return this._transport.startCleanup();
+  cleanup(keepLoadedFonts = false) {
+    return this._transport.startCleanup(keepLoadedFonts || this.isPureXfa);
   }
 
   /**
@@ -1180,14 +1183,14 @@ class PDFPageProxy {
    *   {Array} of the annotation objects.
    */
   getAnnotations({ intent = null } = {}) {
-    if (!this.annotationsPromise || this.annotationsIntent !== intent) {
-      this.annotationsPromise = this._transport.getAnnotations(
+    if (!this._annotationsPromise || this._annotationsIntent !== intent) {
+      this._annotationsPromise = this._transport.getAnnotations(
         this._pageIndex,
         intent
       );
-      this.annotationsIntent = intent;
+      this._annotationsIntent = intent;
     }
-    return this.annotationsPromise;
+    return this._annotationsPromise;
   }
 
   /**
@@ -1480,7 +1483,7 @@ class PDFPageProxy {
       }
     }
     this.objs.clear();
-    this.annotationsPromise = null;
+    this._annotationsPromise = null;
     this._jsActionsPromise = null;
     this._xfaPromise = null;
     this.pendingCleanup = false;
@@ -1515,7 +1518,7 @@ class PDFPageProxy {
 
     this._intentStates.clear();
     this.objs.clear();
-    this.annotationsPromise = null;
+    this._annotationsPromise = null;
     this._jsActionsPromise = null;
     this._xfaPromise = null;
     if (resetStats && this._stats) {
@@ -2789,24 +2792,28 @@ class WorkerTransport {
     return this.messageHandler.sendWithPromise("GetStats", null);
   }
 
-  startCleanup() {
-    return this.messageHandler.sendWithPromise("Cleanup", null).then(() => {
-      for (let i = 0, ii = this.pageCache.length; i < ii; i++) {
-        const page = this.pageCache[i];
-        if (page) {
-          const cleanupSuccessful = page.cleanup();
+  async startCleanup(keepLoadedFonts = false) {
+    await this.messageHandler.sendWithPromise("Cleanup", null);
 
-          if (!cleanupSuccessful) {
-            throw new Error(
-              `startCleanup: Page ${i + 1} is currently rendering.`
-            );
-          }
-        }
+    if (this.destroyed) {
+      return; // No need to manually clean-up when destruction has started.
+    }
+    for (let i = 0, ii = this.pageCache.length; i < ii; i++) {
+      const page = this.pageCache[i];
+      if (!page) {
+        continue;
       }
-      this.commonObjs.clear();
+      const cleanupSuccessful = page.cleanup();
+
+      if (!cleanupSuccessful) {
+        throw new Error(`startCleanup: Page ${i + 1} is currently rendering.`);
+      }
+    }
+    this.commonObjs.clear();
+    if (!keepLoadedFonts) {
       this.fontLoader.clear();
-      this._hasJSActionsPromise = null;
-    });
+    }
+    this._hasJSActionsPromise = null;
   }
 
   get loadingParams() {

test/unit/api_spec.js

@@ -1339,12 +1339,10 @@ describe("api", function () {
         .catch(done.fail);
     });
 
-    it("cleans up document resources", function (done) {
-      const promise = pdfDocument.cleanup();
-      promise.then(function () {
-        expect(true).toEqual(true);
-        done();
-      }, done.fail);
+    it("cleans up document resources", async function () {
+      await pdfDocument.cleanup();
+
+      expect(true).toEqual(true);
     });
 
     it("checks that fingerprints are unique", function (done) {
@@ -1982,85 +1980,69 @@ describe("api", function () {
       ]).then(done);
     });
 
-    it("cleans up document resources after rendering of page", function (done) {
+    it("cleans up document resources after rendering of page", async function () {
       const loadingTask = getDocument(buildGetDocumentParams(basicApiFileName));
-      let canvasAndCtx;
+      const pdfDoc = await loadingTask.promise;
+      const pdfPage = await pdfDoc.getPage(1);
 
-      loadingTask.promise
-        .then(pdfDoc => {
-          return pdfDoc.getPage(1).then(pdfPage => {
-            const viewport = pdfPage.getViewport({ scale: 1 });
-            canvasAndCtx = CanvasFactory.create(
-              viewport.width,
-              viewport.height
-            );
+      const viewport = pdfPage.getViewport({ scale: 1 });
+      const canvasAndCtx = CanvasFactory.create(
+        viewport.width,
+        viewport.height
+      );
 
-            const renderTask = pdfPage.render({
-              canvasContext: canvasAndCtx.context,
-              canvasFactory: CanvasFactory,
-              viewport,
-            });
-            return renderTask.promise.then(() => {
-              return pdfDoc.cleanup();
-            });
-          });
-        })
-        .then(() => {
-          expect(true).toEqual(true);
+      const renderTask = pdfPage.render({
+        canvasContext: canvasAndCtx.context,
+        canvasFactory: CanvasFactory,
+        viewport,
+      });
+      await renderTask.promise;
 
-          CanvasFactory.destroy(canvasAndCtx);
-          loadingTask.destroy().then(done);
-        }, done.fail);
+      await pdfDoc.cleanup();
+
+      expect(true).toEqual(true);
+
+      CanvasFactory.destroy(canvasAndCtx);
+      await loadingTask.destroy();
     });
 
-    it("cleans up document resources during rendering of page", function (done) {
+    it("cleans up document resources during rendering of page", async function () {
       const loadingTask = getDocument(
         buildGetDocumentParams("tracemonkey.pdf")
       );
-      let canvasAndCtx;
+      const pdfDoc = await loadingTask.promise;
+      const pdfPage = await pdfDoc.getPage(1);
 
-      loadingTask.promise
-        .then(pdfDoc => {
-          return pdfDoc.getPage(1).then(pdfPage => {
-            const viewport = pdfPage.getViewport({ scale: 1 });
-            canvasAndCtx = CanvasFactory.create(
-              viewport.width,
-              viewport.height
-            );
+      const viewport = pdfPage.getViewport({ scale: 1 });
+      const canvasAndCtx = CanvasFactory.create(
+        viewport.width,
+        viewport.height
+      );
 
-            const renderTask = pdfPage.render({
-              canvasContext: canvasAndCtx.context,
-              canvasFactory: CanvasFactory,
-              viewport,
-            });
+      const renderTask = pdfPage.render({
+        canvasContext: canvasAndCtx.context,
+        canvasFactory: CanvasFactory,
+        viewport,
+      });
+      // Ensure that clean-up runs during rendering.
+      renderTask.onContinue = function (cont) {
+        waitSome(cont);
+      };
 
-            renderTask.onContinue = function (cont) {
-              waitSome(cont);
-            };
+      try {
+        await pdfDoc.cleanup();
 
-            return pdfDoc
-              .cleanup()
-              .then(
-                () => {
-                  throw new Error("shall fail cleanup");
-                },
-                reason => {
-                  expect(reason instanceof Error).toEqual(true);
-                  expect(reason.message).toEqual(
-                    "startCleanup: Page 1 is currently rendering."
-                  );
-                }
-              )
-              .then(() => {
-                return renderTask.promise;
-              })
-              .then(() => {
-                CanvasFactory.destroy(canvasAndCtx);
-                loadingTask.destroy().then(done);
-              });
-          });
-        })
-        .catch(done.fail);
+        throw new Error("shall fail cleanup");
+      } catch (reason) {
+        expect(reason instanceof Error).toEqual(true);
+        expect(reason.message).toEqual(
+          "startCleanup: Page 1 is currently rendering."
+        );
+      }
+      await renderTask.promise;
+
+      CanvasFactory.destroy(canvasAndCtx);
+      await loadingTask.destroy();
     });
 
     it("caches image resources at the document/page level as expected (issue 11878)", async function () {

web/app.js

@@ -465,7 +465,7 @@ const PDFViewerApplication = {
     this.overlayManager = new OverlayManager();
 
     const pdfRenderingQueue = new PDFRenderingQueue();
-    pdfRenderingQueue.onIdle = this.cleanup.bind(this);
+    pdfRenderingQueue.onIdle = this._cleanup.bind(this);
     this.pdfRenderingQueue = pdfRenderingQueue;
 
     const pdfLinkService = new PDFLinkService({
@@ -1782,17 +1782,20 @@ const PDFViewerApplication = {
     }
   },
 
-  cleanup() {
+  /**
+   * @private
+   */
+  _cleanup() {
     if (!this.pdfDocument) {
       return; // run cleanup when document is loaded
     }
     this.pdfViewer.cleanup();
     this.pdfThumbnailViewer.cleanup();
 
     // We don't want to remove fonts used by active page SVGs.
-    if (this.pdfViewer.renderer !== RendererType.SVG) {
-      this.pdfDocument.cleanup();
-    }
+    this.pdfDocument.cleanup(
+      /* keepLoadedFonts = */ this.pdfViewer.renderer === RendererType.SVG
+    );
   },
 
   forceRendering() {