[Media, Video] Enable the video track for a new renderer.

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 4b8151a4076ce48a68c5b93fd2d18e267bb3d20f..cd6cb97eec97fb239b315242c22ade1aed32601c 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -200,6 +200,7 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
                      base::Unretained(this)),
           base::Bind(&WebMediaPlayerImpl::OnPipelineSeeked, AsWeakPtr()),
           base::Bind(&WebMediaPlayerImpl::OnPipelineSuspended, AsWeakPtr()),
+          base::Bind(&WebMediaPlayerImpl::OnPipelineResumed, AsWeakPtr()),
           base::Bind(&WebMediaPlayerImpl::OnError, AsWeakPtr())),
       load_type_(LoadTypeURL),
       opaque_(false),
@@ -474,6 +475,19 @@ void WebMediaPlayerImpl::pause() {
   DCHECK(watch_time_reporter_);
   watch_time_reporter_->OnPaused();
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::PAUSE));
+
+  // Enable the background video track back so that when the new renderer is
+  // initialized upon resume, it registers itself with the video track.
+  // TODO(avayvod): Remove when disabling video tracks works correctly. See
+  //                https://crbug.com/678374.
+  if (video_track_disabled_) {

[sandersd (OOO until July 31), 2017/01/09 20:12:49]

I think this would be better in an OnBeforeResume() callback, it's not trivially
obvious that doing it in pause() is always enough.

[whywhat, 2017/01/10 01:04:48]

Sure. I guess the fix would be reduced to enabling the video track back in
OnBeforeResume (OnResuming? OnResumeRequested?) and disabling in OnResumed()

+    video_track_disabled_ = false;
+    if (client_->hasSelectedVideoTrack()) {
+      WebMediaPlayer::TrackId trackId = client_->getSelectedVideoTrackId();
+      selectedVideoTrackChanged(&trackId);
+    }
+  }
+
   UpdatePlayState();
 }
 
@@ -671,9 +685,7 @@ void WebMediaPlayerImpl::selectedVideoTrackChanged(
 
   std::ostringstream logstr;
   std::vector<MediaTrack::Id> selectedVideoMediaTrackId;
-  bool canAddVideoTrack =
-      !IsBackgroundVideoTrackOptimizationEnabled() || !IsHidden();
-  if (selectedTrackId && canAddVideoTrack) {
+  if (selectedTrackId && !video_track_disabled_) {
     selectedVideoMediaTrackId.push_back(selectedTrackId->utf8().data());
     logstr << selectedVideoMediaTrackId[0];
   }
@@ -1060,6 +1072,7 @@ void WebMediaPlayerImpl::OnCdmAttached(bool success) {
 void WebMediaPlayerImpl::OnPipelineSeeked(bool time_updated) {
   seeking_ = false;
   seek_time_ = base::TimeDelta();
+
   if (paused_) {
 #if defined(OS_ANDROID)  // WMPI_CAST
     if (isRemote()) {
@@ -1107,6 +1120,18 @@ void WebMediaPlayerImpl::OnPipelineSuspended() {
   }
 }
 
+void WebMediaPlayerImpl::OnPipelineResumed() {
+  // Disable video track if we start playing the video in the background and
+  // haven't disabled the decoder yet.
+  // TODO(avayvod): Remove this when disabling and enabling video tracks in
+  // non-playing state works correctly. See https://crbug.com/678374.
+  if (IsBackgroundVideoTrackOptimizationEnabled() && IsHidden() &&
+      !video_track_disabled_) {
+    video_track_disabled_ = true;
+    selectedVideoTrackChanged(nullptr);
+  }
+}
+
 void WebMediaPlayerImpl::OnDemuxerOpened() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   client_->mediaSourceOpened(
@@ -1357,12 +1382,20 @@ void WebMediaPlayerImpl::OnHidden() {
       paused_when_hidden_ = true;
       return;
     }
-
-    selectedVideoTrackChanged(nullptr);
   }
 
   UpdatePlayState();
 
+  // Only disable video tracks for background videos when they're playing.
+  // Otherwise the video track will get detached from the renderer.
+  // TODO(avayvod): Remove extra check once this is no longer happening.
+  //                See https://crbug.com/678374.
+  if (IsBackgroundVideoTrackOptimizationEnabled() &&
+      delegate_state_ == DelegateState::PLAYING) {

[sandersd (OOO until July 31), 2017/01/09 20:12:49]

Is |delegate_state_| the best thing to check for this? Do we even need this with
OnBeforeResume()?

[whywhat, 2017/01/10 01:04:48]

Will try. I saw this member is gone in your refactor :)

+    video_track_disabled_ = true;
+    selectedVideoTrackChanged(nullptr);
+  }
+
   // Schedule suspended playing media to be paused if the user doesn't come back
   // to it within some timeout period to avoid any autoplay surprises.
   ScheduleIdlePauseTimer();
@@ -1378,22 +1411,23 @@ void WebMediaPlayerImpl::OnShown() {
       base::Bind(&VideoFrameCompositor::SetForegroundTime,
                  base::Unretained(compositor_), base::TimeTicks::Now()));
 
-  if (IsBackgroundVideoTrackOptimizationEnabled()) {
-    if (paused_when_hidden_) {
-      paused_when_hidden_ = false;
-      OnPlay();  // Calls UpdatePlayState() so return afterwards.
-      return;
-    }
+  must_suspend_ = false;
+  background_pause_timer_.Stop();
+
+  if (paused_when_hidden_) {
+    paused_when_hidden_ = false;
+    OnPlay();  // Calls UpdatePlayState() so return afterwards.
+    return;
+  }
 
+  if (video_track_disabled_) {
+    video_track_disabled_ = false;
     if (client_->hasSelectedVideoTrack()) {
       WebMediaPlayer::TrackId trackId = client_->getSelectedVideoTrackId();
       selectedVideoTrackChanged(&trackId);
     }
   }
 
-  must_suspend_ = false;
-  background_pause_timer_.Stop();
-
   UpdatePlayState();
 }