Re-write `PDFHistory` from scratch

This patch completely re-implements `PDFHistory` to get rid of various bugs currently present, and to hopefully make maintenance slightly easier. Most of the interface is similar to the existing one, but it should be somewhat simplified.

The new implementation should be more robust against failure, compared to the old one. Previously, it was too easy to end up in a state which basically caused the browser history to lock-up, preventing the user from navigating back/forward. (In the new implementation, the browser history should not be updated rather than breaking if things go wrong.)

Given that the code has to deal with various edge-cases, it's still not as simple as I would have liked, but it should now be somewhat easier to deal with.
The main source of complication in the code is actually that we allow the user to change the hash of a already loaded document (we'll no longer try to navigate back-and-forth in this case, since the next commit contains a workaround).

In the new code, there's also *a lot* more comments (perhaps too many?) to attempt to explain the logic. This is something that the old implementation was serverly lacking, which is a one of the reasons why it was so difficult to maintain.

One particular thing to note is that the new code uses the `pagehide` event rather than `beforeunload`, since the latter seems to be a bad idea based on https://bugzilla.mozilla.org/show_bug.cgi?id=1336763.

web/app.js

@@ -932,10 +932,14 @@ let PDFViewerApplication = {
         // i.e. not when it is embedded in a web page.
         let resetHistory = !this.viewerPrefs['showPreviousViewOnLoad'];
         this.pdfHistory.initialize(id, resetHistory);
+
+        if (this.pdfHistory.initialBookmark) {
+          this.initialBookmark = this.pdfHistory.initialBookmark;
+        }
       }
 
       let initialParams = {
-        bookmark: this.initialBookmark,
+        bookmark: null,
         hash: null,
       };
       let storePromise = store.getMultiple({
@@ -969,9 +973,11 @@ let PDFViewerApplication = {
           sidebarView,
         };
       }).then(({ hash, sidebarView, }) => {
-        this.setInitialView(hash, { sidebarView, });
+        initialParams.bookmark = this.initialBookmark;
         initialParams.hash = hash;
 
+        this.setInitialView(hash, { sidebarView, });
+
         // Make all navigation keys work on document load,
         // unless the viewer is embedded in a web page.
         if (!this.isViewerEmbedded) {
@@ -1800,7 +1806,7 @@ function webViewerHashchange(evt) {
   }
   if (!PDFViewerApplication.isInitialViewSet) {
     PDFViewerApplication.initialBookmark = hash;
-  } else {
+  } else if (!PDFViewerApplication.pdfHistory.popStateInProgress) {
     PDFViewerApplication.pdfLinkService.setHash(hash);
   }
 }

web/interfaces.js

@@ -73,9 +73,18 @@ class IPDFLinkService {
  * @interface
  */
 class IPDFHistory {
-  initialize() {}
+  /**
+   * @param {string} fingerprint - The PDF document's unique fingerprint.
+   * @param {boolean} resetHistory - (optional) Reset the browsing history.
+   */
+  initialize(fingerprint, resetHistory = false) {}
+
+  /**
+   * @param {Object} params
+   */
+  push({ namedDest, explicitDest, pageNumber, }) {}
 
-  push() {}
+  pushCurrentPosition() {}
 
   back() {}
 

web/pdf_history.js

@@ -13,21 +13,461 @@
  * limitations under the License.
  */
 
+import { parseQueryString, waitOnEventOrTimeout } from './ui_utils';
 import { getGlobalEventBus } from './dom_events';
 
+// Heuristic value used when force-resetting `this._blockHashChange`.
+const HASH_CHANGE_TIMEOUT = 1000; // milliseconds
+
+/**
+ * @typedef {Object} PDFHistoryOptions
+ * @property {IPDFLinkService} linkService - The navigation/linking service.
+ * @property {EventBus} eventBus - The application event bus.
+ */
+
+/**
+ * @typedef {Object} PushParameters
+ * @property {string} namedDest - (optional) The named destination. If absent,
+ *   a stringified version of `explicitDest` is used.
+ * @property {Array} explicitDest - The explicit destination array.
+ * @property {number} pageNumber - The page to which the destination points.
+ */
+
+function getCurrentHash() {
+  return document.location.hash;
+}
+
+function parseCurrentHash(linkService) {
+  let hash = unescape(getCurrentHash()).substring(1);
+  let params = parseQueryString(hash);
+
+  let page = params.page | 0;
+  if (!(Number.isInteger(page) && page > 0 && page <= linkService.pagesCount)) {
+    page = null;
+  }
+  return { hash, page, };
+}
+
 class PDFHistory {
+  /**
+   * @param {PDFHistoryOptions} options
+   */
   constructor({ linkService, eventBus, }) {
     this.linkService = linkService;
     this.eventBus = eventBus || getGlobalEventBus();
+
+    this.initialized = false;
+    this.initialBookmark = null;
+
+    this._boundEvents = Object.create(null);
+    this._isViewerInPresentationMode = false;
+
+    // Ensure that we don't miss a 'presentationmodechanged' event, by
+    // registering the listener immediately.
+    this.eventBus.on('presentationmodechanged', (evt) => {
+      this._isViewerInPresentationMode = evt.active || evt.switchInProgress;
+    });
+  }
+
+  /**
+   * Initialize the history for the PDF document, using either the current
+   * browser history entry or the document hash, whichever is present.
+   * @param {string} fingerprint - The PDF document's unique fingerprint.
+   * @param {boolean} resetHistory - (optional) Reset the browsing history.
+   */
+  initialize(fingerprint, resetHistory = false) {
+    if (!fingerprint || typeof fingerprint !== 'string') {
+      console.error(
+        'PDFHistory.initialize: The "fingerprint" must be a non-empty string.');
+      return;
+    }
+    let reInitialized = this.initialized && this.fingerprint !== fingerprint;
+    this.fingerprint = fingerprint;
+
+    if (!this.initialized) {
+      this._bindEvents();
+    }
+    let state = window.history.state;
+
+    this.initialized = true;
+    this.initialBookmark = null;
+
+    this._popStateInProgress = false;
+    this._blockHashChange = 0;
+    this._currentHash = getCurrentHash();
+
+    this._currentUid = this._uid = 0;
+    this._destination = null;
+    this._position = null;
+
+    if (!this._isValidState(state) || resetHistory) {
+      let { hash, page, } = parseCurrentHash(this.linkService);
+
+      if (!hash || reInitialized || resetHistory) {
+        // Ensure that the browser history is reset on PDF document load.
+        this._pushOrReplaceState(null, /* forceReplace = */ true);
+        return;
+      }
+      // Ensure that the browser history is initialized correctly when
+      // the document hash is present on PDF document load.
+      this._pushOrReplaceState({ hash, page, }, /* forceReplace = */ true);
+      return;
+    }
+
+    // The browser history contains a valid entry, ensure that the history is
+    // initialized correctly on PDF document load.
+    let destination = state.destination;
+    this._updateInternalState(destination, state.uid);
+
+    if (destination.dest) {
+      this.initialBookmark = JSON.stringify(destination.dest);
+
+      // If the history is updated, e.g. through the user changing the hash,
+      // before the initial destination has become visible, then we do *not*
+      // want to potentially add `this._position` to the browser history.
+      this._destination.page = null;
+    } else if (destination.hash) {
+      this.initialBookmark = destination.hash;
+    } else if (destination.page) {
+      // Fallback case; shouldn't be necessary, but better safe than sorry.
+      this.initialBookmark = `page=${destination.page}`;
+    }
+  }
+
+  /**
+   * Push an internal destination to the browser history.
+   * @param {PushParameters}
+   */
+  push({ namedDest, explicitDest, pageNumber, }) {
+    if (!this.initialized) {
+      return;
+    }
+    if ((namedDest && typeof namedDest !== 'string') ||
+        !(explicitDest instanceof Array) ||
+        !(Number.isInteger(pageNumber) &&
+          pageNumber > 0 && pageNumber <= this.linkService.pagesCount)) {
+      console.error('PDFHistory.push: Invalid parameters.');
+      return;
+    }
+
+    let hash = namedDest || JSON.stringify(explicitDest);
+    if (!hash) {
+      // The hash *should* never be undefined, but if that were to occur,
+      // avoid any possible issues by not updating the browser history.
+      return;
+    }
+
+    let forceReplace = false;
+    if (this._destination &&
+        (this._destination.hash === hash ||
+         isDestsEqual(this._destination.dest, explicitDest))) {
+      // When the new destination is identical to `this._destination`, and
+      // its `page` is undefined, replace the current browser history entry.
+      // NOTE: This can only occur if `this._destination` was set either:
+      //  - through the document hash being specified on load.
+      //  - through the user changing the hash of the document.
+      if (this._destination.page) {
+        return;
+      }
+      forceReplace = true;
+    }
+    if (this._popStateInProgress && !forceReplace) {
+      return;
+    }
+
+    this._pushOrReplaceState({
+      dest: explicitDest,
+      hash,
+      page: pageNumber,
+    }, forceReplace);
+  }
+
+  /**
+   * Push the current position to the browser history.
+   */
+  pushCurrentPosition() {
+    if (!this.initialized || this._popStateInProgress) {
+      return;
+    }
+    this._tryPushCurrentPosition();
+  }
+
+  /**
+   * Go back one step in the browser history.
+   * NOTE: Avoids navigating away from the document, useful for "named actions".
+   */
+  back() {
+    if (!this.initialized || this._popStateInProgress) {
+      return;
+    }
+    let state = window.history.state;
+    if (this._isValidState(state) && state.uid > 0) {
+      window.history.back();
+    }
+  }
+
+  /**
+   * Go forward one step in the browser history.
+   * NOTE: Avoids navigating away from the document, useful for "named actions".
+   */
+  forward() {
+    if (!this.initialized || this._popStateInProgress) {
+      return;
+    }
+    let state = window.history.state;
+    if (this._isValidState(state) && state.uid < (this._uid - 1)) {
+      window.history.forward();
+    }
+  }
+
+  /**
+   * @returns {boolean} Indicating if the user is currently moving through the
+   *   browser history, useful e.g. for skipping the next 'hashchange' event.
+   */
+  get popStateInProgress() {
+    return this.initialized &&
+           (this._popStateInProgress || this._blockHashChange > 0);
+  }
+
+  /**
+   * @private
+   */
+  _pushOrReplaceState(destination, forceReplace = false) {
+    let shouldReplace = forceReplace || !this._destination;
+    let newState = {
+      fingerprint: this.fingerprint,
+      uid: shouldReplace ? this._currentUid : this._uid,
+      destination,
+    };
+
+    if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('CHROME') &&
+        window.history.state && window.history.state.chromecomState) {
+      // history.state.chromecomState is managed by chromecom.js.
+      newState.chromecomState = window.history.state.chromecomState;
+    }
+    this._updateInternalState(destination, newState.uid);
+
+    if (shouldReplace) {
+      if (typeof PDFJSDev !== 'undefined' &&
+          PDFJSDev.test('FIREFOX || MOZCENTRAL')) {
+        // Providing the third argument causes a SecurityError for file:// URLs.
+        window.history.replaceState(newState, '');
+      } else {
+        window.history.replaceState(newState, '', document.URL);
+      }
+    } else {
+      if (typeof PDFJSDev !== 'undefined' &&
+          PDFJSDev.test('FIREFOX || MOZCENTRAL')) {
+        // Providing the third argument causes a SecurityError for file:// URLs.
+        window.history.pushState(newState, '');
+      } else {
+        window.history.pushState(newState, '', document.URL);
+      }
+    }
+
+    if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('CHROME') &&
+        top === window) {
+      // eslint-disable-next-line no-undef
+      chrome.runtime.sendMessage('showPageAction');
+    }
+  }
+
+  /**
+   * @private
+   */
+  _tryPushCurrentPosition() {
+    if (!this._position) {
+      return;
+    }
+    let position = this._position;
+
+    if (!this._destination) {
+      this._pushOrReplaceState(position);
+      return;
+    }
+    if (this._destination.hash === position.hash) {
+      return; // The current document position has not changed.
+    }
+    if (!this._destination.page) {
+      // `this._destination` was set through the user changing the hash of
+      // the document. Do not add `this._position` to the browser history,
+      // to avoid "flooding" it with lots of (nearly) identical entries,
+      // since we cannot ensure that the document position has changed.
+      return;
+    }
+
+    let forceReplace = false;
+    if (this._destination.page === position.first ||
+        this._destination.page === position.page) {
+      // When the `page` of `this._destination` is still visible, do not
+      // update the browsing history when `this._destination` either:
+      //  - contains an internal destination, since in this case we
+      //    cannot ensure that the document position has actually changed.
+      //  - was set through the user changing the hash of the document.
+      if (this._destination.dest || !this._destination.first) {
+        return;
+      }
+      // To avoid "flooding" the browser history, replace the current entry.
+      forceReplace = true;
+    }
+    this._pushOrReplaceState(position, forceReplace);
+  }
+
+  /**
+   * @private
+   */
+  _isValidState(state) {
+    if (!state) {
+      return false;
+    }
+    if (state.fingerprint !== this.fingerprint) {
+      // This should only occur in viewers with support for opening more than
+      // one PDF document, e.g. the GENERIC viewer.
+      return false;
+    }
+    if (!Number.isInteger(state.uid) || state.uid < 0) {
+      return false;
+    }
+    if (state.destination === null || typeof state.destination !== 'object') {
+      return false;
     }
+    return true;
+  }
+
+  /**
+   * @private
+   */
+  _updateInternalState(destination, uid) {
+    this._destination = destination;
+    this._currentUid = uid;
+    this._uid = this._currentUid + 1;
+  }
+
+  /**
+   * @private
+   */
+  _updateViewarea({ location, }) {
+    this._position = {
+      hash: this._isViewerInPresentationMode ?
+        `page=${location.pageNumber}` : location.pdfOpenParams.substring(1),
+      page: this.linkService.page,
+      first: location.pageNumber,
+    };
+
+    if (this._popStateInProgress) {
+      return;
+    }
+  }
+
+  /**
+   * @private
+   */
+  _popState({ state, }) {
+    let newHash = getCurrentHash(), hashChanged = this._currentHash !== newHash;
+    this._currentHash = newHash;
 
-  initialize(fingerprint, resetHistory = false) {}
+    if (!state ||
+        (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('CHROME') &&
+         state.chromecomState && !this._isValidState(state))) {
+      // This case corresponds to the user changing the hash of the document.
+      this._currentUid = this._uid;
 
-  push() {}
+      let { hash, page, } = parseCurrentHash(this.linkService);
+      this._pushOrReplaceState({ hash, page, }, /* forceReplace */ true);
+      return;
+    }
+    if (!this._isValidState(state)) {
+      // This should only occur in viewers with support for opening more than
+      // one PDF document, e.g. the GENERIC viewer.
+      return;
+    }
 
-  back() {}
+    // Prevent the browser history from updating until the new destination,
+    // as stored in the browser history, has been scrolled into view.
+    this._popStateInProgress = true;
+
+    if (hashChanged) {
+      // When the hash changed, implying that the 'popstate' event will be
+      // followed by a 'hashchange' event, then we do *not* want to update the
+      // browser history when handling the 'hashchange' event (in web/app.js)
+      // since that would *overwrite* the new destination navigated to below.
+      //
+      // To avoid accidentally disabling all future user-initiated hash changes,
+      // if there's e.g. another 'hashchange' listener that stops the event
+      // propagation, we make sure to always force-reset `this._blockHashChange`
+      // after `HASH_CHANGE_TIMEOUT` milliseconds have passed.
+      this._blockHashChange++;
+      waitOnEventOrTimeout({
+        target: window,
+        name: 'hashchange',
+        delay: HASH_CHANGE_TIMEOUT,
+      }).then(() => {
+        this._blockHashChange--;
+      });
+    }
 
-  forward() {}
+    // This case corresponds to navigation backwards in the browser history.
+    if (state.uid < this._currentUid && this._position && this._destination) {
+      if (this._destination.page &&
+          this._destination.page !== this._position.first &&
+          this._destination.page !== this._position.page) {
+        // If the `page` of the `this._destination` is no longer visible,
+        // push the `this._position` to the browser history before moving back.
+        this._pushOrReplaceState(this._destination);
+        this._pushOrReplaceState(this._position);
+
+        // After `window.history.back()`, we must not enter this block on the
+        // resulting 'popstate' event, since that may cause an infinite loop.
+        this._currentUid = state.uid;
+
+        window.history.back();
+        return;
+      }
+    }
+
+    // Navigate to the new destination.
+    let destination = state.destination;
+    this._updateInternalState(destination, state.uid);
+
+    if (destination.dest) {
+      this.linkService.navigateTo(destination.dest);
+    } else if (destination.hash) {
+      this.linkService.setHash(destination.hash);
+    } else if (destination.page) {
+      // Fallback case; shouldn't be necessary, but better safe than sorry.
+      this.linkService.page = destination.page;
+    }
+
+    // Since `PDFLinkService.navigateTo` is asynchronous, we thus defer the
+    // resetting of `this._popStateInProgress` slightly.
+    Promise.resolve().then(() => {
+      this._popStateInProgress = false;
+    });
+  }
+
+  /**
+   * @private
+   */
+  _bindEvents() {
+    let { _boundEvents, eventBus, } = this;
+
+    _boundEvents.updateViewarea = this._updateViewarea.bind(this);
+    _boundEvents.popState = this._popState.bind(this);
+    _boundEvents.pageHide = (evt) => {
+      // Attempt to push the `this._position` into the browser history when
+      // navigating away from the document. This is *only* done if the history
+      // is currently empty, since otherwise an existing browser history entry
+      // will end up being overwritten (given that new entries cannot be pushed
+      // into the browser history when the 'unload' event has already fired).
+      if (!this._destination) {
+        this._tryPushCurrentPosition();
+      }
+    };
+
+    eventBus.on('updateviewarea', _boundEvents.updateViewarea);
+    window.addEventListener('popstate', _boundEvents.popState);
+    window.addEventListener('pagehide', _boundEvents.pageHide);
+  }
 }
 
 function isDestsEqual(firstDest, secondDest) {

web/pdf_link_service.js

@@ -111,6 +111,13 @@ class PDFLinkService {
         return;
       }
 
+      if (this.pdfHistory) {
+        // Update the browser history before scrolling the new destination into
+        // view, to be able to accurately capture the current document position.
+        this.pdfHistory.pushCurrentPosition();
+        this.pdfHistory.push({ namedDest, explicitDest, pageNumber, });
+      }
+
       this.pdfViewer.scrollPageIntoView({
         pageNumber,
         destArray: explicitDest,