Merge pull request #12417 from Snuffleupagus/pageView-error

Remove the `PDFPageView.{error, stats}` properties, and some related viewer clean-up

web/app.js

@@ -1775,6 +1775,13 @@ const PDFViewerApplication = {
     eventBus._on("findfromurlhash", webViewerFindFromUrlHash);
     eventBus._on("updatefindmatchescount", webViewerUpdateFindMatchesCount);
     eventBus._on("updatefindcontrolstate", webViewerUpdateFindControlState);
+
+    if (AppOptions.get("pdfBug")) {
+      _boundEvents.reportPageStatsPDFBug = reportPageStatsPDFBug;
+
+      eventBus._on("pagerendered", _boundEvents.reportPageStatsPDFBug);
+      eventBus._on("pagechanging", _boundEvents.reportPageStatsPDFBug);
+    }
     if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
       eventBus._on("fileinputchange", webViewerFileInputChange);
       eventBus._on("openfile", webViewerOpenFile);
@@ -1855,6 +1862,13 @@ const PDFViewerApplication = {
     eventBus._off("findfromurlhash", webViewerFindFromUrlHash);
     eventBus._off("updatefindmatchescount", webViewerUpdateFindMatchesCount);
     eventBus._off("updatefindcontrolstate", webViewerUpdateFindControlState);
+
+    if (_boundEvents.reportPageStatsPDFBug) {
+      eventBus._off("pagerendered", _boundEvents.reportPageStatsPDFBug);
+      eventBus._off("pagechanging", _boundEvents.reportPageStatsPDFBug);
+
+      _boundEvents.reportPageStatsPDFBug = null;
+    }
     if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
       eventBus._off("fileinputchange", webViewerFileInputChange);
       eventBus._off("openfile", webViewerOpenFile);
@@ -1966,6 +1980,20 @@ function loadAndEnablePDFBug(enabledTabs) {
   });
 }
 
