diff --git a/lib/player.js b/lib/player.js index b649ba1c7c..dcc1ba01ff 100644 --- a/lib/player.js +++ b/lib/player.js @@ -5737,11 +5737,13 @@ shaka.Player = class extends shaka.util.FakeEventTarget { } else if (this.video_.ended) { updateState = 'ended'; } - history.update(updateState); + const stateChanged = history.update(updateState); - const eventName = shaka.util.FakeEvent.EventName.StateChanged; - const data = (new Map()).set('newstate', updateState); - this.dispatchEvent(this.makeEvent_(eventName, data)); + if (stateChanged) { + const eventName = shaka.util.FakeEvent.EventName.StateChanged; + const data = (new Map()).set('newstate', updateState); + this.dispatchEvent(this.makeEvent_(eventName, data)); + } } /** diff --git a/lib/util/state_history.js b/lib/util/state_history.js index ba29be3247..1277883e13 100644 --- a/lib/util/state_history.js +++ b/lib/util/state_history.js @@ -40,13 +40,15 @@ shaka.util.StateHistory = class { /** * @param {string} state + * @return {boolean} True if this changed the state */ update(state) { // |open_| will only be |null| when we first call |update|. if (this.open_ == null) { this.start_(state); + return true; } else { - this.update_(state); + return this.update_(state); } } @@ -116,6 +118,7 @@ shaka.util.StateHistory = class { /** * @param {string} state + * @return {boolean} True if this changed the state * @private */ update_(state) { @@ -131,7 +134,7 @@ shaka.util.StateHistory = class { // If the state has not changed, there is no need to add a new entry. if (this.open_.state == state) { - return; + return false; } // We have changed states, so "close" the open state. @@ -142,6 +145,7 @@ shaka.util.StateHistory = class { state: state, duration: 0, }; + return true; } /** diff --git a/test/cast/cast_receiver_integration.js b/test/cast/cast_receiver_integration.js index eb32be193d..1d770337fd 100644 --- a/test/cast/cast_receiver_integration.js +++ b/test/cast/cast_receiver_integration.js @@ -139,6 +139,53 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => { } }); + describe('state changed event', () => { + it('does not trigger a stack overflow', async () => { + const p = waitForLoadedData(); + + // We had a regression in which polling attributes eventually triggered a + // stack overflow because of a state change event that fired every time + // we checked the state. The error itself got swallowed and hidden + // inside CastReceiver, but would cause tests to disconnect in some + // environments (consistently in GitHub Actions VMs, inconsistently + // elsewhere). + // + // Testing for this is subtle: if we try to catch the error, it will be + // caught at a point when the stack has already or is about to overflow. + // Then if we call "fail", Jasmine will grow the stack further, + // triggering *another* overflow inside of fail(), causing our test to + // *pass*. So we need to fail fast. The best way I have found is to + // catch the very first recursion of pollAttributes_, long before we + // overflow, fail, then return early to avoid the actual recursion. + const original = /** @type {?} */(receiver).pollAttributes_; + let numRecursions = 0; + // eslint-disable-next-line no-restricted-syntax + /** @type {?} */(receiver).pollAttributes_ = function() { + try { + if (numRecursions > 0) { + fail('Found recursion in pollAttributes_!'); + return undefined; + } + + numRecursions++; + return original.apply(receiver, arguments); + } finally { + numRecursions--; + } + }; + + // Start the process of loading by sending a fake init message. + fakeConnectedSenders(1); + fakeIncomingMessage({ + type: 'init', + initState: fakeInitState, + appData: {}, + }, mockShakaMessageBus); + + await p; + }); + }); + describe('without drm', () => { it('sends reasonably-sized updates', async () => { // Use an unencrypted asset.