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..429f1ad89f889e0d61b3ea2750a5a38f9ad0599a 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,24 @@ 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) {
+      // 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 (ready_state_ == ReadyState::ReadyStateHaveFutureData &&
+        CanPlayThrough()) {
+      SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+    }
   }
 
   return did_loading_progress;
@@ -1274,6 +1281,20 @@ void WebMediaPlayerImpl::OnMetadata(PipelineMetadata metadata) {
   UpdatePlayState();
 }
 
+void WebMediaPlayerImpl::OnProgress() {
+  if (ready_state_ == ReadyState::ReadyStateHaveFutureData &&
+      CanPlayThrough()) {
+    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);

[chcunningham, 2017/04/06 16:55:41]

Do we need this logic both here and in didLoadingProgress? Can we just have this
on? I'm not a fan of how didLoadingProgress has sideffects.

[hubbe, 2017/04/06 21:25:38]

I can remove it from didLoadingProgress()

+  }
+}
+
+bool WebMediaPlayerImpl::CanPlayThrough() {
+  return buffered_data_source_host_.CanPlayThrough(

[chcunningham, 2017/04/06 16:55:41]

What about MSE (no buffered_data_source_host_)?

[hubbe, 2017/04/06 21:25:38]

There is always a buffered_data_source_host_, but of course it's not used. Added
code to make it behave as before for MSE.

+      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 +1314,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()) {
+      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 +1667,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)
+    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);

[mlamouri (slow - plz ping), 2017/04/06 11:32:56]

If I understand correctly, that means that we will advertise as being able to
play through if we have future data and not downloading? Why?

[hubbe, 2017/04/06 21:25:38]

That's what the standard says to do when the buffer is full.

[mlamouri (slow - plz ping), 2017/04/07 11:23:44]

FWIW, what I understood by reading the code is that we are no longer downloading
not that the buffer was full. The buffer being full makes sense but do we stop
downloading only if the buffer is full? Maybe add a comment if this can only
happen when the buffer is full?

   media_log_->AddEvent(
       media_log_->CreateBooleanEvent(MediaLogEvent::NETWORK_ACTIVITY_SET,
                                      "is_downloading_data", is_downloading));