Merge pull request #14295 from Snuffleupagus/rm-viewer-_pagesRequests

Remove the `{BaseViewer, PDFThumbnailViewer}._pagesRequests` caches

Author: timvandermeij

test/unit/api_spec.js

@@ -660,6 +660,20 @@ describe("api", function () {
       await loadingTask.destroy();
     });
 
+    it("gets page multiple time, with working caches", async function () {
+      const promiseA = pdfDocument.getPage(1);
+      const promiseB = pdfDocument.getPage(1);
+
+      expect(promiseA instanceof Promise).toEqual(true);
+      expect(promiseA).toBe(promiseB);
+
+      const pageA = await promiseA;
+      const pageB = await promiseB;
+
+      expect(pageA instanceof PDFPageProxy).toEqual(true);
+      expect(pageA).toBe(pageB);
+    });
+
     it("gets page index", async function () {
       const ref = { num: 17, gen: 0 }; // Reference to second page.
       const pageIndex = await pdfDocument.getPageIndex(ref);

web/base_viewer.js

@@ -601,7 +601,7 @@ class BaseViewer {
         }
         // Set the first `pdfPage` immediately, since it's already loaded,
         // rather than having to repeat the `PDFDocumentProxy.getPage` call in
-        // the `this._ensurePdfPageLoaded` method before rendering can start.
+        // the `this.#ensurePdfPageLoaded` method before rendering can start.
         const firstPageView = this._pages[0];
         if (firstPageView) {
           firstPageView.setPdfPage(firstPdfPage);
@@ -708,7 +708,6 @@ class BaseViewer {
     this._location = null;
     this._pagesRotation = 0;
     this._optionalContentConfigPromise = null;
-    this._pagesRequests = new WeakMap();
     this._firstPageCapability = createPromiseCapability();
     this._onePageRenderedCapability = createPromiseCapability();
     this._pagesCapability = createPromiseCapability();
@@ -1356,32 +1355,25 @@ class BaseViewer {
 
   /**
    * @param {PDFPageView} pageView
-   * @returns {Promise} Returns a promise containing a {PDFPageProxy} object.
-   * @private
+   * @returns {Promise<PDFPageProxy | null>}
    */
-  _ensurePdfPageLoaded(pageView) {
+  async #ensurePdfPageLoaded(pageView) {
     if (pageView.pdfPage) {
-      return Promise.resolve(pageView.pdfPage);
+      return pageView.pdfPage;
     }
-    if (this._pagesRequests.has(pageView)) {
-      return this._pagesRequests.get(pageView);
+    try {
+      const pdfPage = await this.pdfDocument.getPage(pageView.id);
+      if (!pageView.pdfPage) {
+        pageView.setPdfPage(pdfPage);
+      }
+      if (!this.linkService._cachedPageNumber(pdfPage.ref)) {
+        this.linkService.cachePageRef(pageView.id, pdfPage.ref);
+      }
+      return pdfPage;
+    } catch (reason) {
+      console.error("Unable to get page for page view", reason);
+      return null; // Page error -- there is nothing that can be done.
     }
-    const promise = this.pdfDocument
-      .getPage(pageView.id)
-      .then(pdfPage => {
-        if (!pageView.pdfPage) {
-          pageView.setPdfPage(pdfPage);
-        }
-        this._pagesRequests.delete(pageView);
-        return pdfPage;
-      })
-      .catch(reason => {
-        console.error("Unable to get page for page view", reason);
-        // Page error -- there is nothing that can be done.
-        this._pagesRequests.delete(pageView);
-      });
-    this._pagesRequests.set(pageView, promise);
-    return promise;
   }
 
   #getScrollAhead(visible) {
@@ -1432,7 +1424,7 @@ class BaseViewer {
     this.#toggleLoadingIconSpinner(visiblePages.ids);
 
     if (pageView) {
-      this._ensurePdfPageLoaded(pageView).then(() => {
+      this.#ensurePdfPageLoaded(pageView).then(() => {
         this.renderingQueue.renderView(pageView);
       });
       return true;

web/pdf_thumbnail_viewer.js

@@ -166,7 +166,6 @@ class PDFThumbnailViewer {
     this._pageLabels = null;
     this._pagesRotation = 0;
     this._optionalContentConfigPromise = null;
-    this._pagesRequests = new WeakMap();
     this._setImageDisabled = false;
 
     // Remove the thumbnails from the DOM.
@@ -211,7 +210,7 @@ class PDFThumbnailViewer {
         }
         // Set the first `pdfPage` immediately, since it's already loaded,
         // rather than having to repeat the `PDFDocumentProxy.getPage` call in
-        // the `this._ensurePdfPageLoaded` method before rendering can start.
+        // the `this.#ensurePdfPageLoaded` method before rendering can start.
         const firstThumbnailView = this._thumbnails[0];
         if (firstThumbnailView) {
           firstThumbnailView.setPdfPage(firstPdfPage);
@@ -262,32 +261,22 @@ class PDFThumbnailViewer {
 
   /**
    * @param {PDFThumbnailView} thumbView
-   * @returns {PDFPage}
-   * @private
+   * @returns {Promise<PDFPageProxy | null>}
    */
-  _ensurePdfPageLoaded(thumbView) {
+  async #ensurePdfPageLoaded(thumbView) {
     if (thumbView.pdfPage) {
-      return Promise.resolve(thumbView.pdfPage);
+      return thumbView.pdfPage;
     }
-    if (this._pagesRequests.has(thumbView)) {
-      return this._pagesRequests.get(thumbView);
+    try {
+      const pdfPage = await this.pdfDocument.getPage(thumbView.id);
+      if (!thumbView.pdfPage) {
+        thumbView.setPdfPage(pdfPage);
+      }
+      return pdfPage;
+    } catch (reason) {
+      console.error("Unable to get page for thumb view", reason);
+      return null; // Page error -- there is nothing that can be done.
     }
-    const promise = this.pdfDocument
-      .getPage(thumbView.id)
-      .then(pdfPage => {
-        if (!thumbView.pdfPage) {
-          thumbView.setPdfPage(pdfPage);
-        }
-        this._pagesRequests.delete(thumbView);
-        return pdfPage;
-      })
-      .catch(reason => {
-        console.error("Unable to get page for thumb view", reason);
-        // Page error -- there is nothing that can be done.
-        this._pagesRequests.delete(thumbView);
-      });
-    this._pagesRequests.set(thumbView, promise);
-    return promise;
   }
 
   #getScrollAhead(visible) {
@@ -308,7 +297,7 @@ class PDFThumbnailViewer {
       scrollAhead
     );
     if (thumbView) {
-      this._ensurePdfPageLoaded(thumbView).then(() => {
+      this.#ensurePdfPageLoaded(thumbView).then(() => {
         this.renderingQueue.renderView(thumbView);
       });
       return true;