From 37998076c9d847a7e93b52563425fd7fd391fb53 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Mon, 5 Sep 2016 14:43:16 +0200
Subject: [PATCH] In `display/api.js` ensure that we always reject with an
 `Error` in `JpegDecode`, and adjust a couple of other rejection sites as well

In the case where the document was destroyed, we were rejecting the `Promise` in `JpegDecode` with a string instead of an `Error`. The patch also brings the wording more inline with other such rejections.

Use the `isInt` utility function when validating the `pageNumber` parameter in `WorkerTransport_getPage`, to make it more obvious what's actually happening. There's also a couple more unit-tests added, to ensure that we always fail in the expected way.

Finally, we can simplify the rejection handling in `WorkerTransport_getPageIndexByRef` somewhat. (Note that the only reason for using `catch` here is that since the promise is rejected on the worker side, the `reason` becomes a string instead of an `Error` which is why we "re-reject" on the display side.)
---
 src/display/api.js    | 17 ++++++++---------
 test/unit/api_spec.js | 35 +++++++++++++++++++++++++++--------
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/src/display/api.js b/src/display/api.js
index 06fcc18be..89353da18 100644
--- a/src/display/api.js
+++ b/src/display/api.js
@@ -49,6 +49,7 @@ var error = sharedUtil.error;
 var deprecated = sharedUtil.deprecated;
 var getVerbosityLevel = sharedUtil.getVerbosityLevel;
 var info = sharedUtil.info;
+var isInt = sharedUtil.isInt;
 var isArrayBuffer = sharedUtil.isArrayBuffer;
 var isSameOrigin = sharedUtil.isSameOrigin;
 var loadJpegStream = sharedUtil.loadJpegStream;
@@ -1612,7 +1613,7 @@ var WorkerTransport = (function WorkerTransportClosure() {
 
       messageHandler.on('JpegDecode', function(data) {
         if (this.destroyed) {
-          return Promise.reject('Worker was terminated');
+          return Promise.reject(new Error('Worker was destroyed'));
         }
 
         var imageUrl = data[0];
@@ -1662,8 +1663,7 @@ var WorkerTransport = (function WorkerTransportClosure() {
     },
 
     getPage: function WorkerTransport_getPage(pageNumber, capability) {
-      if (pageNumber <= 0 || pageNumber > this.numPages ||
-          (pageNumber|0) !== pageNumber) {
+      if (!isInt(pageNumber) || pageNumber <= 0 || pageNumber > this.numPages) {
         return Promise.reject(new Error('Invalid page request'));
       }
 
@@ -1686,12 +1686,11 @@ var WorkerTransport = (function WorkerTransportClosure() {
     },
 
     getPageIndex: function WorkerTransport_getPageIndexByRef(ref) {
-      return this.messageHandler.sendWithPromise('GetPageIndex', { ref: ref }).
-        then(function (pageIndex) {
-          return pageIndex;
-        }, function (reason) {
-          return Promise.reject(new Error(reason));
-        });
+      return this.messageHandler.sendWithPromise('GetPageIndex', {
+        ref: ref,
+      }).catch(function (reason) {
+        return Promise.reject(new Error(reason));
+      });
     },
 
     getAnnotations: function WorkerTransport_getAnnotations(pageIndex, intent) {
diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js
index 531af34de..1e0e03a6d 100644
--- a/test/unit/api_spec.js
+++ b/test/unit/api_spec.js
@@ -341,13 +341,32 @@ describe('api', function() {
       });
     });
     it('gets non-existent page', function(done) {
-      var promise = doc.getPage(100);
-      promise.then(function () {
-        done.fail('shall fail for non-existent page');
-      }, function(data) {
-        expect(data instanceof Error).toEqual(true);
-        done();
+      var outOfRangePromise = doc.getPage(100);
+      var nonIntegerPromise = doc.getPage(2.5);
+      var nonNumberPromise = doc.getPage('1');
+
+      outOfRangePromise = outOfRangePromise.then(function () {
+        throw new Error('shall fail for out-of-range pageNumber parameter');
+      }, function (reason) {
+        expect(reason instanceof Error).toEqual(true);
       });
+      nonIntegerPromise = nonIntegerPromise.then(function () {
+        throw new Error('shall fail for non-integer pageNumber parameter');
+      }, function (reason) {
+        expect(reason instanceof Error).toEqual(true);
+      });
+      nonNumberPromise = nonNumberPromise.then(function () {
+        throw new Error('shall fail for non-number pageNumber parameter');
+      }, function (reason) {
+        expect(reason instanceof Error).toEqual(true);
+      });
+
+      Promise.all([outOfRangePromise, nonIntegerPromise, nonNumberPromise]).
+        then(function () {
+          done();
+        }).catch(function (reason) {
+          done.fail(reason);
+        });
     });
     it('gets page index', function(done) {
       // reference to second page
@@ -365,8 +384,8 @@ describe('api', function() {
       var promise = doc.getPageIndex(ref);
       promise.then(function () {
         done.fail('shall fail for invalid page reference.');
-      }, function (data) {
-        expect(data instanceof Error).toEqual(true);
+      }).catch(function (reason) {
+        expect(reason instanceof Error).toEqual(true);
         done();
       });
     });