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.)
This commit is contained in:
Jonas Jenwald 2016-09-05 14:43:16 +02:00
parent 38c85039d1
commit 37998076c9
2 changed files with 35 additions and 17 deletions

View File

@ -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) {

View File

@ -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();
});
});