Simplify HTMLMediaElement::textTrackModeChanged

Simplify HTMLMediaElement::textTrackModeChanged

The if-"cascade" that dominates this method does two things:
 * Schedules (adds) cues to the cue timeline
 * Marks tracks originating from HTMLTrackElement as 'configured'.

If these two are separated, the former can be moved to the caller of
textTrackModeChanged (TextTrack::setMode) - and simplified 
significantly.
With this HTMLMediaElement no longer calls CueTimeline::addCues itself.

BUG=321654
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190848

[fs, 2015-02-23 14:19:22]

fs@opera.com changed reviewers:
+ philipj@opera.com

[fs, 2015-02-25 13:11:52]

Ping. This shouldn't be too difficult ;-)

[philipj_slow, 2015-02-25 15:52:14]

Sorry, I must have marked this as read by mistake. LGTM with questions revealing
my ignorance.

https://codereview.chromium.org/952493002/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.cpp (left):

https://codereview.chromium.org/952493002/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.cpp:1171: // If this is the first added track,
create the list of text tracks.
Was this just dead code?

https://codereview.chromium.org/952493002/diff/1/Source/core/html/track/TextT...
File Source/core/html/track/TextTrack.cpp (right):

https://codereview.chromium.org/952493002/diff/1/Source/core/html/track/TextT...
Source/core/html/track/TextTrack.cpp:183: if (mode != disabledKeyword() &&
readinessState() == Loaded) {
This was just copied, but is there any reason ReadinessState Loading isn't
included here? AFAICT there isn't anything that would add any missed cues later.

[fs, 2015-02-25 16:26:11]

https://codereview.chromium.org/952493002/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.cpp (left):

https://codereview.chromium.org/952493002/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.cpp:1171: // If this is the first added track,
create the list of text tracks.
On 2015/02/25 15:52:14, philipj_UTC7 wrote:
> Was this just dead code?

Yes. (This method was only called from within TextTrack::setMode, and only if
mediaElement(), and the latter is "m_trackList ? m_trackList->owner() : 0;" -
i.e. it's hard to send this notification if there was no list in the first
place... Probably some old iteration left behind.)

https://codereview.chromium.org/952493002/diff/1/Source/core/html/track/TextT...
File Source/core/html/track/TextTrack.cpp (right):

https://codereview.chromium.org/952493002/diff/1/Source/core/html/track/TextT...
Source/core/html/track/TextTrack.cpp:183: if (mode != disabledKeyword() &&
readinessState() == Loaded) {
On 2015/02/25 15:52:14, philipj_UTC7 wrote:
> This was just copied, but is there any reason ReadinessState Loading isn't
> included here? AFAICT there isn't anything that would add any missed cues
later.

If Loading, then cues will make their way to the track via the loader and arrive
via addListOfCues.

[fs, 2015-02-25 16:40:26]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2015-02-25 16:40:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/952493002/1

[commit-bot: I haz the power, 2015-02-25 18:37:37]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190848