From c046081c0b48e07945825917c53fc9a23b0ae1e9 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Wed, 15 Aug 2018 10:19:55 -0700 Subject: [PATCH] Fix spurious restrictions errors This fixes two main issues that could cause spurious RESTRICTIONS_CANNOT_BE_MET errors: 1. DrmEngine.getKeyStatuses() always returned the latest information, even if the information was incomplete. DrmEngine batches up multiple updates into one notification, so the getKeyStatuses() method should not expose incomplete information early. 2. Key status announcements happen without regard for session status, so we might get partial announcements when multiple sessions are required. For example, audio keys announced while waiting for video keys, or vice-versa. This would not occur for Widevine if content IDs are used, since content IDs cause all keys to be delivered at once. getKeyStatuses() will now return the same information that was in the most recent status callback, but will never expose that info early. And key status callbacks will be suppressed until all sessions have been loaded with licenses. This adds regression tests for both issues. Backported to v2.4.x Closes #1541 Change-Id: Iaab573be6568333423abf3450c6db00e62803145 --- lib/media/drm_engine.js | 78 ++++++++++++++++++++++++++++----- test/media/drm_engine_unit.js | 82 +++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 10 deletions(-) diff --git a/lib/media/drm_engine.js b/lib/media/drm_engine.js index 0d7da5ca9f..14f301b7cc 100644 --- a/lib/media/drm_engine.js +++ b/lib/media/drm_engine.js @@ -83,9 +83,22 @@ shaka.media.DrmEngine = function(playerInterface) { playerInterface.onError(err); }.bind(this)); - /** @private {!Object.} */ + /** + * The most recent key status information we have. + * We may not have announced this information to the outside world yet, + * which we delay to batch up changes and avoid spurious "missing key" errors. + * @private {!Object.} + */ this.keyStatusByKeyId_ = {}; + /** + * The key statuses most recently announced to other classes. + * We may have more up-to-date information being collected in + * this.keyStatusByKeyId_, which has not been batched up and released yet. + * @private {!Object.} + */ + this.announcedKeyStatusByKeyId_ = {}; + /** @private {shaka.util.Timer} */ this.keyStatusTimer_ = new shaka.util.Timer( this.processKeyStatusChanges_.bind(this)); @@ -181,7 +194,7 @@ shaka.media.DrmEngine.prototype.destroy = function() { // Due to a bug in Chrome, sometimes the Promise returned by close() // never resolves. See issue #1093 and https://crbug.com/690583. let closeTimeout = new Promise(function(resolve) { - setTimeout(resolve, 1000); + setTimeout(resolve, shaka.media.DrmEngine.CLOSE_TIMEOUT_ * 1000); }); async.push(Promise.race([close, closeTimeout])); }); @@ -462,6 +475,16 @@ shaka.media.DrmEngine.prototype.getDrmInfo = function() { }; +/** + * Returns the current key statuses. + * + * @return {!Object.} + */ +shaka.media.DrmEngine.prototype.getKeyStatuses = function() { + return this.announcedKeyStatusByKeyId_; +}; + + /** * @param {!shakaExtern.Manifest} manifest * @param {boolean} offline True if we are storing or loading offline content. @@ -1190,7 +1213,7 @@ shaka.media.DrmEngine.prototype.sendLicenseRequest_ = function(event) { if (this.activeSessions_.every((s) => s.loaded)) { this.allSessionsLoaded_.resolve(); } - }.bind(this), 5000); + }.bind(this), shaka.media.DrmEngine.SESSION_LOAD_TIMEOUT_ * 1000); } }.bind(this)); }.bind(this), function(error) { @@ -1354,9 +1377,6 @@ shaka.media.DrmEngine.prototype.onKeyStatusesChange_ = function(event) { if (status != 'status-pending') { this.activeSessions_[i].loaded = true; - if (this.activeSessions_.every((s) => s.loaded)) { - this.allSessionsLoaded_.resolve(); - } } if (status == 'expired') { @@ -1382,12 +1402,21 @@ shaka.media.DrmEngine.prototype.onKeyStatusesChange_ = function(event) { } } + const allSessionsLoaded = this.activeSessions_.every((s) => s.loaded); + if (!allSessionsLoaded) { + // Don't announce key statuses or resolve the "all loaded" promise until + // everything is loaded. + return; + } + + this.allSessionsLoaded_.resolve(); + // Batch up key status changes before checking them or notifying Player. // This handles cases where the statuses of multiple sessions are set // simultaneously by the browser before dispatching key status changes for // each of them. By batching these up, we only send one status change event // and at most one EXPIRED error on expiration. - this.keyStatusTimer_.schedule(0.5); + this.keyStatusTimer_.schedule(shaka.media.DrmEngine.KEY_STATUS_BATCH_TIME_); }; @@ -1395,6 +1424,12 @@ shaka.media.DrmEngine.prototype.onKeyStatusesChange_ = function(event) { * @private */ shaka.media.DrmEngine.prototype.processKeyStatusChanges_ = function() { + // Copy the latest key statuses into the publicly-accessible map. + this.announcedKeyStatusByKeyId_ = {}; + for (let keyId in this.keyStatusByKeyId_) { + this.announcedKeyStatusByKeyId_[keyId] = this.keyStatusByKeyId_[keyId]; + } + // If all keys are expired, fire an error. function isExpired(keyId, status) { return status == 'expired'; @@ -1402,8 +1437,8 @@ shaka.media.DrmEngine.prototype.processKeyStatusChanges_ = function() { const MapUtils = shaka.util.MapUtils; // Note that every() is always true for an empty map, // but we shouldn't fire an error for a lack of key status info. - let allExpired = !MapUtils.empty(this.keyStatusByKeyId_) && - MapUtils.every(this.keyStatusByKeyId_, isExpired); + let allExpired = !MapUtils.empty(this.announcedKeyStatusByKeyId_) && + MapUtils.every(this.announcedKeyStatusByKeyId_, isExpired); if (allExpired) { this.onError_(new shaka.util.Error( @@ -1412,7 +1447,7 @@ shaka.media.DrmEngine.prototype.processKeyStatusChanges_ = function() { shaka.util.Error.Code.EXPIRED)); } - this.playerInterface_.onKeyStatus(this.keyStatusByKeyId_); + this.playerInterface_.onKeyStatus(this.announcedKeyStatusByKeyId_); }; @@ -1622,3 +1657,26 @@ shaka.media.DrmEngine.prototype.pollExpiration_ = function() { } }.bind(this)); }; + + +/** + * The amount of time, in seconds, we wait to consider a session closed. + * This allows us to work around Chrome bug https://crbug.com/690583. + * @private {number} + */ +shaka.media.DrmEngine.CLOSE_TIMEOUT_ = 1; + +/** + * The amount of time, in seconds, we wait to consider session loaded even if no + * key status information is available. This allows us to support browsers/CDMs + * without key statuses. + * @private {number} + */ +shaka.media.DrmEngine.SESSION_LOAD_TIMEOUT_ = 5; + +/** + * The amount of time, in seconds, we wait to batch up rapid key status changes. + * This allows us to avoid multiple expiration events in most cases. + * @private {number} + */ +shaka.media.DrmEngine.KEY_STATUS_BATCH_TIME_ = 0.5; diff --git a/test/media/drm_engine_unit.js b/test/media/drm_engine_unit.js index 473127aac6..0b5de380a4 100644 --- a/test/media/drm_engine_unit.js +++ b/test/media/drm_engine_unit.js @@ -962,6 +962,80 @@ describe('DrmEngine', function() { }).catch(fail); }); + // See https://github.com/google/shaka-player/issues/1541 + it('does not update public key statuses before callback', async () => { + await initAndAttach(); + + let initData = new Uint8Array(0); + mockVideo.on['encrypted']( + {initDataType: 'webm', initData: initData, keyId: null}); + + let keyId1 = (new Uint8Array(1)).buffer; + let keyId2 = (new Uint8Array(2)).buffer; + let status1 = 'usable'; + let status2 = 'expired'; + session1.keyStatuses.forEach.and.callFake(function(callback) { + callback(keyId1, status1); + callback(keyId2, status2); + }); + + session1.on['keystatuseschange']({target: session1}); + + // The callback waits for some time to pass, to batch up status changes. + expect(onKeyStatusSpy).not.toHaveBeenCalled(); + + // The publicly-accessible key statuses should not show these new + // changes yet. This shows that we have solved the race between the + // callback and any polling done by any other component. + let keyIds = Object.keys(drmEngine.getKeyStatuses()); + expect(keyIds.length).toEqual(0); + + // Wait for the callback to occur, then end the test. + await new Promise((resolve) => { + onKeyStatusSpy.and.callFake(resolve); + }); + + // Now key statuses are available. + keyIds = Object.keys(drmEngine.getKeyStatuses()); + expect(keyIds.length).toEqual(2); + }); + + // See https://github.com/google/shaka-player/issues/1541 + it('does not invoke callback until all sessions are loaded', async () => { + // Set up init data overrides in the manifest so that we get multiple + // sessions. + let initData1 = new Uint8Array(10); + let initData2 = new Uint8Array(11); + manifest.periods[0].variants[0].drmInfos[0].initData = [ + {initData: initData1, initDataType: 'cenc', keyId: null}, + {initData: initData2, initDataType: 'cenc', keyId: null}, + ]; + + let keyId1 = (new Uint8Array(1)).buffer; + let keyId2 = (new Uint8Array(2)).buffer; + session1.keyStatuses.forEach.and.callFake(function(callback) { + callback(keyId1, 'usable'); + }); + session2.keyStatuses.forEach.and.callFake(function(callback) { + callback(keyId2, 'usable'); + }); + + await initAndAttach(); + + // The callback waits for some time to pass, to batch up status changes. + // But even after some time has passed, we should not have invoked the + // callback, because we don't have a status for session2 yet. + session1.on['keystatuseschange']({target: session1}); + await shaka.test.Util.delay(keyStatusBatchTime() + 0.5); + expect(onKeyStatusSpy).not.toHaveBeenCalled(); + + // After both sessions have been loaded, we will finally invoke the + // callback. + session2.on['keystatuseschange']({target: session2}); + await shaka.test.Util.delay(keyStatusBatchTime() + 0.5); + expect(onKeyStatusSpy).toHaveBeenCalled(); + }); + it('causes an EXPIRED error when all keys expire', function(done) { onErrorSpy.and.stub(); @@ -1932,4 +2006,12 @@ describe('DrmEngine', function() { videoRobustness: '' }; } + + /** + * @suppress {visibility} + * @return {number} + */ + function keyStatusBatchTime() { + return shaka.media.DrmEngine.KEY_STATUS_BATCH_TIME_; + } });