From 9efc1784b20f77a93227a15d065d069b2e420482 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 23 Sep 2020 14:29:21 +0200 Subject: [PATCH] Remove the `PDFPageView.error` property, and simply include Errors in the "pagerendered" event instead The way that rendering errors are handled in `PDFPageView` is *very* old, and predates e.g. the introduction of the `EventBus` by several years. Hence we should be able to simplify things a bit here, by including the Error (when it exists) in the "pagerendered" event and thus avoid having to reach into `PDFPageView` for it. --- web/app.js | 9 ++++----- web/pdf_page_view.js | 11 +++++++---- web/pdf_thumbnail_view.js | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/web/app.js b/web/app.js index 6fd9d6d9f..885edab51 100644 --- a/web/app.js +++ b/web/app.js @@ -2126,8 +2126,7 @@ function webViewerResetPermissions() { appConfig.viewerContainer.classList.remove(ENABLE_PERMISSIONS_CLASS); } -function webViewerPageRendered(evt) { - const pageNumber = evt.pageNumber; +function webViewerPageRendered({ pageNumber, timestamp, error }) { const pageIndex = pageNumber - 1; const pageView = PDFViewerApplication.pdfViewer.getPageView(pageIndex); @@ -2155,7 +2154,7 @@ function webViewerPageRendered(evt) { Stats.add(pageNumber, pageView.stats); } - if (pageView.error) { + if (error) { PDFViewerApplication.l10n .get( "rendering_error", @@ -2163,13 +2162,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) { diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 31e54ff0a..90f780df3 100644 --- a/web/pdf_page_view.js +++ b/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; @@ -265,6 +265,7 @@ class PDFPageView { pageNumber: this.id, cssTransform: true, timestamp: performance.now(), + error: this._renderError, }); return; } @@ -293,6 +294,7 @@ class PDFPageView { pageNumber: this.id, cssTransform: true, timestamp: performance.now(), + error: this._renderError, }); return; } @@ -500,7 +502,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 +511,10 @@ class PDFPageView { } if (error instanceof RenderingCancelledException) { - this.error = null; + this._renderError = null; return; } + this._renderError = error; this.renderingState = RenderingStates.FINISHED; @@ -521,7 +524,6 @@ class PDFPageView { } this._resetZoomLayer(/* removeFromDOM = */ true); - this.error = error; this.stats = pdfPage.stats; this.eventBus.dispatch("pagerendered", { @@ -529,6 +531,7 @@ class PDFPageView { pageNumber: this.id, cssTransform: false, timestamp: performance.now(), + error: this._renderError, }); if (error) { diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 685bf53b6..dddd931b5 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -317,7 +317,7 @@ class PDFThumbnailView { this.renderingState = RenderingStates.RUNNING; const renderCapability = createPromiseCapability(); - const finishRenderTask = error => { + const finishRenderTask = (error = null) => { // 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.