Skip to content

Commit

Permalink
Handle dispose (#407)
Browse files Browse the repository at this point in the history
* WIP

* wip

* skip test that fails due to fix

* tests
  • Loading branch information
incompl authored Jul 31, 2018
1 parent 6d5ff0f commit f8fb25f
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 9 deletions.
Empty file added package-lock.json.3120487608
Empty file.
3 changes: 2 additions & 1 deletion src/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,14 @@ const contribAdsPlugin = function(options) {
'play', 'playing', 'ended',
'adsready', 'adscanceled', 'adskip', 'adserror', 'adtimeout',
'ads-ad-started',
'contentchanged', 'contentresumed', 'readyforpostroll',
'contentchanged', 'dispose', 'contentresumed', 'readyforpostroll',
'nopreroll', 'nopostroll'], (e) => {
player.ads._state.handleEvent(e.type);
});

// Clear timeouts and handlers when player is disposed
player.on('dispose', function() {
player.ads.reset();
player.textTracks().removeEventListener('change', textTrackChangeHandler);
});

Expand Down
15 changes: 10 additions & 5 deletions src/states/BeforePreroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,15 @@ export default class BeforePreroll extends ContentState {
this.shouldResumeToContent = true;
}

/*
* Content source change before preroll is currently not handled. When
* developed, this is where to start.
*/
onContentChanged() {}
onContentChanged() {
this.init(this.player);

// TODO: Handle case where player does not play on content change
this.onPlay(this.player);
}

onDispose() {
this.init(this.player);
}

}
3 changes: 3 additions & 0 deletions src/states/abstract/State.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export default class State {
onAdTimeout() {}
onAdStarted() {}
onContentChanged() {}
onDispose() {}
onContentResumed() {}
onReadyForPostroll() {
videojs.log.warn('Unexpected readyforpostroll event');
Expand Down Expand Up @@ -123,6 +124,8 @@ export default class State {
this.onAdStarted(player);
} else if (type === 'contentchanged') {
this.onContentChanged(player);
} else if (type === 'dispose') {
this.onDispose(player);
} else if (type === 'contentresumed') {
this.onContentResumed(player);
} else if (type === 'readyforpostroll') {
Expand Down
14 changes: 11 additions & 3 deletions test/unit/states/test.BeforePreroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,18 @@ QUnit.test('skips the preroll', function(assert) {
assert.equal(this.transitionArg2, true);
});

QUnit.test('does nothing on content change', function(assert) {
this.beforePreroll.init(this.player);
QUnit.test('handles content change', function(assert) {
sinon.spy(this.beforePreroll, "init");
sinon.spy(this.beforePreroll, "onPlay");
this.beforePreroll.onContentChanged(this.player);
assert.equal(this.newState, undefined);
assert.equal(this.beforePreroll.init.calledOnce, true);
assert.equal(this.beforePreroll.onPlay.calledOnce, true);
});

QUnit.test('handles dispose', function(assert) {
sinon.spy(this.beforePreroll, "init");
this.beforePreroll.onDispose(this.player);
assert.equal(this.beforePreroll.init.calledOnce, true);
});

QUnit.test('sets _shouldBlockPlay to true', function(assert) {
Expand Down

0 comments on commit f8fb25f

Please sign in to comment.