Fire canplaythrough as soon as download defers to fix autoplay

media/base/download_rate_monitor.cc

Index: media/base/download_rate_monitor.cc
diff --git a/media/base/download_rate_monitor.cc b/media/base/download_rate_monitor.cc
index f27b2e8059223c6425ba1f3b5c0651e5e19e59d5..9fedd7bd74c36fc68eda40fabcf5772447a5f4b6 100644
--- a/media/base/download_rate_monitor.cc
+++ b/media/base/download_rate_monitor.cc
@@ -91,8 +91,6 @@ void DownloadRateMonitor::SetBufferedBytes(
   if (stopped_)
     return;
 
-  is_downloading_data_ = true;
-
   // Check monotonically nondecreasing constraint.
   base::Time previous_time;
   if (!current_sample_.is_null())
@@ -120,12 +118,18 @@ void DownloadRateMonitor::SetBufferedBytes(
 }
 
 void DownloadRateMonitor::SetNetworkActivity(bool is_downloading_data) {
-  if (is_downloading_data == is_downloading_data_)
-    return;
   // Invalidate the current sample if downloading is going from start to stopped
   // or vice versa.
-  current_sample_.Reset();
-  is_downloading_data_ = is_downloading_data;
+  if (is_downloading_data != is_downloading_data_) {
+    current_sample_.Reset();
+    is_downloading_data_ = is_downloading_data;
+  }
+
+  // Record when download defers for the first time.
+  if (!is_downloading_data && !has_deferred_) {

[acolwell GONE FROM CHROMIUM, 2012/01/11 18:51:01]

Is it possible for this method to get called with is_downloading_data == false
before we get a call with is_downloading_data == true? If so I think we might
accidentally trigger this code. 

Perhaps put this in the section above with a (is_downloading_data_ &&
!is_downloading_data && !has_deferred_) ?

[vrk (LEFT CHROMIUM), 2012/01/13 01:22:49]

Yes, on very short files or very fast downloads, where the download defers as a
result of the first call to didReceiveData.
Actually, I don't think it's wrong to trigger this code in this case.
SetNetworkActivity() will always be called after data has started downloading --
it wouldn't make sense for the BufferedResourceLoader to "defer" if it wasn't
already downloading.

But now that you mention it, I think the code *above* this block is unnecessary.
Since we're going to trigger CanPlayThrough when downloading |has_deferred_|
regardless, there's no need to keep track of the transitions between the state
of |is_downloading_data_|. 

I deleted this code and the |is_downloading_data_| field.

+    has_deferred_ = true;
+    NotifyCanPlayThroughIfNeeded();
+  }
 }
 
 void DownloadRateMonitor::Stop() {
@@ -146,6 +150,7 @@ void DownloadRateMonitor::Reset() {
   bitrate_ = 0;
   stopped_ = true;
   streaming_ = false;
+  has_deferred_ = false;
 }
 
 DownloadRateMonitor::~DownloadRateMonitor() { }
@@ -210,8 +215,9 @@ bool DownloadRateMonitor::ShouldNotifyCanPlayThrough() {
   if (local_source_ || streaming_)
     return true;
 
-  // If all bytes are buffered, fire CanPlayThrough.
-  if (buffered_bytes_ == total_bytes_)
+  // If all bytes are buffered or if enough bytes were buffered such that
+  // downloading has deferred, fire CanPlayThrough.
+  if (buffered_bytes_ == total_bytes_ || has_deferred_)
     return true;
 
   // If bitrate is unknown, optimistically fire CanPlayThrough immediately.
@@ -228,12 +234,9 @@ bool DownloadRateMonitor::ShouldNotifyCanPlayThrough() {
   if (download_rate > 0)
     return download_rate >= bytes_needed_per_second;
 
-  // If download rate is unknown, it may be because the media is being
-  // downloaded so fast that it cannot collect an adequate number of samples
-  // before the download gets deferred.
-  //
-  // To catch this case, we also look at how much data is being downloaded
-  // immediately after the download begins.
+  // With very fast connections, we may want to fire CanPlayThrough before
+  // waiting for the sample window size to reach |kNumberOfSamples|. Check for
+  // this scenario.
   if (sample_window_.size() < kNumberOfSamples) {
     int64 bytes_downloaded_since_start =
         bytes_downloaded_in_window() + current_sample_.bytes_downloaded();