Merge pull request #7595 from Snuffleupagus/api-reject-with-Error

In `display/api.js` ensure that we always reject with an `Error` in `JpegDecode`, and adjust a couple of other rejection sites as well
mozilla · Sep 5, 2016 · b26af7e · b26af7e
2 parents 38c8503 + 3799807
commit b26af7e
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 17 deletions.
diff --git 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
@@ -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();
       });
     });