Allow suspension prior to reaching kHaveFutureData.

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 3389d4a33706376b74273ac2d14951542df66f04..8cc7c905a102c4769c9d5eebd42c99f901d818ee 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -226,7 +226,8 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       overlay_surface_id_(SurfaceManager::kNoSurfaceID),
       suppress_destruction_errors_(false),
       can_suspend_state_(CanSuspendState::UNKNOWN),
-      is_encrypted_(false) {
+      is_encrypted_(false),
+      client_playback_state_(PlaybackState::Paused) {
   DCHECK(!adjust_allocated_memory_cb_.is_null());
   DCHECK(renderer_factory_);
   DCHECK(client_);
@@ -588,6 +589,17 @@ void WebMediaPlayerImpl::setBufferingStrategy(
     data_source_->SetBufferingStrategy(buffering_strategy_);
 }
 
+void WebMediaPlayerImpl::setPlaybackState(
+    WebMediaPlayer::PlaybackState playback_state) {
+  client_playback_state_ = playback_state;
+  if (highest_ready_state_ >= WebMediaPlayer::ReadyStateHaveFutureData) {
+    DCHECK_EQ(client_playback_state_ == WebMediaPlayer::PlaybackState::Paused,
+              paused_);
+  }
+
+  UpdatePlayState();
+}
+
 bool WebMediaPlayerImpl::hasVideo() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
@@ -1639,6 +1651,12 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
   bool have_future_data =
       highest_ready_state_ >= WebMediaPlayer::ReadyStateHaveFutureData;
 
+  // Once we reach HaveFutureData, our internal paused state should be accurate,
+  // due to ordering of calls we may temporarily be out of sync though.

[sandersd (OOO until July 31), 2016/08/30 18:40:17]

It's worth explicitly describing this as retaining the paused state in that
interim time.

I believe though that readyStateChanged() is re-entrant, so this won't happen in
practice.

[DaleCurtis, 2016/08/30 21:56:04]

This does happen, HTMLMediaElement::UpdatePlayState() first calls pause() then
calls WMP::UpdatePlaybackState().

+  bool is_paused =
+      have_future_data ? paused_ : client_playback_state_ ==
+                                       WebMediaPlayer::PlaybackState::Paused;
+
   // 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
@@ -1650,10 +1668,7 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
       delegate_ && delegate_->IsPlayingBackgroundVideo();
   bool background_suspended = is_backgrounded_video &&
                               !(can_play_backgrounded && is_background_playing);
-
-  // The |paused_| state is not reliable until we |have_future_data|.
-  bool background_pause_suspended =
-      is_backgrounded && have_future_data && paused_;
+  bool background_pause_suspended = is_backgrounded && is_paused;
 
   // Idle suspend is enabled once there is future data. We don't want to idle
   // suspend before that because play() may never be triggered to leave the idle
@@ -1663,16 +1678,14 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote,
   //
   // TODO(sandersd): Make the delegate suspend idle players immediately when
   // hidden.
-  // TODO(sandersd): If Blink told us the paused state sooner, we could
-  // idle suspend sooner.
-  bool idle_suspended = is_idle_ && have_future_data;
+  bool idle_suspended = is_idle_ && is_paused;
 
   // Combined suspend state.
   result.is_suspended =
       is_remote || must_suspend_ || idle_suspended || background_suspended ||
       background_pause_suspended ||
       // If we're already suspended, see if we can wait for user interaction.
-      (is_suspended && have_future_data && paused_ && !seeking_);
+      (is_suspended && is_paused && !seeking_);

[sandersd (OOO until July 31), 2016/08/30 18:40:18]

While you're in here, can you pull this condition out into its own variable?

[DaleCurtis, 2016/08/30 21:56:04]

Done.

 
   // We do not treat |playback_rate_| == 0 as paused. For the media session,
   // being paused implies displaying a play button, which is incorrect in this