Allow suspension prior to reaching kHaveFutureData.

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 208 matching lines @@
 #if defined(OS_ANDROID)  // WMPI_CAST
       cast_impl_(this, client_, params.context_3d_cb()),
 #endif
       volume_(1.0),
       volume_multiplier_(1.0),
       renderer_factory_(std::move(renderer_factory)),
       surface_manager_(params.surface_manager()),
       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_);
 
   force_video_overlays_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kForceVideoOverlays);
 
   if (delegate_)
     delegate_id_ = delegate_->AddObserver(this);
 
@@ skipping 341 matching lines @@
   buffering_strategy_ = MultibufferDataSource::BUFFERING_STRATEGY_NORMAL;
 #else
   buffering_strategy_ =
       static_cast<MultibufferDataSource::BufferingStrategy>(buffering_strategy);
 #endif
 
   if (data_source_)
     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());
 
   return pipeline_metadata_.has_video;
 }
 
 bool WebMediaPlayerImpl::hasAudio() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   return pipeline_metadata_.has_audio;
@@ skipping 1031 matching lines @@
   // errors.
   bool has_error = IsNetworkStateError(network_state_);
 
   // After HaveMetadata, we know which tracks are present and the duration.
   bool have_metadata = ready_state_ >= WebMediaPlayer::ReadyStateHaveMetadata;
 
   // After HaveFutureData, Blink will call play() if the state is not paused.
   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
   // 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_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
   // state. There could be other theoretical problems if the page is waiting for
   // other events before actually calling play(), but at least we don't break
   // Blink.
   //
   // 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
   // case. For memory usage reporting, we just use the same definition (but we
   // don't have to).
   //
   // Similarly, we don't consider |ended_| to be paused. Blink will immediately
   // call pause() or seek(), so |ended_| should not affect the computation.
   // Despite that, |ended_| does result in a separate paused state, to simplfy
   // the contract for SetDelegateState().
@@ skipping 106 matching lines @@
       pipeline_metadata_.natural_size,
       base::Bind(&GetCurrentTimeInternal, this)));
   watch_time_reporter_->OnVolumeChange(volume_);
   if (delegate_ && delegate_->IsHidden())
     watch_time_reporter_->OnHidden();
   else
     watch_time_reporter_->OnShown();
 }
 
 }  // namespace media