Improve progress reporting in ChunkedStreamManager, and prevent unnecessary data copy in ChunkedStream.prototype.onReceiveData#20627
Merged
timvandermeij merged 2 commits intomozilla:masterfrom Feb 5, 2026
Conversation
Currently there's two small bugs, which have existed around a decade, in the `loaded` property that's sent via the "DocProgress" message from the `ChunkedStreamManager.prototype.onReceiveData` method. - When the entire PDF has loaded the `loaded` property can become larger than the `total` property, which obviously doesn't make sense. This happens whenever the size of the PDF is *not* a multiple of the `rangeChunkSize` API-option, which is a very common situation. - When streaming is being used, the `loaded` property can become smaller than the actually loaded amount of data. This happens whenever the size of a streamed chunk is *not* a multiple of the `rangeChunkSize` API-option, which is a common situation.
This method is only invoked via `ChunkedStreamManager.prototype.sendRequest`, which currently returns data in `Uint8Array` format (since it potentially combines multiple `ArrayBuffer`s). Hence we end up doing a short-lived, but still completely unnecessary, data copy[1] in `ChunkedStream.prototype.onReceiveData` when handling range requests. In practice this is unlikely to be a big problem by default, given that streaming is used and the (low) value of the `rangeChunkSize` API-option. (However, in custom PDF.js deployments it might affect things more.) Given that no data copy is better than a short lived one, let's fix this small oversight and add non-production `assert`s to keep it working as intended. This way we also improve consistency, since all other streaming and range request methods (see e.g. `BasePDFStream` and related code) only return `ArrayBuffer` data. --- [1] Remember that `new Uint8Array(arrayBuffer)` only creates a view of the underlying `arrayBuffer`, whereas `new Uint8Array(typedArray)` actually creates a copy of the `typedArray`.
Collaborator
Author
|
/botio test |
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/37df1e548c97eaf/output.txt |
Collaborator
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/ed825c8b327ec66/output.txt |
Collaborator
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/37df1e548c97eaf/output.txt Total script time: 42.01 mins
Image differences available at: http://54.241.84.105:8877/37df1e548c97eaf/reftest-analyzer.html#web=eq.log |
Collaborator
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/ed825c8b327ec66/output.txt Total script time: 83.76 mins
Image differences available at: http://54.193.163.58:8877/ed825c8b327ec66/reftest-analyzer.html#web=eq.log |
timvandermeij
approved these changes
Feb 5, 2026
Contributor
|
Thanks! |
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.