Avoid calling destroy on the PDF.js loading task (#21277)

GitOrigin-RevId: b315e78ff739d301583f2139109f3244abceade8

services/web/frontend/js/features/file-view/components/file-view-pdf.tsx

@@ -1,7 +1,6 @@
 import { FC, useCallback } from 'react'
 import useIsMounted from '@/shared/hooks/use-is-mounted'
 import { useFileTreePathContext } from '@/features/file-tree/contexts/file-tree-path'
-import { debugConsole } from '@/utils/debugging'
 
 const FileViewPdf: FC<{
   fileId: string
@@ -21,7 +20,6 @@ const FileViewPdf: FC<{
 
         // bail out if loading PDF.js took too long
         if (!mountedRef.current) {
-          onError()
           return
         }
 
@@ -37,7 +35,6 @@ const FileViewPdf: FC<{
 
         // bail out if loading the PDF took too long
         if (!mountedRef.current) {
-          onError()
           return
         }
 
@@ -47,6 +44,12 @@ const FileViewPdf: FC<{
 
         for (let i = 1; i <= pdf.numPages; i++) {
           const page = await pdf.getPage(i)
+
+          // bail out if the component has been unmounted
+          if (!mountedRef.current) {
+            return
+          }
+
           const viewport = page.getViewport({ scale })
 
           const canvas = document.createElement('canvas')
@@ -64,11 +67,6 @@ const FileViewPdf: FC<{
         }
 
         onLoad()
-
-        return () => {
-          pdf.cleanup().catch(debugConsole.error)
-          pdf.destroy()
-        }
       }
     },
     [mountedRef, pathInFolder, fileId, previewByPath, onLoad, onError]

services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.tsx

@@ -64,24 +64,23 @@ function PdfJsViewer({ url, pdfFile }: PdfJsViewerProps) {
   const handleContainer = useCallback(
     parent => {
       if (parent) {
-        let wrapper: PDFJSWrapper | undefined
         try {
-          wrapper = new PDFJSWrapper(parent.firstChild)
+          setPdfJsWrapper(new PDFJSWrapper(parent.firstChild))
-          setPdfJsWrapper(wrapper)
         } catch (error: any) {
           setLoadingError(true)
           captureException(error)
         }
-
-        return () => {
-          setPdfJsWrapper(null)
-          wrapper?.destroy().catch(debugConsole.error)
-        }
       }
     },
     [setLoadingError]
   )
 
+  useEffect(() => {
+    return () => {
+      setPdfJsWrapper(null)
+    }
+  }, [])
+
   const [startFetch, setStartFetch] = useState(0)
 
   // listen for events and trigger rendering.
@@ -181,7 +180,9 @@ function PdfJsViewer({ url, pdfFile }: PdfJsViewerProps) {
       pdfJsWrapper
         .loadDocument({ url, pdfFile, abortController, handleFetchError })
         .then(doc => {
-          setTotalPages(doc.numPages)
+          if (doc) {
+            setTotalPages(doc.numPages)
+          }
         })
         .catch(error => {
           if (abortController.signal.aborted) return
@@ -190,7 +191,6 @@ function PdfJsViewer({ url, pdfFile }: PdfJsViewerProps) {
         })
       return () => {
         abortController.abort()
-        pdfJsWrapper.abortDocumentLoading()
       }
     }
   }, [pdfJsWrapper, url, pdfFile, setError, setStartFetch])

services/web/frontend/js/features/pdf-preview/util/pdf-js-wrapper.ts

@@ -1,6 +1,5 @@
 import { captureException } from '@/infrastructure/error-reporter'
 import { generatePdfCachingTransportFactory } from './pdf-caching-transport'
-import { debugConsole } from '@/utils/debugging'
 import { PDFJS, loadPdfDocumentFromUrl, imageResourcesPath } from './pdf-js'
 import {
   PDFViewer,
@@ -13,12 +12,11 @@ import 'pdfjs-dist/web/pdf_viewer.css'
 const DEFAULT_RANGE_CHUNK_SIZE = 128 * 1024 // 128K chunks
 
 export default class PDFJSWrapper {
-  private loadDocumentTask: PDFJS.PDFDocumentLoadingTask | undefined
-
   public readonly viewer: PDFViewer
   public readonly eventBus: EventBus
   private readonly linkService: PDFLinkService
   private readonly pdfCachingTransportFactory: any
+  private url?: string
 
   // eslint-disable-next-line no-useless-constructor
   constructor(public container: HTMLDivElement) {
@@ -49,7 +47,7 @@ export default class PDFJSWrapper {
   }
 
   // load a document from a URL
-  loadDocument({
+  async loadDocument({
     url,
     pdfFile,
     abortController,
@@ -60,61 +58,45 @@ export default class PDFJSWrapper {
     abortController: AbortController
     handleFetchError: (error: Error) => void
   }) {
-    // cancel any previous loading task
+    this.url = url
-    if (this.loadDocumentTask) {
+
-      this.loadDocumentTask.destroy().catch(debugConsole.error)
+    const rangeTransport = this.pdfCachingTransportFactory({
-      this.loadDocumentTask = undefined
+      url,
+      pdfFile,
+      abortController,
+      handleFetchError,
+    })
+    let rangeChunkSize = DEFAULT_RANGE_CHUNK_SIZE
+    if (rangeTransport && pdfFile.size < 2 * DEFAULT_RANGE_CHUNK_SIZE) {
+      // pdf.js disables the "bulk" download optimization when providing a
+      //  custom range transport. Restore it by bumping the chunk size.
+      rangeChunkSize = pdfFile.size
     }
 
-    return new Promise<PDFJS.PDFDocumentProxy>((resolve, reject) => {
+    try {
-      const rangeTransport = this.pdfCachingTransportFactory({
+      const doc = await loadPdfDocumentFromUrl(url, {
-        url,
-        pdfFile,
-        abortController,
-        handleFetchError,
-      })
-      let rangeChunkSize = DEFAULT_RANGE_CHUNK_SIZE
-      if (rangeTransport && pdfFile.size < 2 * DEFAULT_RANGE_CHUNK_SIZE) {
-        // pdf.js disables the "bulk" download optimization when providing a
-        //  custom range transport. Restore it by bumping the chunk size.
-        rangeChunkSize = pdfFile.size
-      }
-      this.loadDocumentTask = loadPdfDocumentFromUrl(url, {
         rangeChunkSize,
         range: rangeTransport,
-      })
+      }).promise
 
-      this.loadDocumentTask.promise
+      // check that this is still the current URL
-        .then(doc => {
+      if (url !== this.url) {
-          if (!this.loadDocumentTask || !this.viewer) {
+        return
-            return // ignoring the response since loading task has been aborted
+      }
-          }
 
-          const previousDoc = this.viewer.pdfDocument
+      this.viewer.setDocument(doc)
-          this.viewer.setDocument(doc)
+      this.linkService.setDocument(doc)
-          this.linkService.setDocument(doc)
-          resolve(doc)
 
-          if (previousDoc) {
+      return doc
-            previousDoc.cleanup().catch(debugConsole.error)
+    } catch (error: any) {
-            previousDoc.destroy().catch(debugConsole.error)
+      if (!error || error.name !== 'MissingPDFException') {
-          }
+        captureException(error, {
+          tags: { handler: 'pdf-preview' },
         })
-        .catch(error => {
+      }
-          if (this.loadDocumentTask) {
-            if (!error || error.name !== 'MissingPDFException') {
-              captureException(error, {
-                tags: { handler: 'pdf-preview' },
-              })
-            }
 
-            reject(error)
+      throw error
-          }
+    }
-        })
-        .finally(() => {
-          this.loadDocumentTask = undefined
-        })
-    })
   }
 
   // update the current scale value if the container size changes
@@ -216,14 +198,4 @@ export default class PDFJSWrapper {
   isVisible() {
     return this.viewer.container.offsetParent !== null
   }
-
-  abortDocumentLoading() {
-    this.loadDocumentTask = undefined
-  }
-
-  async destroy() {
-    await this.loadDocumentTask?.destroy().catch(debugConsole.error)
-    this.loadDocumentTask = undefined
-    await this.viewer.pdfDocument?.destroy().catch(debugConsole.error)
-  }
 }