Move automatic text track selection code to AutomaticTrackSelection helper

Move automatic text track selection code to AutomaticTrackSelection helper

Move the body of
HTMLMediaElement::honorUserPreferencesForAutomaticTextTrackSelection and
support functions to a helper class (AutomaticTrackSelection).
A straight-forward code movement with the only difference being that the
helper uses the a configuration object set up from HTMLMediaElement state.

Also moves HTMLMediaElement::markCaptionAndSubtitleTracksAsUnconfigured
closer to it user.

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

[fs, 2015-02-10 17:36:18]

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

[philipj_slow, 2015-02-11 03:00:24]

LGTM % sad bots and some things about state

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
File Source/core/html/track/AutomaticTrackSelection.cpp (right):

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
Source/core/html/track/AutomaticTrackSelection.cpp:71: WTF_LOG(Media,
"TrackSelection::configureTextTrackGroup(%p, %d)", this, group.kind);
All the bots seem to be upset about this.

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
File Source/core/html/track/AutomaticTrackSelection.h (right):

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
Source/core/html/track/AutomaticTrackSelection.h:35:
RawPtrWillBeMember<TextTrackList> m_textTracks;
Could we make this an argument to perform() to save readers the trouble of even
thinking about life time?

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
Source/core/html/track/AutomaticTrackSelection.h:36: Configuration
m_configuration;
Make it const to document that performing track selection doesn't change the
configuration, and thus that a AutomaticTrackSelection object *could* be used
multiple times? (That would only happen in unit tests, though.)

[fs, 2015-02-11 09:25:59]

New patchsets have been uploaded after l-g-t-m from philipj@opera.com

[fs, 2015-02-11 10:02:26]

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
File Source/core/html/track/AutomaticTrackSelection.cpp (right):

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
Source/core/html/track/AutomaticTrackSelection.cpp:71: WTF_LOG(Media,
"TrackSelection::configureTextTrackGroup(%p, %d)", this, group.kind);
On 2015/02/11 03:00:24, philipj_UTC7 wrote:
> All the bots seem to be upset about this.

Yes, this is unpleasant... Some components of this is defined at a different
place than the macro itself (wtf/Assertions.h vs. platform/Logging.h). I find
this pretty useless so I just dropped it. If anyone wants it, they can add it
back...

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
File Source/core/html/track/AutomaticTrackSelection.h (right):

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
Source/core/html/track/AutomaticTrackSelection.h:35:
RawPtrWillBeMember<TextTrackList> m_textTracks;
On 2015/02/11 03:00:24, philipj_UTC7 wrote:
> Could we make this an argument to perform() to save readers the trouble of
even
> thinking about life time?

Done. (Wanted to make it 'const' too, but apparently item() isn't const... I'll
look at that and maybe fix in a separate CL.)

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
Source/core/html/track/AutomaticTrackSelection.h:36: Configuration
m_configuration;
On 2015/02/11 03:00:24, philipj_UTC7 wrote:
> Make it const to document that performing track selection doesn't change the
> configuration, and thus that a AutomaticTrackSelection object *could* be used
> multiple times? (That would only happen in unit tests, though.)

Done. (Unit-testability would require passing the language list too, but that
should be doable.)

[philipj_slow, 2015-02-11 10:29:53]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-02-11 10:29:54]

lgtm

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
File Source/core/html/track/AutomaticTrackSelection.h (right):

https://codereview.chromium.org/868113005/diff/1/Source/core/html/track/Autom...
Source/core/html/track/AutomaticTrackSelection.h:35:
RawPtrWillBeMember<TextTrackList> m_textTracks;
On 2015/02/11 10:02:26, fs wrote:
> On 2015/02/11 03:00:24, philipj_UTC7 wrote:
> > Could we make this an argument to perform() to save readers the trouble of
> even
> > thinking about life time?
> 
> Done. (Wanted to make it 'const' too, but apparently item() isn't const...
I'll
> look at that and maybe fix in a separate CL.)

Well, it does modify the members of the list, so even if const could be made to
work it may give the wrong impression.

[commit-bot: I haz the power, 2015-02-11 10:31:05]

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

[commit-bot: I haz the power, 2015-02-11 12:34:08]

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