[Video] Don't pause video-only elements if they're already paused...

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 1103 matching lines @@
   } else {
     DCHECK(watch_time_reporter_);
     watch_time_reporter_->OnPlaying();
   }
   if (time_updated)
     should_notify_time_changed_ = true;
 
   // Reset underflow count upon seek; this prevents looping videos and user
   // actions from artificially inflating the underflow count.
   underflow_count_ = 0;
+
+  // Background video optimizations are delayed when shown/hidden if pipeline
+  // is seeking.
+  UpdateBackgroundVideoOptimizationState();
 }
 
 void WebMediaPlayerImpl::OnPipelineSuspended() {
 #if defined(OS_ANDROID)
   if (isRemote()) {
     scoped_refptr<VideoFrame> frame = cast_impl_.GetCastingBanner();
     if (frame)
       compositor_->PaintSingleFrame(frame);
   }
 #endif
@@ skipping 19 matching lines @@
   // renderer will attach its callbacks to the video stream properly.
   // TODO(avayvod): Remove this when disabling and enabling video tracks in
   // non-playing state works correctly. See https://crbug.com/678374.
   EnableVideoTrackIfNeeded();
   is_pipeline_resuming_ = true;
 }
 
 void WebMediaPlayerImpl::OnPipelineResumed() {
   is_pipeline_resuming_ = false;
 
-  if (IsHidden()) {
-    DisableVideoTrackIfNeeded();
-  } else {
-    EnableVideoTrackIfNeeded();
-  }
+  UpdateBackgroundVideoOptimizationState();
 }
 
 void WebMediaPlayerImpl::OnDemuxerOpened() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   client_->mediaSourceOpened(
       new WebMediaSourceImpl(chunk_demuxer_, media_log_));
 }
 
 void WebMediaPlayerImpl::OnError(PipelineStatus status) {
   DVLOG(1) << __func__;
@@ skipping 225 matching lines @@
   if (video_weblayer_)
     video_weblayer_->layer()->SetContentsOpaque(opaque_);
 }
 
 void WebMediaPlayerImpl::OnFrameHidden() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   if (watch_time_reporter_)
     watch_time_reporter_->OnHidden();
 
   if (ShouldPauseVideoWhenHidden()) {

[sandersd (OOO until July 31), 2017/01/20 22:57:00]

Should just call UpdateBackgroundVideoOptimizationState().

if (IsHidden())
  UpdateBackgroundVideoOptimizationState();

is a reasonable structure if we are being pedantic, but if anything it would
make more sense to use IsFrameHidden() there instead of OnHidden(); I don't
think we want undo the optimization on frame close.

[whywhat, 2017/01/20 23:24:45]

Hm, here we already know we're hidden.
We also want to return early in the pause case since UpdatePlayState is already
called.
I guess UpdateBVO could return a bool and we'd return if it's true.

[sandersd (OOO until July 31), 2017/01/20 23:29:42]

Ah I see, I missed the early return when comparing the two blocks of code. It's
not a big deal to run UpdatePlayState() twice, but this duplicate code isn't
going to make refactoring any harder.

I do want to point out that we don't actually know for sure here whether
IsHidden() is true. I don't think it's possible to get OnFrameHidden() after
OnFrameClosed(), but I don't think the API explicitly makes that guarantee.

-    if (!paused_when_hidden_) {
-      // OnPause() will set |paused_when_hidden_| to false and call
-      // UpdatePlayState(), so set the flag to true after and then return.
-      OnPause();
-      paused_when_hidden_ = true;
-      return;
-    }
+    PauseVideoIfNeeded();
+    return;
   } else {
     DisableVideoTrackIfNeeded();
   }
 
   UpdatePlayState();
 
   // 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();
 }
@@ skipping 744 matching lines @@
   base::TimeDelta duration = GetPipelineMediaDuration();
   if (duration < max_keyframe_distance_to_disable_background_video_)
     return true;
 
   // Otherwise, only optimize videos with shorter average keyframe distance.
   PipelineStatistics stats = GetPipelineStatistics();
   return stats.video_keyframe_distance_average <
          max_keyframe_distance_to_disable_background_video_;
 }
 
+void WebMediaPlayerImpl::UpdateBackgroundVideoOptimizationState() {
+  if (IsHidden()) {
+    if (ShouldPauseVideoWhenHidden())
+      PauseVideoIfNeeded();
+    else
+      DisableVideoTrackIfNeeded();
+  } else {
+    EnableVideoTrackIfNeeded();
+  }
+}
+
+void WebMediaPlayerImpl::PauseVideoIfNeeded() {
+  DCHECK(IsHidden());
+
+  // Don't pause video while the pipeline is stopped, resuming or seeking.
+  // Also if the video is paused already.
+  if (!pipeline_.IsRunning() || is_pipeline_resuming_ || seeking_ || paused_)
+    return;
+
+  // OnPause() will set |paused_when_hidden_| to false and call
+  // UpdatePlayState(), so set the flag to true after and then return.
+  OnPause();
+  paused_when_hidden_ = true;
+}
+
 void WebMediaPlayerImpl::EnableVideoTrackIfNeeded() {
-  // Don't change video track while the pipeline is resuming or seeking.
-  if (is_pipeline_resuming_ || seeking_)
+  // Don't change video track while the pipeline is stopped, resuming or
+  // seeking.
+  if (!pipeline_.IsRunning() || is_pipeline_resuming_ || seeking_)
     return;
 
   if (video_track_disabled_) {
     video_track_disabled_ = false;
     if (client_->hasSelectedVideoTrack()) {
       WebMediaPlayer::TrackId trackId = client_->getSelectedVideoTrackId();
       selectedVideoTrackChanged(&trackId);
     }
   }
 }
@@ skipping 42 matching lines @@
     UMA_HISTOGRAM_TIMES(
         "Media.Video.TimeFromForegroundToFirstFrame.DisableTrack",
         time_to_first_frame);
   } else {
     UMA_HISTOGRAM_TIMES("Media.Video.TimeFromForegroundToFirstFrame.Paused",
                         time_to_first_frame);
   }
 }
 
 }  // namespace media