Replace the IPDFStream, IPDFStreamReader, and IPDFStreamRangeReader interfaces with proper base classes#20602
Merged
timvandermeij merged 8 commits intomozilla:masterfrom Feb 1, 2026
Conversation
Collaborator
Author
|
/botio test |
Collaborator
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/9d479a12e1db3a9/output.txt |
Collaborator
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/bd343bca84232ef/output.txt |
Collaborator
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/bd343bca84232ef/output.txt Total script time: 42.08 mins
Image differences available at: http://54.241.84.105:8877/bd343bca84232ef/reftest-analyzer.html#web=eq.log |
Collaborator
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/9d479a12e1db3a9/output.txt Total script time: 82.70 mins
Image differences available at: http://54.193.163.58:8877/9d479a12e1db3a9/reftest-analyzer.html#web=eq.log |
calixteman
reviewed
Jan 30, 2026
05115bd to
24714db
Compare
…lass Currently this property is essentially "duplicated", so let's instead use the identical one that's availble on the `ChunkedStream` instance.
…ed` getter This getter was only invoked from `src/display/network.js` and `src/core/chunked_stream.js`, however in both cases it's hardcoded to `false` and thus isn't actually needed. This originated in PR 6879, close to a decade ago, for a potential TODO which was never implemented and it ought to be OK to just simplify this now.
…eReader`-instance, in the `ChunkedStreamManager` class Given that nothing in the `PDFWorkerStreamRangeReader` class attempts to invoke the `onProgress` callback, this is effectively dead code now. Looking briefly at the history of this code it's not clear, at least to me, when this became unused however it's probably close to a decade ago. Finally, note also how progress is already being reported through the `ChunkedStreamManager.prototype.onReceiveData` method.
…erface Note how there's *nowhere* in the code-base where the `IPDFStreamRangeReader.prototype.onProgress` callback is actually being set and used, however the loadingBar (in the viewer) still works just fine since loading progress is already reported via: - The `ChunkedStreamManager` instance respectively the `getPdfManager` function, through the use of a "DocProgress" message, on the worker-thread. - A `IPDFStreamReader.prototype.onProgress` callback, on the main-thread. Furthermore, it would definitely *not* be a good idea to add any `IPDFStreamRangeReader.prototype.onProgress` callbacks since they only include the `loaded`-property which would trigger the "indeterminate" loadingBar (in the viewer). Looking briefly at the history of this code it's not clear, at least to me, when this became unused however it's probably close to a decade ago.
…implementations inherit from Given that there's no less than *five* different, but very similar, implementations this helps reduce code duplication and simplifies maintenance. Also, spotted during rebasing, pass the `enableHWA` option "correctly" (i.e. as part of the existing `transportParams`) to the `WorkerTransport`-class to keep the constructor simpler.
…reamReader` implementations inherit from Given that there's no less than *five* different, but very similar, implementations this helps reduce code duplication and simplifies maintenance. Also, remove the `rangeChunkSize` not defined checks in all the relevant stream-constructor implementations. Note how the API, since some time, always validates *and* provides that parameter when creating a `BasePDFStreamReader`-instance.
…PDFStreamRangeReader` implementations inherit from Given that there's no less than *five* different, but very similar, implementations this helps reduce code duplication and simplifies maintenance.
24714db to
a9a5e2e
Compare
The percentage calculation is currently "spread out" across various viewer functionality, which we can avoid by having the API handle that instead. Also, remove the `this.#lastProgress` special-case[1] and just register a "normal" `fullReader.onProgress` callback unconditionally. Once `headersReady` is resolved the callback can simply be removed when not needed, since the "worst" thing that could theoretically happen is that the loadingBar (in the viewer) updates sooner this way. In practice though, since `fullReader.read` cannot return data until `headersReady` is resolved, this change is not actually observable in the API. --- [1] This was added in PR 8617, close to a decade ago, but it's not obvious to me that it was ever necessary to implement it that way.
a9a5e2e to
ecb09d6
Compare
timvandermeij
approved these changes
Feb 1, 2026
Contributor
|
Thank you for refactoring this! |
Collaborator
Author
|
Thanks for the review! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.