From 9201c8dad4aa0fc177504e4f40bc6a838d2499f7 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 6 Oct 2019 13:52:25 +0200 Subject: [PATCH 1/2] [MessageHandler] Convert the `deleteStreamController` helper function to a "private" method instead --- src/shared/message_handler.js | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/shared/message_handler.js b/src/shared/message_handler.js index c0adeb460..451152dd2 100644 --- a/src/shared/message_handler.js +++ b/src/shared/message_handler.js @@ -334,20 +334,6 @@ MessageHandler.prototype = { const streamId = data.streamId; const comObj = this.comObj; - let deleteStreamController = () => { - // Delete the `streamController` only when the start, pull, and cancel - // capabilities have settled, to prevent `TypeError`s. - Promise.all([ - this.streamControllers[streamId].startCall, - this.streamControllers[streamId].pullCall, - this.streamControllers[streamId].cancelCall - ].map(function(capability) { - return capability && capability.promise.catch(function() { }); - })).then(() => { - delete this.streamControllers[streamId]; - }); - }; - switch (data.stream) { case StreamKind.START_COMPLETE: if (data.success) { @@ -423,14 +409,14 @@ MessageHandler.prototype = { } this.streamControllers[streamId].isClosed = true; this.streamControllers[streamId].controller.close(); - deleteStreamController(); + this._deleteStreamController(streamId); break; case StreamKind.ERROR: assert(this.streamControllers[streamId], 'error should have stream controller'); this.streamControllers[streamId].controller.error( wrapReason(data.reason)); - deleteStreamController(); + this._deleteStreamController(streamId); break; case StreamKind.CANCEL_COMPLETE: if (data.success) { @@ -439,7 +425,7 @@ MessageHandler.prototype = { this.streamControllers[streamId].cancelCall.reject( wrapReason(data.reason)); } - deleteStreamController(); + this._deleteStreamController(streamId); break; case StreamKind.CANCEL: if (!this.streamSinks[streamId]) { @@ -475,6 +461,19 @@ MessageHandler.prototype = { } }, + async _deleteStreamController(streamId) { + // Delete the `streamController` only when the start, pull, and cancel + // capabilities have settled, to prevent `TypeError`s. + await Promise.all([ + this.streamControllers[streamId].startCall, + this.streamControllers[streamId].pullCall, + this.streamControllers[streamId].cancelCall + ].map(function(capability) { + return capability && capability.promise.catch(function() { }); + })); + delete this.streamControllers[streamId]; + }, + /** * Sends raw message to the comObj. * @private From eabedab38ec27e519d34aa0d5adf5cf8ac6cec36 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 6 Oct 2019 14:07:53 +0200 Subject: [PATCH 2/2] [MessageHandler] Add a non-PRODUCTION/TESTING check to ensure that `wrapReason` is called with a valid `reason` There shouldn't be any situation where `reason` isn't either an `Error`, or a cloned "Error" sent via `postMessage`. --- src/shared/message_handler.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/shared/message_handler.js b/src/shared/message_handler.js index 451152dd2..2a3178255 100644 --- a/src/shared/message_handler.js +++ b/src/shared/message_handler.js @@ -31,7 +31,13 @@ const StreamKind = { }; function wrapReason(reason) { - if (typeof reason !== 'object') { + if (typeof PDFJSDev === 'undefined' || + PDFJSDev.test('!PRODUCTION || TESTING')) { + assert(reason instanceof Error || + (typeof reason === 'object' && reason !== null), + 'wrapReason: Expected "reason" to be a (possibly cloned) Error.'); + } + if (typeof reason !== 'object' || reason === null) { return reason; } switch (reason.name) {