Don't attempt to enable non-existant placeholder tracks.

The media pipeline does not expect to be called about track changes
prior to HaveCurrentData. There's also no reason for the element
to create disabled video+audio tracks and then tell the renderer
to enable those very same non-existant tracks. Just create them
as enabled instead.

It's possible we can remove even the creation of placeholder
tracks, but that's a bigger web facing change, so I don't want
to hold up this bug fix on that analysis.

BUG=873837
TEST=new unittest, twitter videos no longer hang.

Change-Id: I9863ae4feca29a58140e6dad1532bc8b7a380c3b
Reviewed-on: https://chromium-review.googlesource.com/1180347
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585334}

third_party/blink/renderer/core/html/media/html_media_element.cc

@@ -1828,8 +1828,6 @@ void HTMLMediaElement::SetReadyState(ReadyState state) {
   if (ready_state_ >= kHaveMetadata && old_state < kHaveMetadata) {
     CreatePlaceholderTracksIfNecessary();
 
-    SelectInitialTracksIfNecessary();
-
     MediaFragmentURIParser fragment_parser(current_src_);
     fragment_end_time_ = fragment_parser.EndTime();
 
@@ -3981,30 +3979,17 @@ void HTMLMediaElement::CreatePlaceholderTracksIfNecessary() {
   // didn't explicitly announce the tracks.
   if (HasAudio() && !audioTracks().length()) {
     AddAudioTrack("audio", WebMediaPlayerClient::kAudioTrackKindMain,
-                  "Audio Track", "", false);
+                  "Audio Track", "", true);
   }
 
   // Create a placeholder video track if the player says it has video but it
   // didn't explicitly announce the tracks.
   if (HasVideo() && !videoTracks().length()) {
     AddVideoTrack("video", WebMediaPlayerClient::kVideoTrackKindMain,
-                  "Video Track", "", false);
+                  "Video Track", "", true);
   }
 }
 
