Fire canplaythrough as soon as download defers to fix autoplay

media/base/download_rate_monitor_unittest.cc

Index: media/base/download_rate_monitor_unittest.cc
diff --git a/media/base/download_rate_monitor_unittest.cc b/media/base/download_rate_monitor_unittest.cc
index 321ad630e51c0011a8551386120f22f1846bb851..e41e3cdfcb64902852eb245a34c1b58e08ac837d 100644
--- a/media/base/download_rate_monitor_unittest.cc
+++ b/media/base/download_rate_monitor_unittest.cc
@@ -15,151 +15,211 @@ namespace media {
 
 class DownloadRateMonitorTest : public ::testing::Test {
  public:
-  DownloadRateMonitorTest() {
-    monitor_.set_total_bytes(kMediaSizeInBytes);
-  }
+  DownloadRateMonitorTest()
+      : canplaythrough_cb_(base::Bind(&DownloadRateMonitorTest::CanPlayThrough,
+                                      base::Unretained(this))) { }
 
   virtual ~DownloadRateMonitorTest() { }
 
   MOCK_METHOD0(CanPlayThrough, void());
 
  protected:
-  static const int kMediaSizeInBytes = 20 * 1024 * 1024;
-
-  // Simulates downloading of the media file. Packets are timed evenly in
-  // |ms_between_packets| intervals, starting at |starting_time|, which is
-  // number of seconds since unix epoch (Jan 1, 1970).
-  // Returns the number of bytes buffered in the media file after the
-  // network activity.
-  int SimulateNetwork(double starting_time,
-                      int starting_bytes,
-                      int bytes_per_packet,
-                      int ms_between_packets,
-                      int number_of_packets) {
-    int bytes_buffered = starting_bytes;
-    base::Time packet_time = base::Time::FromDoubleT(starting_time);
-
-    monitor_.SetNetworkActivity(true);
-    // Loop executes (number_of_packets + 1) times because a packet needs a
-    // starting and end point.
-    for (int i = 0; i < number_of_packets + 1; ++i) {
-      monitor_.SetBufferedBytes(bytes_buffered, packet_time);
-      packet_time += base::TimeDelta::FromMilliseconds(ms_between_packets);
-      bytes_buffered += bytes_per_packet;
+  static const int kMediaDuration = 120;
+  static const int kMediaBitrate = 1024 * 1024 * 8;
+  static const int kMediaByterate = kMediaBitrate / 8;
+  static const int kMediaSizeInBytes = kMediaDuration * kMediaByterate;
+  static const int kDeferThreshold = 30 * kMediaByterate;
+
+  // Simulates downloading |bytes_to_download| bytes of the media file, at
+  // |download_speed_in_bps| bytes per second.
+  void SimulateNetwork(int bytes_to_download, int download_speed_in_bps) {
+    int bytes_downloaded = 0;
+    while (bytes_downloaded < bytes_to_download &&
+           !data_source_.is_deferred()) {
+      int bytes_left_to_download = bytes_to_download - bytes_downloaded;
+      int packet_size = std::min(download_speed_in_bps, bytes_left_to_download);
+      time_elapsed_ += base::TimeDelta::FromMilliseconds(
+          1000 * packet_size / download_speed_in_bps);
+      data_source_.ReceiveData(
+          packet_size,time_elapsed_ + base::Time::UnixEpoch());

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

space after ,

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

Done.

+      bytes_downloaded += packet_size;
     }
-    monitor_.SetNetworkActivity(false);
-    return bytes_buffered;
   }
 
-  void StartMonitor(int bitrate) {
-    StartMonitor(bitrate, false, false);
+  void Initialize() {
+    Initialize(false, false);
   }
 
-  void StartMonitor(int bitrate, bool streaming, bool local_source) {
-    monitor_.Start(base::Bind(&DownloadRateMonitorTest::CanPlayThrough,
-                   base::Unretained(this)), bitrate, streaming, local_source);
+  void Initialize(bool streaming, bool local_source) {
+    data_source_.Initialize(
+        kMediaBitrate, streaming, local_source, canplaythrough_cb_);
   }
 
-  DownloadRateMonitor monitor_;
+  bool DownloadIsDeferred() {
+    return data_source_.is_deferred();
+  }
+
+  void SeekTo(int offset) {
+    data_source_.SeekTo(offset);
+  }
 
  private:
+  // Helper class to simulate a buffered data source.
+  class FakeDataSource {
+   public:
+    FakeDataSource()
+        : total_bytes_(kMediaSizeInBytes),
+          buffered_bytes_(0),
+          offset_(0),
+          defer_threshold_(kDeferThreshold),
+          is_downloading_data_(true) { }
+
+    bool is_deferred() { return !is_downloading_data_; }
+
+    void ReceiveData(int bytes_received, base::Time packet_time) {
+      CHECK(is_downloading_data_);
+      buffered_bytes_ += bytes_received;
+      // This simulates that the download is being deferred.
+      if (buffered_bytes_ >= defer_threshold_)
+        is_downloading_data_ = false;
+
+      // Update monitor's state.
+      monitor_.SetNetworkActivity(is_downloading_data_);
+      monitor_.set_total_bytes(total_bytes_);
+      monitor_.SetBufferedBytes(buffered_bytes_ + offset_, packet_time);
+    }
+
+    void Initialize(int bitrate, bool streaming, bool local_source,
+                    const base::Closure& canplaythrough_cb) {
+      monitor_.Start(canplaythrough_cb, bitrate, streaming, local_source);
+    }
+
+    void SeekTo(int byte_position) {
+      offset_ = byte_position;
+      // Simulate recreating the buffer after a seek.
+      buffered_bytes_ = 0;
+      is_downloading_data_ = true;

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

Should this trigger SetXXX calls?

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

I don't think so. When a seek occurs (to a location outside of the in-memory
buffer), BufferedDataSource recreates BufferedResourceLoader. This doesn't
trigger a Network Event, so the SetXXX calls shouldn't get called.

+    }
+
+   private:
+    DownloadRateMonitor monitor_;
+
+    // Size of the media file being downloaded, in bytes.
+    int total_bytes_;
+
+    // Number of bytes currently in buffer.
+    int buffered_bytes_;
+
+    // The byte offset into the file from which the download began.
+    int offset_;
+
+    // The size of |buffered_bytes_| at which downloading should defer.
+    int defer_threshold_;
+
+    // True if download is active (not deferred_, false otherwise.
+    bool is_downloading_data_;
+  };
+
+  FakeDataSource data_source_;
+  base::Closure canplaythrough_cb_;
+
+  // Amount of time elapsed while downloading data.
+  base::TimeDelta time_elapsed_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadRateMonitorTest);
 };
 
 TEST_F(DownloadRateMonitorTest, DownloadRateGreaterThanBitrate) {
-  static const int media_bitrate = 1024 * 1024 * 8;
-
   // Simulate downloading at double the media's bitrate.
-  StartMonitor(media_bitrate);
+  Initialize();
   EXPECT_CALL(*this, CanPlayThrough());
-  SimulateNetwork(1, 0, 2 * media_bitrate / 8, 1000, 10);
+  SimulateNetwork(20 * kMediaByterate, 2 * kMediaByterate);

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

It's a little weird seeing byte rates for both parameters here and below. It
makes it hard to tell which is the actual transfer rate. Perhaps use a
SecondsToBytes() helper function or a kOneSecondOfBytes?

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

Good idea. Done.

 }
 
-// If the user pauses and the pipeline stops downloading data, make sure the
-// DownloadRateMonitor understands that the download is not stalling.
-TEST_F(DownloadRateMonitorTest, DownloadRateGreaterThanBitrate_Pause) {
-  static const int media_bitrate = 1024 * 1024 * 8;
-  static const int download_byte_rate = 1.1 * media_bitrate / 8;
+TEST_F(DownloadRateMonitorTest, DownloadRateLessThanBitrate_Defer) {
+  Initialize();
 
-  // Start downloading faster than the media's bitrate.
-  StartMonitor(media_bitrate);
+  // Download slower than the media's bitrate, but buffer enough data such that
+  // the data source defers.
   EXPECT_CALL(*this, CanPlayThrough());
-  int buffered = SimulateNetwork(1, 0, download_byte_rate, 1000, 2);
-
-  // Then "pause" for 3 minutes and continue downloading at same rate.
-  SimulateNetwork(60 * 3, buffered, download_byte_rate, 1000, 4);
+  SimulateNetwork(kDeferThreshold, 0.3 * kMediaByterate);
+  CHECK(DownloadIsDeferred());
 }
 
 TEST_F(DownloadRateMonitorTest, DownloadRateGreaterThanBitrate_SeekForward) {
-  static const int media_bitrate = 1024 * 1024 * 8;
-  static const int download_byte_rate = 1.1 * media_bitrate / 8;
-
   // Start downloading faster than the media's bitrate.
-  EXPECT_CALL(*this, CanPlayThrough());
-  StartMonitor(media_bitrate);
-  SimulateNetwork(1, 0, download_byte_rate, 1000, 2);
+  Initialize();
+  SimulateNetwork(3 * kMediaByterate, 1.1 * kMediaByterate);
 
   // Then seek forward mid-file and continue downloading at same rate.
-  SimulateNetwork(4, kMediaSizeInBytes / 2, download_byte_rate, 1000, 4);
+  SeekTo(kMediaSizeInBytes / 2);
+  EXPECT_CALL(*this, CanPlayThrough());
+  SimulateNetwork(7 * kMediaByterate, 1.1 * kMediaByterate);
+
+  // CanPlayThrough should not be fired because of deferring.
+  CHECK(!DownloadIsDeferred());
 }
 
 TEST_F(DownloadRateMonitorTest, DownloadRateGreaterThanBitrate_SeekBackward) {
-  static const int media_bitrate = 1024 * 1024 * 8;
-  static const int download_byte_rate = 1.1 * media_bitrate / 8;
-
   // Start downloading faster than the media's bitrate, in middle of file.
-  StartMonitor(media_bitrate);
-  SimulateNetwork(1, kMediaSizeInBytes / 2, download_byte_rate, 1000, 2);
+  Initialize();
+  SeekTo(kMediaSizeInBytes / 2);
+  SimulateNetwork(3 * kMediaByterate, 1.1 * kMediaByterate);
 
   // Then seek back to beginning and continue downloading at same rate.
+  SeekTo(0);
   EXPECT_CALL(*this, CanPlayThrough());
-  SimulateNetwork(4, 0, download_byte_rate, 1000, 4);
-}
+  SimulateNetwork(7 * kMediaByterate, 1.1 * kMediaByterate);
 
-TEST_F(DownloadRateMonitorTest, DownloadRateLessThanBitrate) {
-  static const int media_bitrate = 1024 * 1024 * 8;
+  // CanPlayThrough should not be fired because of deferring.
+  CHECK(!DownloadIsDeferred());
+}
 
+TEST_F(DownloadRateMonitorTest, DownloadRateLessThanByterate) {
   // Simulate downloading at half the media's bitrate.
   EXPECT_CALL(*this, CanPlayThrough())
       .Times(0);
-  StartMonitor(media_bitrate);
-  SimulateNetwork(1, 0, media_bitrate / 8 / 2, 1000, 10);
+  Initialize();
+  SimulateNetwork(10 * kMediaByterate, kMediaByterate / 2);
 }
 
 TEST_F(DownloadRateMonitorTest, MediaSourceIsLocal) {
-  static const int media_bitrate = 1024 * 1024 * 8;
-
   // Simulate no data downloaded.
   EXPECT_CALL(*this, CanPlayThrough());
-  StartMonitor(media_bitrate, false, true);
+  Initialize(false, true);
 }
 
 TEST_F(DownloadRateMonitorTest, MediaSourceIsStreaming) {
-  static const int media_bitrate = 1024 * 1024 * 8;
-
   // Simulate downloading at the media's bitrate while streaming.
   EXPECT_CALL(*this, CanPlayThrough());
-  StartMonitor(media_bitrate, true, false);
-  SimulateNetwork(1, 0, media_bitrate / 8, 1000, 10);
+  Initialize(true, false);
+  SimulateNetwork(10 * kMediaByterate, kMediaByterate);
 }
 
 TEST_F(DownloadRateMonitorTest, VeryFastDownloadRate) {
-  static const int media_bitrate = 1024 * 1024 * 8;
-
   // Simulate downloading half the video very quickly in one chunk.
-  StartMonitor(media_bitrate);
+  Initialize();
   EXPECT_CALL(*this, CanPlayThrough());
-  SimulateNetwork(1, 0, kMediaSizeInBytes / 2, 10, 1);
+  SimulateNetwork(kMediaSizeInBytes / 2, kMediaSizeInBytes * 10);
 }
 
 TEST_F(DownloadRateMonitorTest, DownloadEntireVideo) {
-  static const int seconds_of_data = 20;
-  static const int media_bitrate = kMediaSizeInBytes * 8 / seconds_of_data;
-
   // Simulate downloading entire video at half the bitrate of the video.
-  StartMonitor(media_bitrate);
+  Initialize();
   EXPECT_CALL(*this, CanPlayThrough());
-  SimulateNetwork(1, 0, media_bitrate / 8 / 2, 1000, seconds_of_data * 2);
+  SimulateNetwork(kMediaSizeInBytes, kMediaByterate / 2);
+}
+
+TEST_F(DownloadRateMonitorTest, DownloadEndOfVideo) {
+  Initialize();
+  // Seek to 10s before the end of the file, then download the remainder of the
+  // file at half the bitrate.
+  SeekTo((kMediaDuration - 10) * kMediaByterate);
+  EXPECT_CALL(*this, CanPlayThrough());
+  SimulateNetwork(10 * kMediaByterate, kMediaByterate/ 2);
+
+  // CanPlayThrough should not be fired because of deferring.

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

I think this comment is a little confusing. CanPlayThrough does fire right?
Perhaps something like "Verify deferring is not what caused CanPlayThrough to
fire"?

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

GOOD POINT! Haha I was reading it in my head with different intonation! Changed
here and elsewhere :)

+  CHECK(!DownloadIsDeferred());
 }
 
 }  // namespace media