Skip to content

Commit

Permalink
refactor: remove special loadstart handling (#3906)
Browse files Browse the repository at this point in the history
This is both a change as well as a bug fix. We tried to have better awareness of when the underlying video element changed underneath us so we can dispose of the source handler but that broke some use cases of MSE. Given that we weren't able to fix it in a reasonable non-breaking and non-invasive solution, we're taking it out.

BREAKING CHANGE: remove the double loadstart handlers that dispose the tech/source handlers if a secondary loadstart event is heard.
  • Loading branch information
brandonocasey authored and gkatsev committed Jan 18, 2017
1 parent 844e4f0 commit 73b6316
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 72 deletions.
28 changes: 0 additions & 28 deletions src/js/tech/tech.js
Original file line number Diff line number Diff line change
Expand Up @@ -1141,12 +1141,6 @@ Tech.withSourceHandlers = function(_Tech) {

if (sh !== _Tech.nativeSourceHandler) {
this.currentSource_ = source;

// Catch if someone replaced the src without calling setSource.
// If they do, set currentSource_ to null and dispose our source handler.
this.off(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_);
this.off(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_);
this.one(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_);
}

this.sourceHandler_ = sh.handleSource(source, this, this.options_);
Expand All @@ -1155,26 +1149,6 @@ Tech.withSourceHandlers = function(_Tech) {
return this;
};

/**
* Called once for the first loadstart of a video.
*
* @listens Tech#loadstart
*/
_Tech.prototype.firstLoadStartListener_ = function() {
this.one(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_);
};

// On successive loadstarts when setSource has not been called again
/**
* Called after the first loadstart for a video occurs.
*
* @listens Tech#loadstart
*/
_Tech.prototype.successiveLoadStartListener_ = function() {
this.disposeSourceHandler();
this.one(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_);
};

/**
* Clean up any existing SourceHandlers and listeners when the Tech is disposed.
*
Expand All @@ -1193,8 +1167,6 @@ Tech.withSourceHandlers = function(_Tech) {
this.cleanupAutoTextTracks();

if (this.sourceHandler_) {
this.off(this.el_, 'loadstart', _Tech.prototype.firstLoadStartListener_);
this.off(this.el_, 'loadstart', _Tech.prototype.successiveLoadStartListener_);

if (this.sourceHandler_.dispose) {
this.sourceHandler_.dispose();
Expand Down
44 changes: 0 additions & 44 deletions test/unit/tech/tech.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -506,50 +506,6 @@ QUnit.test('Tech.isTech returns correct answers for techs and components', funct
assert.ok(!isTech(isTech), 'A function is not a Tech');
});

QUnit.test('Tech#setSource clears currentSource_ after repeated loadstart', function(assert) {
let disposed = false;
const MyTech = extendFn(Tech);

Tech.withSourceHandlers(MyTech);
const tech = new MyTech();

const sourceHandler = {
canPlayType(type) {
return true;
},
canHandleSource(source, options) {
return true;
},
handleSource(source, tech_, options) {
return {
dispose() {
disposed = true;
}
};
}
};

// Test registering source handlers
MyTech.registerSourceHandler(sourceHandler);

// First loadstart
tech.setSource('test');
tech.currentSource_ = 'test';
tech.trigger('loadstart');
assert.equal(tech.currentSource_, 'test', 'Current source is test');

// Second loadstart
tech.trigger('loadstart');
assert.equal(tech.currentSource_, null, 'Current source is null');
assert.equal(disposed, true, 'disposed is true');

// Third loadstart
tech.currentSource_ = 'test';
tech.trigger('loadstart');
assert.equal(tech.currentSource_, null, 'Current source is still null');

});

QUnit.test('setSource after tech dispose should dispose source handler once', function(assert) {
const MyTech = extendFn(Tech);

Expand Down

0 comments on commit 73b6316

Please sign in to comment.