-void HTMLMediaElement::SelectInitialTracksIfNecessary() {
-  if (!MediaTracksEnabledInternally())
-    return;
-
-  // Enable the first audio track if an audio track hasn't been enabled yet.
-  if (audioTracks().length() > 0 && !audioTracks().HasEnabledTrack())
-    audioTracks().AnonymousIndexedGetter(0)->setEnabled(true);
-
-  // Select the first video track if a video track hasn't been selected yet.
-  if (videoTracks().length() > 0 && videoTracks().selectedIndex() == -1)
-    videoTracks().AnonymousIndexedGetter(0)->setSelected(true);
-}
-
 void HTMLMediaElement::SetNetworkState(NetworkState state) {
   if (network_state_ == state)
     return;

third_party/blink/renderer/core/html/media/html_media_element.h

@@ -534,16 +534,12 @@ class CORE_EXPORT HTMLMediaElement
   DirectionOfPlayback GetDirectionOfPlayback() const;
 
   // Creates placeholder AudioTrack and/or VideoTrack objects when
-  // WebMemediaPlayer objects advertise they have audio and/or video, but don't
+  // WebMediaPlayer objects advertise they have audio and/or video, but don't
   // explicitly signal them via addAudioTrack() and addVideoTrack().
   // FIXME: Remove this once all WebMediaPlayer implementations properly report
   // their track info.
   void CreatePlaceholderTracksIfNecessary();
 
-  // Sets the selected/enabled tracks if they aren't set before we initially
-  // transition to kHaveMetadata.
-  void SelectInitialTracksIfNecessary();
-
   void SetNetworkState(NetworkState);
 
   void AudioTracksTimerFired(TimerBase*);

third_party/blink/renderer/core/html/media/html_media_element_test.cc

@@ -12,6 +12,8 @@
 #include "third_party/blink/renderer/core/html/media/html_audio_element.h"
 #include "third_party/blink/renderer/core/html/media/html_video_element.h"
 #include "third_party/blink/renderer/core/html/media/media_error.h"
+#include "third_party/blink/renderer/core/html/track/audio_track_list.h"
+#include "third_party/blink/renderer/core/html/track/video_track_list.h"
 #include "third_party/blink/renderer/core/loader/empty_clients.h"
 #include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
 #include "third_party/blink/renderer/platform/network/network_state_notifier.h"
@@ -26,8 +28,12 @@ namespace blink {
 
 class MockWebMediaPlayer : public EmptyWebMediaPlayer {
  public:
+  MOCK_CONST_METHOD0(HasAudio, bool());
+  MOCK_CONST_METHOD0(HasVideo, bool());
   MOCK_CONST_METHOD0(Duration, double());
   MOCK_CONST_METHOD0(CurrentTime, double());
+  MOCK_METHOD1(EnabledAudioTracksChanged, void(const WebVector<TrackId>&));
+  MOCK_METHOD1(SelectedVideoTrackChanged, void(TrackId*));
   MOCK_METHOD3(
       Load,
       WebMediaPlayer::LoadTiming(LoadType load_type,
@@ -69,7 +75,15 @@ class HTMLMediaElementTest : public testing::TestWithParam<MediaTestParam> {
 
     // Most tests do not care about this call, nor its return value. Those that
     // do will clear this expectation and set custom expectations/returns.
-    EXPECT_CALL(*MockMediaPlayer(), Load(_, _, _))
+    EXPECT_CALL(*mock_media_player, HasAudio())
+        .WillRepeatedly(testing::Return(true));
+    EXPECT_CALL(*mock_media_player, HasVideo())
+        .WillRepeatedly(testing::Return(true));
+    EXPECT_CALL(*mock_media_player, Duration())
+        .WillRepeatedly(testing::Return(1.0));
+    EXPECT_CALL(*mock_media_player, CurrentTime())
+        .WillRepeatedly(testing::Return(0));
+    EXPECT_CALL(*mock_media_player, Load(_, _, _))
         .Times(AnyNumber())
         .WillRepeatedly(Return(WebMediaPlayer::LoadTiming::kImmediate));
 
@@ -235,8 +249,6 @@ TEST_P(HTMLMediaElementTest, CouldPlayIfEnoughDataRespondsToEnded) {
   MockWebMediaPlayer* mock_wmpi =
       reinterpret_cast<MockWebMediaPlayer*>(Media()->GetWebMediaPlayer());
   EXPECT_NE(mock_wmpi, nullptr);
-  EXPECT_CALL(*mock_wmpi, Duration()).WillRepeatedly(testing::Return(1.0));
-  EXPECT_CALL(*mock_wmpi, CurrentTime()).WillRepeatedly(testing::Return(0));
   EXPECT_TRUE(CouldPlayIfEnoughData());
 
   // Playback can only end once the ready state is above kHaveMetadata.
@@ -262,8 +274,6 @@ TEST_P(HTMLMediaElementTest, CouldPlayIfEnoughDataRespondsToError) {
   MockWebMediaPlayer* mock_wmpi =
       reinterpret_cast<MockWebMediaPlayer*>(Media()->GetWebMediaPlayer());
   EXPECT_NE(mock_wmpi, nullptr);
-  EXPECT_CALL(*mock_wmpi, Duration()).WillRepeatedly(testing::Return(1.0));
-  EXPECT_CALL(*mock_wmpi, CurrentTime()).WillRepeatedly(testing::Return(0));
   EXPECT_TRUE(CouldPlayIfEnoughData());
 
   SetReadyState(HTMLMediaElement::kHaveMetadata);
@@ -419,4 +429,19 @@ TEST_P(HTMLMediaElementTest, ImmediateMediaPlayerLoadDoesDelayWindowLoadEvent) {
   EXPECT_TRUE(ShouldDelayLoadEvent());
 }
 
+TEST_P(HTMLMediaElementTest, DefaultTracksAreEnabled) {
+  // Default tracks should start enabled, not be created then enabled.
+  EXPECT_CALL(*MockMediaPlayer(), EnabledAudioTracksChanged(_)).Times(0);
+  EXPECT_CALL(*MockMediaPlayer(), SelectedVideoTrackChanged(_)).Times(0);
+
+  Media()->SetSrc(SrcSchemeToURL(TestURLScheme::kHttp));
+  test::RunPendingTasks();
+  SetReadyState(HTMLMediaElement::kHaveFutureData);
+
+  ASSERT_EQ(1u, Media()->audioTracks().length());
+  ASSERT_EQ(1u, Media()->videoTracks().length());
+  EXPECT_TRUE(Media()->audioTracks().AnonymousIndexedGetter(0)->enabled());
+  EXPECT_TRUE(Media()->videoTracks().AnonymousIndexedGetter(0)->selected());
+}
+
 }  // namespace blink