Various PageViewport and MessageHandler cleanup
#9777
Merged
Conversation
This file contains 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
…e `gulp lib` build target To avoid having to manually update the "shared files" list in `gulpfile.js`, whenever any changes are made to the file structure in the `src/shared/` folder, let's simply list the files we do *not* want instead. Given that the excluded files have been consistent ever since the `lib` build target was added, this patch should help avoid unnecessary churn in `gulpfile.js` in the future.
…ls.js` Since the `PageViewport` is not used in the worker, duplicating this code on both the main and worker sides seems completely unnecessary.
The `fontScale` property was added in PR 1531, see commit mozilla@b312719 in particular, apparently for the sole purpose of supporting the "acroforms" example. However, the `fontScale` property was never used anywhere else in the code-base, and after the modernization of the "acroforms" example in PR 8030 it's been completely unused. Finally, note that there's also a (more suitably named) `scale` property on `PageViewport` instances, which contains the exact same information as the property being removed here.
Also converts all `var` to `let` for good measure.
…he JSDoc comments for the class This changes the constructor to take a parameter object, rather than a string of parameters.
… file The `MessageHandler` itself, and its assorted helper functions, are currently the single largest[1] piece of code in the `src/shared/util.js` file. By moving this code into its own file, `src/shared/util.js` thus becomes smaller and more manageable.
There's no good reason, as far as I can tell, to duplicate the functionality of the `LoopbackPort` in the unit-tests. The only difference between the implementations is that `LoopbackPort` mimics the (native) structured cloning, however that shouldn't matter here since the tests are only sending "simple" data (strings respectively arrays with numbers). Furthermore the patch also changes `LoopbackPort` to default to using "structured cloning" and deferred invocation of the listeners, since native typed array support is now a requirement for using the PDF.js library.
Snuffleupagus
force-pushed
the
PageViewport-MessageHandler-cleanup
branch
from
c8dafd2 to
89caaf4
Compare
Snuffleupagus
changed the title
PageViewport and MessageHandler cleanup
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/59d88aaf33c49f4/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/064b83b7d5a7f98/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/064b83b7d5a7f98/output.txt Total script time: 18.95 mins
|
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/59d88aaf33c49f4/output.txt Total script time: 24.55 mins
Image differences available at: http://54.215.176.217:8877/59d88aaf33c49f4/reftest-analyzer.html#web=eq.log |
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/a954ef0f7f0a8a9/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/a954ef0f7f0a8a9/output.txt Total script time: 2.92 mins Published |
timvandermeij
approved these changes
Jun 4, 2018
|
Thank you for fixing this and splitting it up into manageable pull requests! |
movsb
pushed a commit
to movsb/pdf.js
that referenced
this pull request
Jul 14, 2018
…geHandler-cleanup Various `PageViewport` and `MessageHandler` cleanup
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.
Split off from PR #9729, as requested in #9729 (comment). /cc @timvandermeij
For easier reviewing I'd suggest first of all to look at one commit at a time, and second of all to use the
?w=1flag to reduce the sizes of some of the diffs.