Skip to content

Commit

Permalink
Surface "idling" event from walker
Browse files Browse the repository at this point in the history 
The walker knows when it is about to go idle. This even can be useful to
our tests and cast receiver.

This change takes the first step and surfaces the event from
|shaka.routing.Walker| through |shaka.Player|.

Some tests were updated to make use of the state-idle-event and
state-change-event.

Issue #816
Issue #997
Issue #1843

Change-Id: I809a6963f49c569883ab58d4ed8e8f5898726ef4
  • Loading branch information
@vaage
vaage committed Mar 21, 2019
1 parent 10f8536 commit e081c57
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 32 deletions.
17 changes: 17 additions & 0 deletions lib/player.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,11 @@ shaka.Player = function(mediaElement, dependencyInjector) {

return this.detachNode_;
},
onIdle: (node) => {
this.dispatchEvent(new shaka.util.FakeEvent(
/* name= */ 'onstateidle',
/* data= */ {'state': node.name}));
},
};

/** @private {shaka.routing.Walker} */
Expand Down Expand Up @@ -416,6 +421,18 @@ shaka.Deprecate.init(shaka.Player.version);
* @exportDoc
*/

/**
* @event shaka.Player.StateIdleEvent
* @description Fired when the player has stopped changing states and will
* remain idle until a new state change request (e.g. load, attach, etc.) is
* made.
* @property {string} type
* 'onstateidle'
* @property {string} state
* The name of the state that the player stopped in.
* @exportDoc
*/

/**
* @event shaka.Player.EmsgEvent
* @description Fired when a non-typical emsg is found in a segment.
Expand Down
19 changes: 15 additions & 4 deletions lib/routing/walker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,13 @@ shaka.routing.Walker = class {
goog.asserts.assert(this.waitForWork_ == null,
'We should not have a promise yet.');

// We have no work to do, so wait on a new promise so that the next time
// work is provided we will be unblocked. This is to stop the main loop
// from acting like a busy-wait.
// We have no more work to do. We will wait until new work has been provided
// via request route or until we are destroyed.

this.implementation_.onIdle(this.currentlyAt_);

// Wait on a new promise so that we can be resolved by |waitForWork|. This
// avoids us acting like a busy-wait.
this.waitForWork_ = new shaka.util.PublicPromise();
return this.waitForWork_;
}
Expand Down Expand Up @@ -402,7 +406,8 @@ shaka.routing.Walker = class {
* shaka.routing.Payload):!shaka.util.AbortableOperation,
* handleError: function(
* shaka.routing.Payload,
* !Error):!Promise.<shaka.routing.Node>
* !Error):!Promise.<shaka.routing.Node>,
* onIdle: function(shaka.routing.Node)
* }}
*
* @description
Expand Down Expand Up @@ -436,6 +441,12 @@ shaka.routing.Walker = class {
* OPERATION_ABORTED. It should reset all external dependences, modify the
* payload, and return the new current node. Calls to |handleError| should
* always resolve and the walker should always be able to continue operating.
*
* @property {function(shaka.routing.Node)} onIdle
* This is the callback for when the walker has finished processing all route
* requests and needs to wait for more work. |onIdle| will be passed the
* current node. After |onIdle| has been called, the walker will block until
* a new request is made, or the walker is destroyed.
*/
shaka.routing.Walker.Implementation;

