Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-factor PDFViewerApplication.load such that {PDFViewer, PDFThumbnailViewer}.setDocument happens slightly earlier #11725

Merged
merged 2 commits into from
Mar 22, 2020

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Mar 21, 2020

Please refer the the individual commit messages.

Much smaller diff with https://github.com/mozilla/pdf.js/pull/11725/files?w=1


Edit: I also, as a follow-up, want to look into delaying the getPageLabels/getMetadata calls until e.g. firstPagePromise is resolved since neither of those API calls should be necessary early on.

 - Ensure that `database.files` actually contains an Array, rather than some arbitrary data.

 - Only try to lookup an existing entry when the `database` existed on load, since there's obviously nothing to find when `database.files = []` was set (this case is very common in the MOZCENTRAL build since `sessionStorage` is being used there).
…nailViewer}.setDocument` happens slightly earlier

The `BaseViewer.setDocument` method in particular is necessary for rendering to start when the viewer loads, hence you obviously want that to happen as soon as possible and without any unnecessary delays.
Unfortunately *some* API calls need to be done before that, note existing comments, however the `ViewHistory` initialization (and subsequent fetching of data) in particular can be moved slightly without any adverse effects.

As part of testing I've used logging with `performance.now()` inserted in various parts of this code, and there's *obviously* no discernible changes between `master` and this patch for e.g. rendering starting in the viewer.

*Note:* The vast majority of this patch is simple indentation changes, which were forced by Prettier (and done automatically with `gulp lint --fix`).
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/c29a89b9fa404bb/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/c29a89b9fa404bb/output.txt

Total script time: 2.51 mins

Published

@timvandermeij
Copy link
Contributor

Looks good to me. Thanks!

@timvandermeij timvandermeij merged commit b86df97 into mozilla:master Mar 22, 2020
@Snuffleupagus Snuffleupagus deleted the app-ViewHistory-init branch March 22, 2020 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants