Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Convert {BaseViewer, PDFThumbnailViewer}._pagesRequests from an Array to a WeakMap

Browse Source 
Obviously the `_pagesRequests` functionality is *mainly* used when `disableAutoFetch` is set, but it will also be used during ranged/streamed loading of documents.
However, the `_pagesRequests` property is currently an Array which seems a bit strange:

 - Arrays are zero-indexed, but the first element will never actually be set in the code.
 - The `_pagesRequests` Array is never cleared, unless a new document is loaded, and once the `PDFDocumentProxy.getPage` call has resolved/rejected the element is just replaced by `null`.
 - Unless the document is browsed *in order* the resulting `_pagesRequests` Array can also be arbitrarily sparse.

All in all, I don't believe that an Array is an appropriate data structure to use for this purpose.
This commit is contained in:
Jonas Jenwald 2019-11-19 14:12:47 +01:00
parent ef1cd11908
commit 0f73758222
2 changed files with 16 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
web/base_viewer.js
View File

@ -533,7 +533,7 @@ class BaseViewer {
this._buffer = new PDFPageViewBuffer(DEFAULT_CACHE_SIZE);
this._location = null;
this._pagesRotation = 0;
this._pagesRequests = [];
this._pagesRequests = new WeakMap();
this._pageViewsReady = false;
this._scrollMode = ScrollMode.VERTICAL;
this._spreadMode = SpreadMode.NONE;
@ -961,22 +961,21 @@ class BaseViewer {
if (pageView.pdfPage) {
return Promise.resolve(pageView.pdfPage);
}
let pageNumber = pageView.id;
if (this._pagesRequests[pageNumber]) {
return this._pagesRequests[pageNumber];
if (this._pagesRequests.has(pageView)) {
return this._pagesRequests.get(pageView);
}
let promise = this.pdfDocument.getPage(pageNumber).then((pdfPage) => {
const promise = this.pdfDocument.getPage(pageView.id).then((pdfPage) => {
if (!pageView.pdfPage) {
pageView.setPdfPage(pdfPage);
}
this._pagesRequests[pageNumber] = null;
this._pagesRequests.delete(pageView);
return pdfPage;
}).catch((reason) => {
console.error('Unable to get page for page view', reason);
// Page error -- there is nothing can be done.
this._pagesRequests[pageNumber] = null;
// Page error -- there is nothing that can be done.
this._pagesRequests.delete(pageView);
});
this._pagesRequests[pageNumber] = promise;
this._pagesRequests.set(pageView, promise);
return promise;
}

17
web/pdf_thumbnail_viewer.js
View File

@ -147,7 +147,7 @@ class PDFThumbnailViewer {
this._currentPageNumber = 1;
this._pageLabels = null;
this._pagesRotation = 0;
this._pagesRequests = [];
this._pagesRequests = new WeakMap();
// Remove the thumbnails from the DOM.
this.container.textContent = '';
@ -231,20 +231,19 @@ class PDFThumbnailViewer {
if (thumbView.pdfPage) {
return Promise.resolve(thumbView.pdfPage);
}
let pageNumber = thumbView.id;
if (this._pagesRequests[pageNumber]) {
return this._pagesRequests[pageNumber];
if (this._pagesRequests.has(thumbView)) {
return this._pagesRequests.get(thumbView);
}
let promise = this.pdfDocument.getPage(pageNumber).then((pdfPage) => {
const promise = this.pdfDocument.getPage(thumbView.id).then((pdfPage) => {
thumbView.setPdfPage(pdfPage);
this._pagesRequests[pageNumber] = null;
this._pagesRequests.delete(thumbView);
return pdfPage;
}).catch((reason) => {
console.error('Unable to get page for thumb view', reason);
// Page error -- there is nothing can be done.
this._pagesRequests[pageNumber] = null;
// Page error -- there is nothing that can be done.
this._pagesRequests.delete(thumbView);
});
this._pagesRequests[pageNumber] = promise;
this._pagesRequests.set(thumbView, promise);
return promise;
}