Expand Down
57 changes: 43 additions & 14 deletions test/player_integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,9 @@ describe('Player Manifest Retries', function() {
/** @type {shaka.Player} */
let player;

/** @type {!jasmine.Spy} */
let stateChangeSpy;

beforeAll(() => {
video = /** @type {!HTMLVideoElement} */ (document.createElement('video'));
video.width = 600;
Expand All @@ -514,7 +517,12 @@ describe('Player Manifest Retries', function() {
});

beforeEach(async () => {
stateChangeSpy = jasmine.createSpy('stateChange');

player = new shaka.Player();
player.addEventListener(
'onstatechange', shaka.test.Util.spyFunc(stateChangeSpy));

await player.attach(video);
});

Expand All @@ -525,9 +533,14 @@ describe('Player Manifest Retries', function() {
it('unload prevents further manifest load retries', async () => {
const loading = player.load('reject://www.foo.com/bar.mpd');

// TODO: We don't need to do time based waits here anymore, we can wait
// until we enter a load state.
await shaka.test.Util.delay(0.1);
// Wait until we are part way through the load process so that we can ensure
// we are interrupting mid-way.
await new Promise((resolve) => stateChangeSpy.and.callFake((event) => {
if (event.state == 'manifest-parser') {
resolve();
}
}));

await player.unload();

try {
Expand All @@ -541,9 +554,14 @@ describe('Player Manifest Retries', function() {
it('detach prevents further manifest load retries', async () => {
const loading = player.load('reject://www.foo.com/bar.mpd');

// TODO: We don't need to do time based waits here anymore, we can wait
// until we enter a load state.
await shaka.test.Util.delay(0.1);
// Wait until we are part way through the load process so that we can ensure
// we are interrupting mid-way.
await new Promise((resolve) => stateChangeSpy.and.callFake((event) => {
if (event.state == 'manifest-parser') {
resolve();
}
}));

await player.detach();

try {
Expand All @@ -557,9 +575,14 @@ describe('Player Manifest Retries', function() {
it('destroy prevents further manifest load retries', async () => {
const loading = player.load('reject://www.foo.com/bar.mpd');

// TODO: We don't need to do time based waits here anymore, we can wait
// until we enter a load state.
await shaka.test.Util.delay(0.1);
// Wait until we are part way through the load process so that we can ensure
// we are interrupting mid-way.
await new Promise((resolve) => stateChangeSpy.and.callFake((event) => {
if (event.state == 'manifest-parser') {
resolve();
}
}));

await player.destroy();

try {
Expand Down Expand Up @@ -605,6 +628,9 @@ describe('Player Load Path', () => {
/** @type {!jasmine.Spy} */
let stateChangeSpy;

/** @type {!jasmine.Spy} */
let stateIdleSpy;

beforeAll(async () => {
video = /** @type {!HTMLVideoElement} */ (document.createElement('video'));
video.width = 600;
Expand All @@ -621,6 +647,7 @@ describe('Player Load Path', () => {

beforeEach(() => {
stateChangeSpy = jasmine.createSpy('stateChange');
stateIdleSpy = jasmine.createSpy('stateIdle');
});

/**
Expand All @@ -631,6 +658,9 @@ describe('Player Load Path', () => {
player.addEventListener(
'onstatechange',
shaka.test.Util.spyFunc(stateChangeSpy));
player.addEventListener(
'onstateidle',
shaka.test.Util.spyFunc(stateIdleSpy));
}

// Even though some test will destroy the player, we want to make sure that
Expand All @@ -653,16 +683,15 @@ describe('Player Load Path', () => {
expect(video.src).toBeTruthy();
});

// TODO(vaage): Create a way for the load graph to notify us when it enters
// an idle state (waiting for more work). That will make it
// easier to test these scenarios.
it('does not set video.src when no video is provided', async function() {
expect(video.src).toBeFalsy();

createPlayer(/* attachedTo= */ null);

// This should always be enough time to set up MediaSource.
await shaka.test.Util.delay(2);
// Wait until the player has hit an idle state (no more internal loading
// actions).
await new Promise((resolve) => stateIdleSpy.and.callFake(resolve));

expect(video.src).toBeFalsy();
});

Expand Down
129 changes: 115 additions & 14 deletions test/routing/walker_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,23 +83,28 @@ describe('Walker', () => {
/** @type {!shaka.routing.Walker} */
let walker;


/** @type {!jasmine.Spy} */
let enterNodeSpy;

/** @type {!jasmine.Spy} */
let handleErrorSpy;

/** @type {!jasmine.Spy} */
let idleSpy;

beforeEach(() => {
enterNodeSpy = jasmine.createSpy('enterNode');
enterNodeSpy.and.returnValue(AbortableOperation.completed(undefined));

handleErrorSpy = jasmine.createSpy('handleError');

idleSpy = jasmine.createSpy('idle');

const implementation = {
getNext: (at, has, goingTo, wants) => getNext(at, goingTo),
enterNode: shaka.test.Util.spyFunc(enterNodeSpy),
handleError: shaka.test.Util.spyFunc(handleErrorSpy),
onIdle: shaka.test.Util.spyFunc(idleSpy),
};

walker = new shaka.routing.Walker(nodeA, payload, implementation);
Expand All @@ -109,6 +114,40 @@ describe('Walker', () => {
await walker.destroy();
});

it('enters idle after initialization', async () => {
await waitOnSpy(idleSpy);
});

it('enters idle after completing route', async () => {
// Execute a route but then wait a couple interrupter cycles to allow the
// walker time to idle.
await completesRoute(startNewRoute(nodeD, /* interruptible= */ false));
await waitOnSpy(idleSpy);
});

it('enters idle after error', async () => {
// The specific error does not matter.
const error = new shaka.util.Error(
shaka.util.Error.Severity.CRITICAL,
shaka.util.Error.Category.TEXT,
shaka.util.Error.Code.BAD_ENCODING);

enterNodeSpy.and.callFake((at) => {
return at == nodeC ?
shaka.util.AbortableOperation.failed(error) :
shaka.util.AbortableOperation.completed(undefined);
});

handleErrorSpy.and.returnValue(nodeA);

// Go to nodeD, passing through nodeC. This will fail, calling handleError,
// and returning the walker to nodeA. Wait a couple interrupter cycles to
// allow the walker time to idle. The route needs to be interruptible since
// we are going to return an aborted operation.
await failsRoute(startNewRoute(nodeD, /* interruptible= */ true));
await waitOnSpy(idleSpy);
});

// The walker should move node-by-node, so starting in node A (see beforeEach)
// and going to nodeD, we should see the walker enter nodeB, nodeC, and nodeD.
// We won't see it enter nodeA because it starts there and therefore never
Expand All @@ -117,13 +156,7 @@ describe('Walker', () => {
// We don't expect any errors in this test.
handleErrorSpy.and.callFake(fail);

const events = startNewRoute(nodeD, /* interruptible= */ false);

await new Promise((resolve, reject) => {
events.onEnd = resolve;
events.onCancel = reject;
events.onError = reject;
});
await completesRoute(startNewRoute(nodeD, /* interruptible= */ false));

const steps = getStepsTaken();
expect(steps).toEqual([nodeB, nodeC, nodeD]);
Expand Down Expand Up @@ -239,7 +272,7 @@ describe('Walker', () => {
route3.onStart = () => { currentRoute = 3; };

// Wait until we get to the end of route 3, that should be the end.
await new Promise((resolve) => { route3.onEnd = resolve; });
await completesRoute(route3);

// Make sure we had the correct routes when taking each step.
expect(tookSteps).toEqual([
Expand Down Expand Up @@ -277,6 +310,33 @@ describe('Walker', () => {
expect(canceledBSpy).toHaveBeenCalled();
});

it('calls handleError when step fails', async () => {
// The specific error does not matter.
const error = new shaka.util.Error(
shaka.util.Error.Severity.CRITICAL,
shaka.util.Error.Category.TEXT,
shaka.util.Error.Code.BAD_ENCODING);

// Make the walker fail when it hits node C. This should allow us to
// exercise the |handleError| path.
enterNodeSpy.and.callFake((at) => {
return at == nodeC ?
shaka.util.AbortableOperation.failed(error) :
shaka.util.AbortableOperation.completed(undefined);
});

// We want to handle the error and return to a safe state, so just put the
// walker back at node A.
handleErrorSpy.and.returnValue(nodeA);

// Go to D (passing through C). This should throw an error, so wait for the
// error to be seen. The route must be abortable because we are going to
// throw an abortable error.
await failsRoute(startNewRoute(nodeD, /* interruptible */ true));

expect(handleErrorSpy).toHaveBeenCalled();
});

// When we interrupt a route that has a step that can be aborted, it should
// abort the operation and enter the error recovery mode.
//
Expand All @@ -298,11 +358,7 @@ describe('Walker', () => {
await waitUntilEntering(goingToD, nodeC);
await shaka.test.Util.delay(0.1);

const goingToE = startNewRoute(nodeE, /* interruptible */ true);
await new Promise((resolve) => {
goingToE.onEnd = resolve;
});

await completesRoute(startNewRoute(nodeE, /* interruptible */ true));
expect(handleErrorSpy).toHaveBeenCalled();
});

Expand Down Expand Up @@ -406,4 +462,49 @@ describe('Walker', () => {
};
});
}

/**
* Create a promise from a walker's route's listeners. This assumes that the
* route should finish. The promise will resolve if the route completes and
* will reject if the route fails to complete for any reason.
*
* @param {shaka.routing.Walker.Listeners} events
* @return {!Promise}
*/
function completesRoute(events) {
return new Promise((resolve, reject) => {
events.onEnd = resolve;
events.onCancel = reject;
events.onError = reject;
});
}

/**
* Create a promise from a walker's route's listeners. This assumes that the
* route should not finish. The promise will resolve if the route fails and
* will reject if the route completes.
*
* @param {shaka.routing.Walker.Listeners} events
* @return {!Promise}
*/
function failsRoute(events) {
return new Promise((resolve, reject) => {
events.onEnd = reject;
events.onCancel = resolve;
events.onError = resolve;
});
}

/**
* Wrap a spy in a promise so that the promise will resolve once the spy is
* called.
*
* @param {!jasmine.Spy} spy
* @return {!Promise}
*/
function waitOnSpy(spy) {
return new Promise((resolve) => {
spy.and.callFake(resolve);
});
}
});

0 comments on commit e081c57

Please sign in to comment.