From a591c3de848f1efaeb1b28dddb5ee9dbf227ba79 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 14 Jun 2023 23:07:53 +0200 Subject: [PATCH] Ensure that `cleanup` during rendering is actually ignored, to prevent a blank canvas The existing unit-test doesn't work as intended, since the page never actually renders. Note how `cleanup` is *not* allowed to run when parsing and/or rendering is ongoing, however an (old) incorrect condition could prevent rendering from ever starting. This is very old code, which has been slightly re-factored a couple of times (many years ago), however this doesn't appear to affect e.g. the default viewer since the incorrect behaviour seem highly dependent on "unlucky" timing. Note also how at the start of the `PDFPageProxy.prototype.render`-method we purposely cancel any pending `cleanup`-call, to prevent unnecessary re-parsing for multiple sequential `render`-calls. Finally, avoid running `cleanup` when document/page destruction has already started since it's pointless in that case. --- src/display/api.js | 4 ++-- test/unit/api_spec.js | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 46ceef1ac..1b1c572cc 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1524,7 +1524,7 @@ class PDFPageProxy { optionalContentConfigPromise, ]) .then(([transparency, optionalContentConfig]) => { - if (this.#pendingCleanup) { + if (this.destroyed) { complete(); return; } @@ -1727,7 +1727,7 @@ class PDFPageProxy { #tryCleanup(delayed = false) { this.#abortDelayedCleanup(); - if (!this.#pendingCleanup) { + if (!this.#pendingCleanup || this.destroyed) { return false; } if (delayed) { diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index f3e7453bf..6fefe8798 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -3204,6 +3204,7 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) const renderTask = pdfPage.render({ canvasContext: canvasAndCtx.context, viewport, + background: "#FF0000", // See comment below. }); expect(renderTask instanceof RenderTask).toEqual(true); @@ -3226,6 +3227,11 @@ Caron Broadcasting, Inc., an Ohio corporation (“Lessee”).`) await renderTask.promise; expect(renderTask.separateAnnots).toEqual(false); + // Use the red background-color to, more easily, tell that the page was + // actually rendered successfully. + const { data } = canvasAndCtx.context.getImageData(0, 0, 1, 1); + expect(data).toEqual(new Uint8ClampedArray([255, 0, 0, 255])); + CanvasFactory.destroy(canvasAndCtx); await loadingTask.destroy(); });