For very large and complex PDF files this will help performance slightly, since `Parser.shift` is called *a lot* during parsing.
This patch was tested using the PDF file from issue 2618, i.e. http://bugzilla-attachments.gnome.org/attachment.cgi?id=226471 (with well over *four million* `Parser.shift` calls for just the one page), using the following manifest file:
```
[
{ "id": "issue2618",
"file": "../web/pdfs/issue2618.pdf",
"md5": "",
"rounds": 100,
"type": "eq"
}
]
```
This gave the following results when comparing this patch against the `master` branch:
```
-- Grouped By browser, stat --
browser | stat | Count | Baseline(ms) | Current(ms) | +/- | % | Result(P<.05)
------- | ------------ | ----- | ------------ | ----------- | --- | ----- | -------------
Firefox | Overall | 100 | 3386 | 3322 | -65 | -1.92 | faster
Firefox | Page Request | 100 | 1 | 1 | 0 | -8.08 |
Firefox | Rendering | 100 | 3385 | 3321 | -65 | -1.92 | faster
```
Remove an unnecessary `PDFDocumentProperties.setFileSize` call, relevant for the Firefox built-in viewer, and use the "normal" code-path in `PDFViewerApplication.open` instead
Since calling `getDocument` with a `PDFDataRangeTransport` argument will always unconditionally override a manually provided `length` argument, see a1a667809f/src/display/api.js (L390-L394), this patch should thus be safe.
Attempt to significantly reduce the number of `ChunkedStream.{ensureByte, ensureRange}` calls by inlining the `this.progressiveDataLength` checks at the call-sites
This functionality is very old, and pre-dates e.g. the introduction of the `EventBus` by a number of years. Rather than attaching two callback functions to every single `PDFPageView` instance, it's thus now possible to utilize the `EventBus` such that you only need a grand total of two listeners to achieve the same result.
For the `onAfterDraw` callback the replacement is particularly simple, given that a 'pagerendered' event is already being dispatched in the appropriate spot. An added benefit here is the ability to remove the event listener, since we only ever care about *one* (arbitrary) page being rendered for the `BaseViewer.onePageRendered` promise.
For the `onBeforeDraw` callback, a new 'pagerender' event was thus added to replace the callback.
Given that this special-case only matters for the Firefox PDF viewer, it's probably better to just move it into `firefoxcom.js` instead to reduce unnecessary confusion.
The way that this method handles documents without an `ID` entry in the Trailer dictionary feels overly complicated to me. Hence this patch adds `getByteRange` methods to the various Stream implementations[1], and utilize that rather than manually calling `ensureRange` when computing a fallback `fingerprint`.
---
[1] Note that `PDFDocument` is only ever initialized with either a `Stream` or a `ChunkedStream`, hence why the `DecodeStream.getByteRange` method isn't implemented.
The `finalize` helper function has only a *single* call-site, and furthermore it's just a one-liner too. Furthermore it's only ever called with a `Promise` as its argument, meaning that it's unnecessarily convoluted as well (i.e. the `Promise.resolve()` part shouldn't be necessary).
Hence this code can be both simplified *and* inlined at its only call-site instead.
Currently `wrapReason` is manually called at *every* `resolveOrReject` call-site, despite it being completely unnecessary unless there's an actual error being handled. This is obviously inefficient, and it's easy enough to avoid by having `resolveOrReject` handle this only when actually needed.
Similar to the `zoomReset` method we need to ensure that this code won't run for zoom events originating within the browser UI itself, since checks in e.g. the `keydown` event handler won't help in that case.
Note that, in the old code, there was a code-path which could prevent this from happening thus affecting future cleanup.
Furthermore, ensure that we'll always attempt to cleanup when handling the 'PageError' message, similar to the code in e.g. the `PDFPageProxy._renderPageChunk` method.
The `receivingOperatorList` property is currently tracked *twice* in the rendering code, both directly and inversely through the `intentState.operatorList.lastChunk` boolean. This type of double bookkeeping is never a good idea, since it's just too easy for the properties to accidentally fall out of sync.
In this case there's even a `cleanup`-related bug caused by this, which means that `PDFPageProxy._tryCleanup` will never be able to discard any data if there's an error on the worker-thread (as handled through the 'PageError' message).
Hence the simplest solution seems, at least to me, to update `PDFPageProxy._tryCleanup` to replace the `intentState.receivingOperatorList` check with a `!intentState.operatorList.lastChunk` check and completely remove the former property.
When searching occurs for the first time in a document, the `textContent` of all pages will be fetched from the API. If there's a pending search operation when the document loads that will thus lead to a lot of `getTextContent` calls very early on, which may unnecessarily delay rendering of the first page. Generally, in the viewer, a number of non-essential API calls[1] will be deferred until the first page has been rendered, and there's no good reason as far as I can tell to handle searching differently.
---
[1] Such as e.g. `getOutline` and `getAttachments`.
*Please note:* A a similar change was attempted in PR 5005, but it was subsequently backed out in PR 5069.
Unfortunately I don't think anyone ever tried to debug *exactly* why it didn't work, since it ought to have worked, and having re-tested this now I'm not able to reproduce the problem any more. However, given just how inefficient the current code is, with thousands of strictly unnecessary function calls for each `find` invocation, I'd really like to try fixing this again.
