Slightly refactor the pages rotation handling code in the viewer

This changes both `PDFViewer` and `PDFThumbnailViewer` to return early in the `pagesRotation` setters if the rotation doesn't change.
It also fixes an existing issue, in `PDFViewer`, that would cause errors if the rotation changes *before* the scale has been set to a non-default value.

Finally, in preparation for subsequent patches, it also refactors the rotation code in `web/app.js` to update the thumbnails and trigger rendering with the new `rotationchanging` event.

test/unit/ui_utils_spec.js

@@ -14,8 +14,8 @@
  */
 
 import {
-  binarySearchFirstItem, EventBus, getPDFFileNameFromURL, waitOnEventOrTimeout,
+  binarySearchFirstItem, EventBus, getPDFFileNameFromURL, isValidRotation,
-  WaitOnType
+  waitOnEventOrTimeout, WaitOnType
 } from '../../web/ui_utils';
 import { createObjectURL, isNodeJS } from '../../src/shared/util';
 
@@ -261,6 +261,29 @@ describe('ui_utils', function() {
     });
   });
 
+  describe('isValidRotation', function() {
+    it('should reject non-integer angles', function() {
+      expect(isValidRotation()).toEqual(false);
+      expect(isValidRotation(null)).toEqual(false);
+      expect(isValidRotation(NaN)).toEqual(false);
+      expect(isValidRotation([90])).toEqual(false);
+      expect(isValidRotation('90')).toEqual(false);
+      expect(isValidRotation(90.5)).toEqual(false);
+    });
+
+    it('should reject non-multiple of 90 degree angles', function() {
+      expect(isValidRotation(45)).toEqual(false);
+      expect(isValidRotation(-123)).toEqual(false);
+    });
+
+    it('should accept valid angles', function() {
+      expect(isValidRotation(0)).toEqual(true);
+      expect(isValidRotation(90)).toEqual(true);
+      expect(isValidRotation(-270)).toEqual(true);
+      expect(isValidRotation(540)).toEqual(true);
+    });
+  });
+
   describe('waitOnEventOrTimeout', function() {
     let eventBus;
 

web/app.js

@@ -1232,16 +1232,10 @@ let PDFViewerApplication = {
     if (!this.pdfDocument) {
       return;
     }
-    let { pdfViewer, pdfThumbnailViewer, } = this;
+    let newRotation = (this.pdfViewer.pagesRotation + 360 + delta) % 360;
-    let pageNumber = pdfViewer.currentPageNumber;
+    this.pdfViewer.pagesRotation = newRotation;
-    let newRotation = (pdfViewer.pagesRotation + 360 + delta) % 360;
+    // Note that the thumbnail viewer is updated, and rendering is triggered,
-
+    // in the 'rotationchanging' event handler.
-    pdfViewer.pagesRotation = newRotation;
-    pdfThumbnailViewer.pagesRotation = newRotation;
-
-    this.forceRendering();
-    // Ensure that the active page doesn't change during rotation.
-    pdfViewer.currentPageNumber = pageNumber;
   },
 
   requestPresentationMode() {
@@ -1266,6 +1260,7 @@ let PDFViewerApplication = {
     eventBus.on('updateviewarea', webViewerUpdateViewarea);
     eventBus.on('pagechanging', webViewerPageChanging);
     eventBus.on('scalechanging', webViewerScaleChanging);
+    eventBus.on('rotationchanging', webViewerRotationChanging);
     eventBus.on('sidebarviewchanged', webViewerSidebarViewChanged);
     eventBus.on('pagemode', webViewerPageMode);
     eventBus.on('namedaction', webViewerNamedAction);
@@ -1343,6 +1338,7 @@ let PDFViewerApplication = {
     eventBus.off('updateviewarea', webViewerUpdateViewarea);
     eventBus.off('pagechanging', webViewerPageChanging);
     eventBus.off('scalechanging', webViewerScaleChanging);
+    eventBus.off('rotationchanging', webViewerRotationChanging);
     eventBus.off('sidebarviewchanged', webViewerSidebarViewChanged);
     eventBus.off('pagemode', webViewerPageMode);
     eventBus.off('namedaction', webViewerNamedAction);
@@ -1926,6 +1922,14 @@ function webViewerScaleChanging(evt) {
   PDFViewerApplication.pdfViewer.update();
 }
 
+function webViewerRotationChanging(evt) {
+  PDFViewerApplication.pdfThumbnailViewer.pagesRotation = evt.pagesRotation;
+
+  PDFViewerApplication.forceRendering();
+  // Ensure that the active page doesn't change during rotation.
+  PDFViewerApplication.pdfViewer.currentPageNumber = evt.pageNumber;
+}
+
 function webViewerPageChanging(evt) {
   let page = evt.pageNumber;
 

web/pdf_thumbnail_viewer.js

@@ -14,7 +14,7 @@
  */
 
 import {
-  getVisibleElements, NullL10n, scrollIntoView, watchScroll
+  getVisibleElements, isValidRotation, NullL10n, scrollIntoView, watchScroll
 } from './ui_utils';
 import { PDFThumbnailView } from './pdf_thumbnail_view';
 
@@ -95,12 +95,15 @@ class PDFThumbnailViewer {
   }
 
   set pagesRotation(rotation) {
-    if (!(typeof rotation === 'number' && rotation % 90 === 0)) {
+    if (!isValidRotation(rotation)) {
       throw new Error('Invalid thumbnails rotation angle.');
     }
     if (!this.pdfDocument) {
       return;
     }
+    if (this._pagesRotation === rotation) {
+      return; // The rotation didn't change.
+    }
     this._pagesRotation = rotation;
 
     for (let i = 0, ii = this._thumbnails.length; i < ii; i++) {

web/pdf_viewer.js

@@ -16,8 +16,8 @@
 import { createPromiseCapability, PDFJS } from 'pdfjs-lib';
 import {
   CSS_UNITS, DEFAULT_SCALE, DEFAULT_SCALE_VALUE, getVisibleElements,
-  MAX_AUTO_SCALE, NullL10n, RendererType, SCROLLBAR_PADDING, scrollIntoView,
+  isValidRotation, MAX_AUTO_SCALE, NullL10n, RendererType, SCROLLBAR_PADDING,
-  UNKNOWN_SCALE, VERTICAL_PADDING, watchScroll
+  scrollIntoView, UNKNOWN_SCALE, VERTICAL_PADDING, watchScroll
 } from './ui_utils';
 import { PDFRenderingQueue, RenderingStates } from './pdf_rendering_queue';
 import { AnnotationLayerBuilder } from './annotation_layer_builder';
@@ -271,20 +271,34 @@ class PDFViewer {
    * @param {number} rotation - The rotation of the pages (0, 90, 180, 270).
    */
   set pagesRotation(rotation) {
-    if (!(typeof rotation === 'number' && rotation % 90 === 0)) {
+    if (!isValidRotation(rotation)) {
       throw new Error('Invalid pages rotation angle.');
     }
     if (!this.pdfDocument) {
       return;
     }
+    if (this._pagesRotation === rotation) {
+      return; // The rotation didn't change.
+    }
     this._pagesRotation = rotation;
 
+    let pageNumber = this._currentPageNumber;
+
     for (let i = 0, ii = this._pages.length; i < ii; i++) {
       let pageView = this._pages[i];
       pageView.update(pageView.scale, rotation);
     }
+    // Prevent errors in case the rotation changes *before* the scale has been
+    // set to a non-default value.
+    if (this._currentScaleValue) {
+      this._setScale(this._currentScaleValue, true);
+    }
 
-    this._setScale(this._currentScaleValue, true);
+    this.eventBus.dispatch('rotationchanging', {
+      source: this,
+      pagesRotation: rotation,
+      pageNumber,
+    });
 
     if (this.defaultRenderingQueue) {
       this.update();

web/ui_utils.js

@@ -443,6 +443,10 @@ function normalizeWheelEventDelta(evt) {
   return delta;
 }
 
+function isValidRotation(angle) {
+  return Number.isInteger(angle) && angle % 90 === 0;
+}
+
 function cloneObj(obj) {
   let result = Object.create(null);
   for (let i in obj) {
@@ -655,6 +659,7 @@ export {
   MAX_AUTO_SCALE,
   SCROLLBAR_PADDING,
   VERTICAL_PADDING,
+  isValidRotation,
   cloneObj,
   RendererType,
   mozL10n,