Fire canplaythrough as soon as download defers to fix autoplay

media/base/download_rate_monitor_unittest.cc

 // Copyright (c) 2011 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/base/download_rate_monitor.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 25 matching lines @@
                       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);

[acolwell GONE FROM CHROMIUM, 2012/01/05 22:33:56]

Consider creating a helper object that models how the XXXDataSource make calls
on host() under different scenarios. That might make it easier to describe
various things like deferral behavior and help to ensure we have everything
covered.

Obviously this only makes sense if a relatively simple model object can be
created.

[vrk (LEFT CHROMIUM), 2012/01/09 19:01:18]

Made two objects, a FakeFilterHost and FakeDataSource. The test much more
accurately reflects what actually happens in the code now, and I think it's
still pretty readable. Let me know what you think.

       packet_time += base::TimeDelta::FromMilliseconds(ms_between_packets);
       bytes_buffered += bytes_per_packet;
     }
-    monitor_.SetNetworkActivity(false);
     return bytes_buffered;
   }
 
   void StartMonitor(int bitrate) {
     StartMonitor(bitrate, false, false);
   }
 
   void StartMonitor(int bitrate, bool streaming, bool local_source) {
     monitor_.Start(base::Bind(&DownloadRateMonitorTest::CanPlayThrough,
                    base::Unretained(this)), bitrate, streaming, local_source);
   }
 
   DownloadRateMonitor monitor_;
 
  private:
   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);
   EXPECT_CALL(*this, CanPlayThrough());
   SimulateNetwork(1, 0, 2 * media_bitrate / 8, 1000, 10);
 }
 
-// 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) {

[acolwell GONE FROM CHROMIUM, 2012/01/05 22:33:56]

Based on what the comment sayd, I think this test case is still valid and we
should keep it.

[vrk (LEFT CHROMIUM), 2012/01/09 19:01:18]

Ahhh, now I remember why I deleted this. The test is trying to catch the case
where the video is in a deferred state for a long time, and to make sure a stall
in download due to deferring doesn't hinder download speed.

However, now the DownloadRateMonitor will call CanPlayThrough as soon as you
defer, so the test doesn't really make sense anymore -- even if the download
speed estimation was flawed, CanPlayThrough already fired so we don't need the
download speed estimation anymore.

+TEST_F(DownloadRateMonitorTest, DownloadRateGreaterThanBitrate_Defer) {
   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.
   StartMonitor(media_bitrate);
+  SimulateNetwork(1, 0, download_byte_rate, 1000, 2);
+
+  // Now defer download because the client has decided enough data has been
+  // buffered.
   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);
+  monitor_.SetNetworkActivity(false);
 }
 
 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);
@@ skipping 55 matching lines @@
   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);
   EXPECT_CALL(*this, CanPlayThrough());
   SimulateNetwork(1, 0, media_bitrate / 8 / 2, 1000, seconds_of_data * 2);
 }
 
 }  // namespace media