[Video] Pause videos with no audio when in the background.

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 1696 matching lines @@
 
 #if defined(OS_ANDROID)  // WMPI_CAST
   bool is_remote = isRemote();
   bool is_streaming = false;
 #else
   bool is_remote = false;
   bool is_streaming = data_source_ && data_source_->IsStreaming();
 #endif
 
   bool is_suspended = pipeline_controller_.IsSuspended();
-  bool is_backgrounded = IsBackgroundedSuspendEnabled() && IsHidden();
+  bool is_backgrounded = IsHidden();
   PlayState state = UpdatePlayState_ComputePlayState(
       is_remote, is_streaming, is_suspended, is_backgrounded);
   SetDelegateState(state.delegate_state);
   SetMemoryReportingState(state.is_memory_reporting_enabled);
   SetSuspendState(state.is_suspended || pending_suspend_resume_cycle_);
 }
 
 void WebMediaPlayerImpl::SetDelegateState(DelegateState new_state) {
   if (!delegate_)
     return;
@@ skipping 102 matching lines @@
 
   // Background suspend is not enabled for audio-only players unless paused,
   // though in the case of audio-only the session should be kept.
   // Videos are not suspended if the user resumed the playback via the remote
   // controls earlier and it's still playing.
   bool is_backgrounded_video = is_backgrounded && have_metadata && hasVideo();
   bool can_play_backgrounded = is_backgrounded_video && !is_remote &&
                                hasAudio() && IsResumeBackgroundVideosEnabled();
   bool is_background_playing =
       delegate_ && delegate_->IsPlayingBackgroundVideo();
-  bool background_suspended = !is_streaming && is_backgrounded_video &&
+  // Video-only media should always be suspended when in background.
+  bool should_suspend_background_media =
+      IsBackgroundedSuspendEnabled() || !hasAudio();
+  bool background_suspended = should_suspend_background_media &&
+                              !is_streaming && is_backgrounded_video &&
                               !(can_play_backgrounded && is_background_playing);
   bool background_pause_suspended =
       !is_streaming && is_backgrounded && paused_ && have_future_data;
 
   // Idle suspension is allowed prior to have future data since there exist
   // mechanisms to exit the idle state when the player is capable of reaching
   // the have future data state; see didLoadingProgress().
   //
   // TODO(sandersd): Make the delegate suspend idle players immediately when
   // hidden.
@@ skipping 93 matching lines @@
            << ", Video: " << stats.video_memory_usage << ", DataSource: "
            << (data_source_ ? data_source_->GetMemoryUsage() : 0)
            << ", Demuxer: " << demuxer_memory_usage;
 
   const int64_t delta = current_memory_usage - last_reported_memory_usage_;
   last_reported_memory_usage_ = current_memory_usage;
   adjust_allocated_memory_cb_.Run(delta);
 }
 
 void WebMediaPlayerImpl::ScheduleIdlePauseTimer() {
-  // Only schedule the pause timer if we're playing and are suspended.
-  if (paused_ || !pipeline_controller_.IsSuspended())
+  // Only schedule the pause timer if we're playing, are suspended and
+  // have audio.
+  if (paused_ || !pipeline_controller_.IsSuspended() || !hasAudio())

[sandersd (OOO until July 31), 2016/12/15 01:09:47]

This doesn't look quite right to me. The purpose of the pause timer to to define
a threshold after which suspended media becomes paused, rather than resuming
automatically when foregrounded.

This seems to say that media without audio is always resumed automatically,
which is a rather broader statement than the CL description. If this change is
necessary to avoid breaking sites, it may mean that the pause timer was a bad
idea to begin with, or that a much more involved heuristic is required.

[whywhat, 2016/12/16 01:20:24]

Well, it might be - AFAIK it only affects Android (do we suspend videos that are
offscreen on desktop atm?) and if the video is an animation with no controls,
there's no way to resume it for the user once it's paused, no?

So autoresuming the silent videos seems natural to me and that's how it worked
with WMPA IIRC. That's probably how it should work on desktop as well.

[whywhat, 2016/12/16 20:09:37]

Dan, am I correct that the timer would only trigger for videos suspended in the
background? It seems like it doesn't trigger for the audible videos even on
Android since background video playback was added and it breaks pages when
triggered silent videos. Is there any other use for it? I'm hesitant to just
remove it assuming I don't know of all the use cases.

Maybe this should be a separate change as a bug fix for Android autoplay videos
(I'd even merge it to M56 as I suspect gfycat.com is broken there too).

     return;
 
 #if defined(OS_ANDROID)
   // Remote players will be suspended and locally paused.
   if (isRemote())
     return;
 #endif
 
   // Idle timeout chosen arbitrarily.
   background_pause_timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(5),
@@ skipping 13 matching lines @@
     watch_time_reporter_->OnShown();
 }
 
 bool WebMediaPlayerImpl::IsHidden() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   return delegate_ && delegate_->IsHidden();
 }
 
 }  // namespace media