Allow media track switching.

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index 119810f9214b8611ebb33a8a391d5fc556132f9b..41c1ac8b922cc72949bc2d80ffa46f8ad4f5a7ee 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -1040,19 +1040,23 @@ void WebMediaPlayerImpl::OnFFmpegMediaTracksUpdated(
   DCHECK(!chunk_demuxer_);
 
   // Report the media track information to blink.
+  bool first_audio_track = true;

[xhwang, 2017/03/27 19:13:53]

is_first_audio_track ?

[servolk, 2017/03/27 22:34:12]

Done.

+  bool first_video_track = true;
   for (const auto& track : tracks->tracks()) {
     if (track->type() == MediaTrack::Audio) {
       client_->addAudioTrack(blink::WebString::fromUTF8(track->id()),
                              blink::WebMediaPlayerClient::AudioTrackKindMain,
                              blink::WebString::fromUTF8(track->label()),
                              blink::WebString::fromUTF8(track->language()),
-                             /*enabled*/ true);
+                             /*enabled*/ first_audio_track);

[xhwang, 2017/03/27 19:13:53]

This line doesn't make sense now :)

[servolk, 2017/03/27 22:34:12]

If you are talking about the comment, it still does make sense. The comment
clarifies that the last parameter is 'bool enabled' and tells the client whether
the added media track should be in the enabled or disabled state initially. It
just so happens that we want only the first track to be enabled initially, and
so we pass the is_first_audio_track in there. Would it be clearer to change
comment to something like /* enabled= */ ?

[xhwang, 2017/03/29 00:16:47]

I see. Thanks for the context. I think this style where you put comments before
the variable is very rare and IMHO confusing.

Also, probably add comments about why we only enable/select the first track?

[servolk, 2017/03/29 01:49:33]

Ok, I guess I can completely remove the parameter comments, if that's confusing.
Done.

+      first_audio_track = false;
     } else if (track->type() == MediaTrack::Video) {
       client_->addVideoTrack(blink::WebString::fromUTF8(track->id()),
                              blink::WebMediaPlayerClient::VideoTrackKindMain,
                              blink::WebString::fromUTF8(track->label()),
                              blink::WebString::fromUTF8(track->language()),
-                             /*selected*/ true);
+                             /*selected*/ first_video_track);

[xhwang, 2017/03/27 19:13:53]

ditto

[servolk, 2017/03/27 22:34:12]

Acknowledged.

+      first_video_track = false;
     } else {
       // Text tracks are not supported through this code path yet.
       NOTREACHED();