Skip to content

Commit

Permalink
Correct oversights in the player's load graph
Browse files Browse the repository at this point in the history
There were some oversights when moving code out of Player.onLoad_. The
method header needed to be updated to better reflect the actions of the
method and some data no longer needed to be moved from |wants| to |has|.

The largest offender was asserts. The assert messages were not updated
when copied to new nodes.

Issue #816
Issue #997

Change-Id: I1e3e8a7c883af6a665c2223b1b2fcab330438e4e
  • Loading branch information
vaage committed Mar 12, 2019
1 parent 71fb7a0 commit 2c845c7
Showing 1 changed file with 46 additions and 39 deletions.
85 changes: 46 additions & 39 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -1099,10 +1099,12 @@ shaka.Player.prototype.onInitializeMediaSourceEngine_ = async function(
has, wants) {
goog.asserts.assert(
has.mediaElement,
'We should have a media element when loading.');
'We should have a media element when initializing media source.');
goog.asserts.assert(
has.mediaElement == wants.mediaElement,
'|has| and |wants| should have the same media element when loading.');
'|has| and |wants| should have the same media element when ' +
'initializing media source.');

goog.asserts.assert(
this.mediaSourceEngine_ == null,
'We should not have a media source engine yet.');
Expand Down Expand Up @@ -1142,26 +1144,29 @@ shaka.Player.prototype.onInitializeMediaSourceEngine_ = async function(
*/
shaka.Player.prototype.onInitializeParser_ = async function(has, wants) {
goog.asserts.assert(
this.networkingEngine_,
'Need networking engine to parse manifest.');
has.mediaElement,
'We should have a media element when initializing the parser.');
goog.asserts.assert(
this.config_,
'Need player config to parse manifest.');
has.mediaElement == wants.mediaElement,
'|has| and |wants| should have the same media element when ' +
'initializing the parser.');

goog.asserts.assert(
has.mediaElement,
'We should have a media element when loading.');
this.networkingEngine_,
'Need networking engine when initializing the parser.');
goog.asserts.assert(
has.mediaElement == wants.mediaElement,
'|has| and |wants| should have the same media element when loading.');
this.config_,
'Need player config when initializing the parser.');

// We are going to "lock-in" the factory, mime type, and uri since they are
// what we are going to use to create our parser and parse the manifest.
has.factory = wants.factory;
has.mimeType = wants.mimeType;
has.uri = wants.uri;

goog.asserts.assert(has.uri, 'We should have an assert uri when parsing.');
goog.asserts.assert(
has.uri,
'We should have an asset uri when initializing the parsing.');

// Store references to things we asserted so that we don't need to reassert
// them again later.
Expand Down Expand Up @@ -1199,34 +1204,34 @@ shaka.Player.prototype.onInitializeParser_ = async function(has, wants) {
* @private
*/
shaka.Player.prototype.onParseManifest_ = function(has, wants) {
goog.asserts.assert(
this.networkingEngine_,
'Need networking engine to parse manifest.');
goog.asserts.assert(
this.config_,
'Need player config to parse manifest.');

goog.asserts.assert(
this.parser_,
'|this.parser_| should have been set in an earlier step.');

goog.asserts.assert(
has.factory == wants.factory,
'|has| and |wants| should have the same factory when loading.');
'|has| and |wants| should have the same factory when parsing.');
goog.asserts.assert(
has.mimeType == wants.mimeType,
'|has| and |wants| should have the same mime type when loading.');
'|has| and |wants| should have the same mime type when parsing.');
goog.asserts.assert(
has.uri == wants.uri,
'|has| and |wants| should have the same uri when loading.');
'|has| and |wants| should have the same uri when parsing.');

goog.asserts.assert(
has.uri,
'|has| should have a valid uri when parsing a manifest.');
'|has| should have a valid uri when parsing.');
goog.asserts.assert(
has.uri == this.assetUri_,
'|has.uri| should match the cached asset uri.');

goog.asserts.assert(
this.networkingEngine_,
'Need networking engine to parse manifest.');
goog.asserts.assert(
this.config_,
'Need player config to parse manifest.');

goog.asserts.assert(
this.parser_,
'|this.parser_| should have been set in an earlier step.');

// Store references to things we asserted so that we don't need to reassert
// them again later.
const assetUri = has.uri;
Expand Down Expand Up @@ -1347,34 +1352,36 @@ shaka.Player.prototype.onInitializeDrm_ = async function(has, wants) {
*
* Loading is defined as:
* - Attaching all playback-related listeners to the media element
* - Initializing all playback components
* - Initializing playback and observers
* - Initializing ABR Manager
* - Initializing Streaming Engine
* - Starting playback at |wants.startTime|
*
* @param {shaka.routing.Payload} has
* @param {shaka.routing.Payload} wants
* @private
*/
shaka.Player.prototype.onLoad_ = async function(has, wants) {
goog.asserts.assert(
has.factory == wants.factory,
'|has| and |wants| should have the same factory when loading.');
goog.asserts.assert(
has.mimeType == wants.mimeType,
'|has| and |wants| should have the same mime type when loading.');
goog.asserts.assert(
has.uri == wants.uri,
'|has| and |wants| should have the same uri when loading.');

goog.asserts.assert(
has.mediaElement,
'We should have a media element when loading.');
goog.asserts.assert(
has.mediaElement == wants.mediaElement,
'|has| and |wants| should have the same media element when loading.');
goog.asserts.assert(
wants.startTimeOfLoad,
'|wants| should tell us when the load was originally requested');

// Copy over all the data from |wants| to |has| that we are going to use and
// therefore depend on.
// Since we are about to start playback, we will lock in the start time as
// something we are now depending on.
has.startTime = wants.startTime;
has.factory = wants.factory;
has.mimeType = wants.mimeType;
has.uri = wants.uri;

goog.asserts.assert(
has.uri,
'We should have an assert uri when loading.');

// Store a reference to values in |has| after asserting so that closure will
// know that they will still be non-null between calls to await.
Expand Down

0 comments on commit 2c845c7

Please sign in to comment.