Reset state upon load if showPreviousViewOnLoad

[Rob--W]

Fixes #5648

[Snuffleupagus]

I'm sorry, but given the asynchronous nature of the code involved I don't understand how we can guarantee that this will work (even if it currently does).
For it to work reliably, the code in the patch needs to be executed before viewer.js#L869. Note how both firstPagePromise and the preference fetching are async.
If these async operations happen in the wrong order, not only will the code in this PR not work, but it would also cause breakage in the history.

I'm instead suggesting that we move Preferences.get('showPreviousViewOnLoad') up to the rest of the prefs near the top of the file, and then place the code in this patch immediately before viewer.js#L869.
This would also, for free, prevent any possible issues with the browser not supporting replaceState.

Edit: Yes, I'm aware that this means that the pref won't be read again if a new file is opened in the viewer. However, I regard that as an edge case (given the main use-case for the default viewer), and I'm more worried about this patch causing unnecessary and easily preventable regressions in the future.

[Rob--W]

If you worry about race conditions, what about:

        showPreviousViewOnLoadPromise.then(function() {
            PDFHistory.initialize(self.documentFingerprint, self);
        });

[Snuffleupagus]

That would introduce a completely unnecessary dependency between the history and the preferences.
If we fail to initialize the preferences, this means that the history wouldn't work either, which would be bad.
All in all, since this is really an edge case I think that we should complicate it as little as possible, hence why I'm suggesting to move the preference fetching code to one central spot.

[Rob--W]

@Snuffleupagus Done.

I guess that by the same reasoning, we should move defaultZoomValue up.

I've noticed that we're using var self = this; in various places, where this is just the singletion PDFViewerApplication. To make it easier to search for the declarations and uses of certain properties, shouldn't we get rid of self.foo and use PDFViewerApplication.foo directly?

[Snuffleupagus]

Seems fine, but please also add preferenceShowPreviousViewOnLoad: true here: https://github.com/mozilla/pdf.js/blob/master/web/viewer.js#L112.

I guess that by the same reasoning, we should move defaultZoomValue up.

Yes, I think that would be best.
The reason that this code is spread out, is that the various pieces was added at different points in time, and no-one got around to moving it yet (hence the TODO).
(The recent effort, as far as refactoring the viewer code, has been (and still is) on preparation work to be able to release embeddable viewer APIs, see e.g. the viewer components.)

I've noticed that we're using var self = this; in various places, where this is just the singletion PDFViewerApplication. To make it easier to search for the declarations and uses of certain properties, shouldn't we get rid of self.foo and use PDFViewerApplication.foo directly?

It's fairly common in the PDF.js codebase to use self as an alias for this of the surrounding scope, to avoid the need to create unnecessary closures with bind(this).
Considering the length of e.g. PDFViewerApplication vs self, spelling it out actually feels slightly unwieldy to me. (Anyway, I think this discussion is out of scope of the current PR.)

[Rob--W]

@Snuffleupagus Done. Moving the other parameter can be done in a separate commit.

[Snuffleupagus]

Thanks for the patch!