+function reportPageStatsPDFBug({ pageNumber }) {
+  if (typeof Stats === "undefined" || !Stats.enabled) {
+    return;
+  }
+  const pageView = PDFViewerApplication.pdfViewer.getPageView(
+    /* index = */ pageNumber - 1
+  );
+  const pageStats = pageView && pageView.pdfPage && pageView.pdfPage.stats;
+  if (!pageStats) {
+    return;
+  }
+  Stats.add(pageNumber, pageStats);
+}
+
 function webViewerInitialized() {
   const appConfig = PDFViewerApplication.appConfig;
   let file;
@@ -2126,36 +2154,27 @@ function webViewerResetPermissions() {
   appConfig.viewerContainer.classList.remove(ENABLE_PERMISSIONS_CLASS);
 }
 
-function webViewerPageRendered(evt) {
+function webViewerPageRendered({ pageNumber, timestamp, error }) {
-  const pageNumber = evt.pageNumber;
-  const pageIndex = pageNumber - 1;
-  const pageView = PDFViewerApplication.pdfViewer.getPageView(pageIndex);
-
   // If the page is still visible when it has finished rendering,
   // ensure that the page number input loading indicator is hidden.
   if (pageNumber === PDFViewerApplication.page) {
     PDFViewerApplication.toolbar.updateLoadingIndicatorState(false);
   }
 
-  // Prevent errors in the edge-case where the PDF document is removed *before*
-  // the 'pagerendered' event handler is invoked.
-  if (!pageView) {
-    return;
-  }
-
   // Use the rendered page to set the corresponding thumbnail image.
   if (PDFViewerApplication.pdfSidebar.isThumbnailViewVisible) {
-    const thumbnailView = PDFViewerApplication.pdfThumbnailViewer.getThumbnail(
+    const pageView = PDFViewerApplication.pdfViewer.getPageView(
-      pageIndex
+      /* index = */ pageNumber - 1
     );
+    const thumbnailView = PDFViewerApplication.pdfThumbnailViewer.getThumbnail(
+      /* index = */ pageNumber - 1
+    );
+    if (pageView && thumbnailView) {
       thumbnailView.setImage(pageView);
     }
-
-  if (typeof Stats !== "undefined" && Stats.enabled && pageView.stats) {
-    Stats.add(pageNumber, pageView.stats);
   }
 
-  if (pageView.error) {
+  if (error) {
     PDFViewerApplication.l10n
       .get(
         "rendering_error",
@@ -2163,13 +2182,13 @@ function webViewerPageRendered(evt) {
         "An error occurred while rendering the page."
       )
       .then(msg => {
-        PDFViewerApplication.error(msg, pageView.error);
+        PDFViewerApplication.error(msg, error);
       });
   }
 
   PDFViewerApplication.externalServices.reportTelemetry({
     type: "pageInfo",
-    timestamp: evt.timestamp,
+    timestamp,
   });
   // It is a good time to report stream and font types.
   PDFViewerApplication.pdfDocument.getStats().then(function (stats) {
@@ -2279,9 +2298,10 @@ function webViewerUpdateViewarea(evt) {
 
   // Show/hide the loading indicator in the page number input element.
   const currentPage = PDFViewerApplication.pdfViewer.getPageView(
-    PDFViewerApplication.page - 1
+    /* index = */ PDFViewerApplication.page - 1
   );
-  const loading = currentPage.renderingState !== RenderingStates.FINISHED;
+  const loading =
+    (currentPage && currentPage.renderingState) !== RenderingStates.FINISHED;
   PDFViewerApplication.toolbar.updateLoadingIndicatorState(loading);
 }
 
@@ -2526,22 +2546,12 @@ function webViewerRotationChanging(evt) {
   PDFViewerApplication.pdfViewer.currentPageNumber = evt.pageNumber;
 }
 
-function webViewerPageChanging(evt) {
+function webViewerPageChanging({ pageNumber, pageLabel }) {
-  const page = evt.pageNumber;
+  PDFViewerApplication.toolbar.setPageNumber(pageNumber, pageLabel);
-
+  PDFViewerApplication.secondaryToolbar.setPageNumber(pageNumber);
-  PDFViewerApplication.toolbar.setPageNumber(page, evt.pageLabel || null);
-  PDFViewerApplication.secondaryToolbar.setPageNumber(page);
 
   if (PDFViewerApplication.pdfSidebar.isThumbnailViewVisible) {
-    PDFViewerApplication.pdfThumbnailViewer.scrollThumbnailIntoView(page);
+    PDFViewerApplication.pdfThumbnailViewer.scrollThumbnailIntoView(pageNumber);
-  }
-
-  // We need to update stats.
-  if (typeof Stats !== "undefined" && Stats.enabled) {
-    const pageView = PDFViewerApplication.pdfViewer.getPageView(page - 1);
-    if (pageView && pageView.stats) {
-      Stats.add(page, pageView.stats);
-    }
   }
 }
 

web/pdf_page_view.js

@@ -114,7 +114,7 @@ class PDFPageView {
     this.paintedViewportMap = new WeakMap();
     this.renderingState = RenderingStates.INITIAL;
     this.resume = null;
-    this.error = null;
+    this._renderError = null;
 
     this.annotationLayer = null;
     this.textLayer = null;
@@ -139,7 +139,6 @@ class PDFPageView {
       scale: this.scale * CSS_UNITS,
       rotation: totalRotation,
     });
-    this.stats = pdfPage.stats;
     this.reset();
   }
 
@@ -265,6 +264,7 @@ class PDFPageView {
         pageNumber: this.id,
         cssTransform: true,
         timestamp: performance.now(),
+        error: this._renderError,
       });
       return;
     }
@@ -293,6 +293,7 @@ class PDFPageView {
           pageNumber: this.id,
           cssTransform: true,
           timestamp: performance.now(),
+          error: this._renderError,
         });
         return;
       }
@@ -348,16 +349,7 @@ class PDFPageView {
       scaleX = height / width;
       scaleY = width / height;
     }
-    const cssTransform =
+    target.style.transform = `rotate(${relativeRotation}deg) scale(${scaleX}, ${scaleY})`;
-      "rotate(" +
-      relativeRotation +
-      "deg) " +
-      "scale(" +
-      scaleX +
-      "," +
-      scaleY +
-      ")";
-    target.style.transform = cssTransform;
 
     if (this.textLayer) {
       // Rotating the text layer is more complicated since the divs inside the
@@ -396,19 +388,9 @@ class PDFPageView {
       }
 
       textLayerDiv.style.transform =
-        "rotate(" +
+        `rotate(${textAbsRotation}deg) ` +
-        textAbsRotation +
+        `scale(${scale}) ` +
-        "deg) " +
+        `translate(${transX}, ${transY})`;
-        "scale(" +
-        scale +
-        ", " +
-        scale +
-        ") " +
-        "translate(" +
-        transX +
-        ", " +
-        transY +
-        ")";
       textLayerDiv.style.transformOrigin = "0% 0%";
     }
 
@@ -500,7 +482,7 @@ class PDFPageView {
       };
     }
 
-    const finishPaintTask = async error => {
+    const finishPaintTask = async (error = null) => {
       // The paintTask may have been replaced by a new one, so only remove
       // the reference to the paintTask if it matches the one that is
       // triggering this callback.
@@ -509,9 +491,10 @@ class PDFPageView {
       }
 
       if (error instanceof RenderingCancelledException) {
-        this.error = null;
+        this._renderError = null;
         return;
       }
+      this._renderError = error;
 
       this.renderingState = RenderingStates.FINISHED;
 
@@ -521,14 +504,12 @@ class PDFPageView {
       }
       this._resetZoomLayer(/* removeFromDOM = */ true);
 
-      this.error = error;
-      this.stats = pdfPage.stats;
-
       this.eventBus.dispatch("pagerendered", {
         source: this,
         pageNumber: this.id,
         cssTransform: false,
         timestamp: performance.now(),
+        error: this._renderError,
       });
 
       if (error) {

web/pdf_rendering_queue.js

@@ -13,6 +13,8 @@
  * limitations under the License.
  */
 
+import { RenderingCancelledException } from "pdfjs-lib";
+
 const CLEANUP_TIMEOUT = 30000;
 
 const RenderingStates = {
@@ -170,6 +172,9 @@ class PDFRenderingQueue {
             this.renderHighestPriority();
           })
           .catch(reason => {
+            if (reason instanceof RenderingCancelledException) {
+              return;
+            }
             console.error(`renderView: "${reason}"`);
           });
         break;

web/pdf_thumbnail_view.js

@@ -13,11 +13,8 @@
  * limitations under the License.
  */
 
-import {
-  createPromiseCapability,
-  RenderingCancelledException,
-} from "pdfjs-lib";
 import { getOutputScale, NullL10n } from "./ui_utils.js";
+import { RenderingCancelledException } from "pdfjs-lib";
 import { RenderingStates } from "./pdf_rendering_queue.js";
 
 const MAX_NUM_SCALING_STEPS = 3;
@@ -316,8 +313,7 @@ class PDFThumbnailView {
 
     this.renderingState = RenderingStates.RUNNING;
 
-    const renderCapability = createPromiseCapability();
+    const finishRenderTask = async (error = null) => {
-    const finishRenderTask = error => {
       // The renderTask may have been replaced by a new one, so only remove
       // the reference to the renderTask if it matches the one that is
       // triggering this callback.
@@ -326,17 +322,14 @@ class PDFThumbnailView {
       }
 
       if (error instanceof RenderingCancelledException) {
-        renderCapability.resolve(undefined);
         return;
       }
 
       this.renderingState = RenderingStates.FINISHED;
       this._convertCanvasToImage();
 
-      if (!error) {
+      if (error) {
-        renderCapability.resolve(undefined);
+        throw error;
-      } else {
-        renderCapability.reject(error);
       }
     };
 
@@ -362,7 +355,7 @@ class PDFThumbnailView {
     const renderTask = (this.renderTask = pdfPage.render(renderContext));
     renderTask.onContinue = renderContinueCallback;
 
-    renderTask.promise.then(
+    const resultPromise = renderTask.promise.then(
       function () {
         finishRenderTask(null);
       },
@@ -370,7 +363,7 @@ class PDFThumbnailView {
         finishRenderTask(error);
       }
     );
-    return renderCapability.promise;
+    return resultPromise;
   }
 
   setImage(pageView) {