fix canplaythrough

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 97c5b7f87324732622838bd82182c5c15acdcffe..adf67e38b80e80c85d4607eab68e5d20d79db20d 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -215,6 +215,8 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       last_reported_memory_usage_(0),
       supports_save_(true),
       chunk_demuxer_(NULL),
+      buffered_data_source_host_(
+          base::Bind(&WebMediaPlayerImpl::OnProgress, AsWeakPtr())),
       url_index_(url_index),
       // Threaded compositing isn't enabled universally yet.
       compositor_task_runner_(params.compositor_task_runner()
@@ -845,19 +847,20 @@ bool WebMediaPlayerImpl::didLoadingProgress() {
   const bool data_progress = buffered_data_source_host_.DidLoadingProgress();
   const bool did_loading_progress = pipeline_progress || data_progress;
 
-  if (did_loading_progress &&
-      highest_ready_state_ < ReadyState::ReadyStateHaveFutureData) {
-    // Reset the preroll attempt clock.
-    preroll_attempt_pending_ = true;
-    preroll_attempt_start_time_ = base::TimeTicks();
-
-    // Clear any 'stale' flag and give the pipeline a chance to resume. If we
-    // are already resumed, this will cause |preroll_attempt_start_time_| to be
-    // set.
-    // TODO(sandersd): Should this be on the same stack? It might be surprising
-    // that didLoadingProgress() can synchronously change state.
-    delegate_->ClearStaleFlag(delegate_id_);
-    UpdatePlayState();
+  if (did_loading_progress) {
+    if (highest_ready_state_ < ReadyState::ReadyStateHaveFutureData) {

[DaleCurtis, 2017/04/06 22:51:40]

This whole block can actually be moved to OnProgress() now without the first
conditional.

[hubbe, 2017/04/07 19:35:51]

Done.

+      // Reset the preroll attempt clock.
+      preroll_attempt_pending_ = true;
+      preroll_attempt_start_time_ = base::TimeTicks();
+
+      // Clear any 'stale' flag and give the pipeline a chance to resume. If we
+      // are already resumed, this will cause |preroll_attempt_start_time_| to
+      // be set.
+      // TODO(sandersd): Should this be on the same stack? It might be
+      // surprising that didLoadingProgress() can synchronously change state.
+      delegate_->ClearStaleFlag(delegate_id_);
+      UpdatePlayState();
+    }
   }
 
   return did_loading_progress;
@@ -1274,6 +1277,22 @@ void WebMediaPlayerImpl::OnMetadata(PipelineMetadata metadata) {
   UpdatePlayState();
 }
 
+void WebMediaPlayerImpl::OnProgress() {
+  if (ready_state_ == ReadyState::ReadyStateHaveFutureData &&
+      CanPlayThrough()) {
+    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+  }
+}
+
+bool WebMediaPlayerImpl::CanPlayThrough() {
+  if (chunk_demuxer_)

[DaleCurtis, 2017/04/06 22:51:40]

Ternary?

[hubbe, 2017/04/07 19:35:51]

Makes the expression too long I think.
It's already long...

+    return true;
+  return buffered_data_source_host_.CanPlayThrough(
+      base::TimeDelta::FromSecondsD(currentTime()),
+      base::TimeDelta::FromSecondsD(duration()),
+      playback_rate_ == 0.0 ? 1.0 : playback_rate_);
+}
+
 void WebMediaPlayerImpl::OnBufferingStateChange(BufferingState state) {
   DVLOG(1) << __func__ << "(" << state << ")";
   DCHECK(main_task_runner_->BelongsToCurrentThread());
@@ -1293,9 +1312,11 @@ void WebMediaPlayerImpl::OnBufferingStateChange(BufferingState state) {
       RecordUnderflowDuration(base::TimeDelta());
     }
 
-    // TODO(chcunningham): Monitor playback position vs buffered. Potentially
-    // transition to HAVE_FUTURE_DATA here if not enough is buffered.
-    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+    if (CanPlayThrough()) {

[DaleCurtis, 2017/04/06 22:51:40]

drop one line {}

[hubbe, 2017/04/07 19:35:51]

How about ternary? :)

+      SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+    } else {
+      SetReadyState(WebMediaPlayer::ReadyStateHaveFutureData);
+    }
 
     // Let the DataSource know we have enough data. It may use this information
     // to release unused network connections.
@@ -1644,6 +1665,8 @@ void WebMediaPlayerImpl::NotifyDownloading(bool is_downloading) {
     SetNetworkState(WebMediaPlayer::NetworkStateIdle);
   else if (is_downloading && network_state_ == WebMediaPlayer::NetworkStateIdle)
     SetNetworkState(WebMediaPlayer::NetworkStateLoading);
+  if (ready_state_ == ReadyState::ReadyStateHaveFutureData && !is_downloading)

[DaleCurtis, 2017/04/06 22:51:40]

Don't think this function should call SetReadyState; since it's otherwise only
SetNetworkState. Can we just let this happen via the progress callback above?

[hubbe, 2017/04/07 19:35:51]

No, we won't get progress callbacks after we stop downloading...

+    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
   media_log_->AddEvent(
       media_log_->CreateBooleanEvent(MediaLogEvent::NETWORK_ACTIVITY_SET,
                                      "is_downloading_data", is_downloading));