Merge pull request #11573 from Snuffleupagus/api-cleanup-returns

[api-minor] Change `PDFDocumentProxy.cleanup`/`PDFPageProxy.cleanup` to return data

src/display/api.js

@@ -758,10 +758,16 @@ class PDFDocumentProxy {
   }
 
   /**
-   * Cleans up resources allocated by the document, e.g. created `@font-face`.
+   * Cleans up resources allocated by the document, on both the main- and
+   * worker-threads.
+   *
+   * NOTE: Do not, under any circumstances, call this method when rendering is
+   *       currently ongoing since that may lead to rendering errors.
+   *
+   * @returns {Promise} A promise that is resolved when clean-up has finished.
    */
   cleanup() {
-    this._transport.startCleanup();
+    return this._transport.startCleanup();
   }
 
   /**
@@ -1269,10 +1275,11 @@ class PDFPageProxy {
    * Cleans up resources allocated by the page.
    * @param {boolean} [resetStats] - Reset page stats, if enabled.
    *   The default value is `false`.
+   * @returns {boolean} Indicating if clean-up was successfully run.
    */
   cleanup(resetStats = false) {
     this.pendingCleanup = true;
-    this._tryCleanup(resetStats);
+    return this._tryCleanup(resetStats);
   }
 
   /**
@@ -1290,7 +1297,7 @@ class PDFPageProxy {
         );
       })
     ) {
-      return;
+      return false;
     }
 
     Object.keys(this.intentStates).forEach(intent => {
@@ -1302,6 +1309,7 @@ class PDFPageProxy {
       this._stats = new StatTimer();
     }
     this.pendingCleanup = false;
+    return true;
   }
 
   /**
@@ -2555,11 +2563,17 @@ class WorkerTransport {
   }
 
   startCleanup() {
-    this.messageHandler.sendWithPromise("Cleanup", null).then(() => {
+    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) {
-          page.cleanup();
+          const cleanupSuccessful = page.cleanup();
+
+          if (!cleanupSuccessful) {
+            throw new Error(
+              `startCleanup: Page ${i + 1} is currently rendering.`
+            );
+          }
         }
       }
       this.commonObjs.clear();

test/unit/api_spec.js

@@ -1151,6 +1151,14 @@ describe("api", function() {
         .catch(done.fail);
     });
 
+    it("cleans up document resources", function(done) {
+      const promise = doc.cleanup();
+      promise.then(function() {
+        expect(true).toEqual(true);
+        done();
+      }, done.fail);
+    });
+
     it("checks that fingerprints are unique", function(done) {
       const loadingTask1 = getDocument(
         buildGetDocumentParams("issue4436r.pdf")
@@ -1764,6 +1772,83 @@ describe("api", function() {
         ),
       ]).then(done);
     });
+
+    it("cleans up document resources after rendering of page", function(done) {
+      const loadingTask = getDocument(buildGetDocumentParams(basicApiFileName));
+      let canvasAndCtx;
+
+      loadingTask.promise
+        .then(pdfDoc => {
+          return pdfDoc.getPage(1).then(pdfPage => {
+            const viewport = pdfPage.getViewport({ scale: 1 });
+            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);
+
+          CanvasFactory.destroy(canvasAndCtx);
+          loadingTask.destroy().then(done);
+        }, done.fail);
+    });
+
+    it("cleans up document resources during rendering of page", function(done) {
+      const loadingTask = getDocument(
+        buildGetDocumentParams("tracemonkey.pdf")
+      );
+      let canvasAndCtx;
+
+      loadingTask.promise
+        .then(pdfDoc => {
+          return pdfDoc.getPage(1).then(pdfPage => {
+            const viewport = pdfPage.getViewport({ scale: 1 });
+            canvasAndCtx = CanvasFactory.create(
+              viewport.width,
+              viewport.height
+            );
+
+            const renderTask = pdfPage.render({
+              canvasContext: canvasAndCtx.context,
+              canvasFactory: CanvasFactory,
+              viewport,
+            });
+
+            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);
+    });
   });
   describe("Multiple `getDocument` instances", function() {
     // Regression test for https://github.com/mozilla/pdf.js/issues/6205