Numerous fixes to audio/video buffered resource loading.

This patch fixes a few related issues:
  1) Default loading strategy is now threshold-then-defer
  2) Specify more reasonable default bitrate/playback rate values
  3) Use a minimum buffer window size to prevent underflows on low bitrate content
  4) Remember bitrate/playback rate values between resource loaders

The default loading strategy of read-then-defer had a negative impact on initial latency as we were constantly deferring/undeferring the connection during the time when we need data the fastest.

While this change does result in loading a pinch more data for preload=metadata scenarios, it vastly improves the common preload=auto scenario.

BUG=99775
TEST=test_shell_tests

Review URL: http://codereview.chromium.org/8224028

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@105121 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 352 insertions, 229 deletions

diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index 4f90442..407a73a 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -61,9 +61,11 @@ BufferedDataSource::BufferedDataSource(
       stopped_on_render_loop_(false),
       media_is_paused_(true),
       media_has_played_(false),
-      preload_(media::METADATA),
+      preload_(media::AUTO),
       using_range_request_(true),
       cache_miss_retries_left_(kNumCacheMissRetries),
+      bitrate_(0),
+      playback_rate_(0.0),
       media_log_(media_log) {
 }
 
@@ -79,6 +81,9 @@ BufferedResourceLoader* BufferedDataSource::CreateResourceLoader(
   return new BufferedResourceLoader(url_,
                                     first_byte_position,
                                     last_byte_position,
+                                    ChooseDeferStrategy(),
+                                    bitrate_,
+                                    playback_rate_,
                                     media_log_);
 }
 
@@ -304,8 +309,6 @@ void BufferedDataSource::RestartLoadingTask() {
   }
 
   loader_ = CreateResourceLoader(read_position_, kPositionNotSpecified);
-  BufferedResourceLoader::DeferStrategy strategy = ChooseDeferStrategy();
-  loader_->UpdateDeferStrategy(strategy);
   loader_->Start(
       NewCallback(this, &BufferedDataSource::PartialReadStartCallback),
       base::Bind(&BufferedDataSource::NetworkEventCallback, this),
@@ -316,6 +319,7 @@ void BufferedDataSource::SetPlaybackRateTask(float playback_rate) {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
 
+  playback_rate_ = playback_rate;
   loader_->SetPlaybackRate(playback_rate);
 
   bool previously_paused = media_is_paused_;
@@ -336,28 +340,27 @@ void BufferedDataSource::SetPreloadTask(media::Preload preload) {
 void BufferedDataSource::SetBitrateTask(int bitrate) {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
+
+  bitrate_ = bitrate;
   loader_->SetBitrate(bitrate);
 }
 
 BufferedResourceLoader::DeferStrategy
 BufferedDataSource::ChooseDeferStrategy() {
   DCHECK(MessageLoop::current() == render_loop_);
-  // If the user indicates preload=metadata, then just load exactly
-  // what is needed for starting the pipeline and prerolling frames.
-  if (preload_ == media::METADATA && !media_has_played_)
+  // If the page indicated preload=metadata, then load exactly what is needed
+  // needed for starting playback.
+  if (!media_has_played_ && preload_ == media::METADATA)
     return BufferedResourceLoader::kReadThenDefer;
 
-  // In general, we want to try to buffer the entire video when the video
-  // is paused. But we don't want to do this if the video hasn't played yet
-  // and preload!=auto.
-  if (media_is_paused_ &&
-      (preload_ == media::AUTO || media_has_played_)) {
+  // If the playback has started (at which point the preload value is ignored)
+  // and we're paused, then try to load as much as possible.
+  if (media_has_played_ && media_is_paused_)
     return BufferedResourceLoader::kNeverDefer;
-  }
 
-  // When the video is playing, regardless of preload state, we buffer up
-  // to a hard limit and enable/disable deferring when the buffer is
-  // depleted/full.
+  // If media is currently playing or the page indicated preload=auto,
+  // use threshold strategy to enable/disable deferring when the buffer
+  // is full/depleted.
   return BufferedResourceLoader::kThresholdDefer;
 }
 
@@ -418,7 +421,6 @@ void BufferedDataSource::HttpInitialStartCallback(int error) {
   int64 instance_size = loader_->instance_size();
   bool success = error == net::OK;
 
-
   bool initialize_cb_is_null = false;
   {
     base::AutoLock auto_lock(lock_);
diff --git a/webkit/glue/media/buffered_data_source.h b/webkit/glue/media/buffered_data_source.h
index 48cde3a..b157d5e 100644
--- a/webkit/glue/media/buffered_data_source.h
+++ b/webkit/glue/media/buffered_data_source.h
@@ -67,6 +67,8 @@ class BufferedDataSource : public WebDataSource {
       int64 first_byte_position, int64 last_byte_position);
 
  private:
+  friend class BufferedDataSourceTest2;
+
   // Posted to perform initialization on render thread and start resource
   // loading.
   void InitializeTask();
@@ -215,6 +217,12 @@ class BufferedDataSource : public WebDataSource {
   // Number of cache miss retries left.
   int cache_miss_retries_left_;
 
+  // Bitrate of the content, 0 if unknown.
+  int bitrate_;
+
+  // Current playback rate.
+  float playback_rate_;
+
   scoped_refptr<media::MediaLog> media_log_;
 
   DISALLOW_COPY_AND_ASSIGN(BufferedDataSource);
diff --git a/webkit/glue/media/buffered_data_source_unittest.cc b/webkit/glue/media/buffered_data_source_unittest.cc
index 1a7c28c..1e29e2b 100644
--- a/webkit/glue/media/buffered_data_source_unittest.cc
+++ b/webkit/glue/media/buffered_data_source_unittest.cc
@@ -11,9 +11,12 @@
 #include "media/base/mock_filter_host.h"
 #include "media/base/mock_filters.h"
 #include "net/base/net_errors.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebURLResponse.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
 #include "webkit/glue/media/buffered_data_source.h"
 #include "webkit/mocks/mock_webframeclient.h"
+#include "webkit/mocks/mock_weburlloader.h"
 
 using ::testing::_;
 using ::testing::Assign;
@@ -32,6 +35,9 @@ using ::testing::NiceMock;
 using ::testing::WithArgs;
 
 using WebKit::WebFrame;
+using WebKit::WebString;
+using WebKit::WebURLError;
+using WebKit::WebURLResponse;
 using WebKit::WebView;
 
 namespace webkit_glue {
@@ -71,7 +77,8 @@ class MockBufferedDataSource : public BufferedDataSource {
 class MockBufferedResourceLoader : public BufferedResourceLoader {
  public:
   MockBufferedResourceLoader()
-      : BufferedResourceLoader(GURL(), 0, 0, new media::MediaLog()) {
+      : BufferedResourceLoader(GURL(), 0, 0, kThresholdDefer,
+                               0, 0, new media::MediaLog()) {
   }
 
   MOCK_METHOD3(Start, void(net::OldCompletionCallback* read_callback,
@@ -590,4 +597,192 @@ TEST_F(BufferedDataSourceTest, BoundedCacheMisses) {
   StopDataSource();
 }
 
+// TODO(scherkus): de-dupe from buffered_resource_loader_unittest.cc
+ACTION_P(RequestCanceled, loader) {
+  WebURLError error;
+  error.reason = net::ERR_ABORTED;
+  error.domain = WebString::fromUTF8(net::kErrorDomain);
+  loader->didFail(NULL, error);
+}
+
+// A more realistic BufferedDataSource that uses BufferedResourceLoader instead
+// of a mocked version but injects a MockWebURLLoader.
+//
+// TODO(scherkus): re-write these tests to use this class then drop the "2"
+// suffix.
+class MockBufferedDataSource2 : public BufferedDataSource {
+ public:
+  MockBufferedDataSource2(MessageLoop* message_loop, WebFrame* frame)
+      : BufferedDataSource(message_loop, frame, new media::MediaLog()),
+        url_loader_(NULL) {
+  }
+
+  virtual base::TimeDelta GetTimeoutMilliseconds() {
+    return base::TimeDelta::FromMilliseconds(
+                            TestTimeouts::tiny_timeout_ms());
+  }
+
+  virtual BufferedResourceLoader* CreateResourceLoader(int64 first_position,
+                                                       int64 last_position) {
+    loader_ = BufferedDataSource::CreateResourceLoader(first_position,
+                                                       last_position);
+
+    url_loader_ = new NiceMock<MockWebURLLoader>();
+    ON_CALL(*url_loader_, cancel())
+        .WillByDefault(RequestCanceled(loader_));
+
+    loader_->SetURLLoaderForTest(url_loader_);
+    return loader_;
+  }
+
+  const scoped_refptr<BufferedResourceLoader>& loader() { return loader_; }
+  NiceMock<MockWebURLLoader>* url_loader() { return url_loader_; }
+
+ private:
+  scoped_refptr<BufferedResourceLoader> loader_;
+  NiceMock<MockWebURLLoader>* url_loader_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockBufferedDataSource2);
+};
+
+class BufferedDataSourceTest2 : public testing::Test {
+ public:
+  BufferedDataSourceTest2()
+      : view_(WebView::create(NULL)),
+        message_loop_(MessageLoop::current()) {
+    view_->initializeMainFrame(&client_);
+  }
+
+  virtual ~BufferedDataSourceTest2() {
+    view_->close();
+  }
+
+  void InitializeDataSource(const char* url) {
+    gurl_ = GURL(url);
+
+    data_source_ = new MockBufferedDataSource2(message_loop_,
+                                               view_->mainFrame());
+    data_source_->set_host(&host_);
+    data_source_->Initialize(url,
+                             media::NewExpectedStatusCB(media::PIPELINE_OK));
+    message_loop_->RunAllPending();
+
+    // Simulate 206 response for a 5,000,000 byte length file.
+    WebURLResponse response(gurl_);
+    response.setHTTPHeaderField(WebString::fromUTF8("Accept-Ranges"),
+                                WebString::fromUTF8("bytes"));
+    response.setHTTPHeaderField(WebString::fromUTF8("Content-Range"),
+                                WebString::fromUTF8("bytes 0-4999999/5000000"));
+    response.setHTTPHeaderField(WebString::fromUTF8("Content-Length"),
+                                WebString::fromUTF8("5000000"));
+    response.setExpectedContentLength(5000000);
+    response.setHTTPStatusCode(206);
+
+    // We should receive corresponding information about the media resource.
+    EXPECT_CALL(host_, SetLoaded(false));
+    EXPECT_CALL(host_, SetTotalBytes(5000000));
+    EXPECT_CALL(host_, SetBufferedBytes(0));
+
+    data_source_->loader()->didReceiveResponse(data_source_->url_loader(),
+                                               response);
+
+    message_loop_->RunAllPending();
+  }
+
+  void StopDataSource() {
+    data_source_->Stop(media::NewExpectedClosure());
+    message_loop_->RunAllPending();
+  }
+
+  MOCK_METHOD1(ReadCallback, void(size_t size));
+  media::DataSource::ReadCallback NewReadCallback(size_t size) {
+    EXPECT_CALL(*this, ReadCallback(size));
+    return base::Bind(&BufferedDataSourceTest2::ReadCallback,
+                      base::Unretained(this));
+  }
+
+  // Accessors for private variables on |data_source_|.
+  media::Preload preload() { return data_source_->preload_; }
+  BufferedResourceLoader::DeferStrategy defer_strategy() {
+    return data_source_->loader()->defer_strategy_;
+  }
+  int data_source_bitrate() { return data_source_->bitrate_; }
+  int data_source_playback_rate() { return data_source_->playback_rate_; }
+  int loader_bitrate() { return data_source_->loader()->bitrate_; }
+  int loader_playback_rate() { return data_source_->loader()->playback_rate_; }
+
+  scoped_refptr<MockBufferedDataSource2> data_source_;
+
+  GURL gurl_;
+  MockWebFrameClient client_;
+  WebView* view_;
+
+  StrictMock<media::MockFilterHost> host_;
+  MessageLoop* message_loop_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BufferedDataSourceTest2);
+};
+
+TEST_F(BufferedDataSourceTest2, Default) {
+  InitializeDataSource("http://localhost/foo.webm");
+
+  // Ensure we have sane values for default loading scenario.
+  EXPECT_EQ(media::AUTO, preload());
+  EXPECT_EQ(BufferedResourceLoader::kThresholdDefer, defer_strategy());
+
+  EXPECT_EQ(0, data_source_bitrate());
+  EXPECT_EQ(0.0f, data_source_playback_rate());
+  EXPECT_EQ(0, loader_bitrate());
+  EXPECT_EQ(0.0f, loader_playback_rate());
+
+  StopDataSource();
+}
+
+TEST_F(BufferedDataSourceTest2, SetBitrate) {
+  InitializeDataSource("http://localhost/foo.webm");
+
+  data_source_->SetBitrate(1234);
+  message_loop_->RunAllPending();
+  EXPECT_EQ(1234, data_source_bitrate());
+  EXPECT_EQ(1234, loader_bitrate());
+
+  // Read so far ahead to cause the loader to get recreated.
+  BufferedResourceLoader* old_loader = data_source_->loader();
+
+  uint8 buffer[1024];
+  data_source_->Read(4000000, 1024, buffer,
+                     NewReadCallback(media::DataSource::kReadError));
+  message_loop_->RunAllPending();
+
+  // Verify loader changed but still has same bitrate.
+  EXPECT_NE(old_loader, data_source_->loader().get());
+  EXPECT_EQ(1234, loader_bitrate());
+
+  StopDataSource();
+}
+
+TEST_F(BufferedDataSourceTest2, SetPlaybackRate) {
+  InitializeDataSource("http://localhost/foo.webm");
+
+  data_source_->SetPlaybackRate(2.0f);
+  message_loop_->RunAllPending();
+  EXPECT_EQ(2.0f, data_source_playback_rate());
+  EXPECT_EQ(2.0f, loader_playback_rate());
+
+  // Read so far ahead to cause the loader to get recreated.
+  BufferedResourceLoader* old_loader = data_source_->loader();
+
+  uint8 buffer[1024];
+  data_source_->Read(4000000, 1024, buffer,
+                     NewReadCallback(media::DataSource::kReadError));
+  message_loop_->RunAllPending();
+
+  // Verify loader changed but still has same bitrate.
+  EXPECT_NE(old_loader, data_source_->loader().get());
+  EXPECT_EQ(2.0f, loader_playback_rate());
+
+  StopDataSource();
+}
+
 }  // namespace webkit_glue
diff --git a/webkit/glue/media/buffered_resource_loader.cc b/webkit/glue/media/buffered_resource_loader.cc
index 07f5729..06de3f2 100644
--- a/webkit/glue/media/buffered_resource_loader.cc
+++ b/webkit/glue/media/buffered_resource_loader.cc
@@ -35,11 +35,10 @@ static const int kHttpPartialContent = 206;
 // Define the number of bytes in a megabyte.
 static const size_t kMegabyte = 1024 * 1024;
 
-// Default backward capacity of the buffer.
-static const size_t kBackwardCapacity = 2 * kMegabyte;
-
-// Default forward capacity of the buffer.
-static const size_t kForwardCapacity = 10 * kMegabyte;
+// Minimum capacity of the buffer in forward or backward direction.
+//
+// 2MB is an arbitrary limit; it just seems to be "good enough" in practice.
+static const size_t kMinBufferCapacity = 2 * kMegabyte;
 
 // Maximum capacity of the buffer in forward or backward direction. This is
 // effectively the largest single read the code path can handle.
@@ -52,14 +51,60 @@ static const size_t kMaxBufferCapacity = 20 * kMegabyte;
 // location and will instead reset the request.
 static const int kForwardWaitThreshold = 2 * kMegabyte;
 
+// Computes the suggested backward and forward capacity for the buffer
+// if one wants to play at |playback_rate| * the natural playback speed.
+// Use a value of 0 for |bitrate| if it is unknown.
+static void ComputeTargetBufferWindow(float playback_rate, int bitrate,
+                                      size_t* out_backward_capacity,
+                                      size_t* out_forward_capacity) {
+  static const size_t kDefaultBitrate = 200 * 1024 * 8;  // 200 Kbps.
+  static const size_t kMaxBitrate = 20 * kMegabyte * 8;  // 20 Mbps.
+  static const float kMaxPlaybackRate = 25.0;
+  static const size_t kTargetSecondsBufferedAhead = 10;
+  static const size_t kTargetSecondsBufferedBehind = 2;
+
+  // Use a default bit rate if unknown and clamp to prevent overflow.
+  if (bitrate <= 0)
+    bitrate = kDefaultBitrate;
+  bitrate = std::min(static_cast<size_t>(bitrate), kMaxBitrate);
+
+  // Only scale the buffer window for playback rates greater than 1.0 in
+  // magnitude and clamp to prevent overflow.
+  bool backward_playback = false;
+  if (playback_rate < 0.0f) {
+    backward_playback = true;
+    playback_rate *= -1.0f;
+  }
+
+  playback_rate = std::max(playback_rate, 1.0f);
+  playback_rate = std::min(playback_rate, kMaxPlaybackRate);
+
+  size_t bytes_per_second = static_cast<size_t>(playback_rate * bitrate / 8.0);
+
+  // Clamp between kMinBufferCapacity and kMaxBufferCapacity.
+  *out_forward_capacity = std::max(
+      kTargetSecondsBufferedAhead * bytes_per_second, kMinBufferCapacity);
+  *out_backward_capacity = std::max(
+      kTargetSecondsBufferedBehind * bytes_per_second, kMinBufferCapacity);
+
+  *out_forward_capacity = std::min(*out_forward_capacity, kMaxBufferCapacity);
+  *out_backward_capacity = std::min(*out_backward_capacity, kMaxBufferCapacity);
+
+  if (backward_playback)
+    std::swap(*out_forward_capacity, *out_backward_capacity);
+}
+
+
 BufferedResourceLoader::BufferedResourceLoader(
     const GURL& url,
     int64 first_byte_position,
     int64 last_byte_position,
+    DeferStrategy strategy,
+    int bitrate,
+    float playback_rate,
     media::MediaLog* media_log)
-    : buffer_(new media::SeekableBuffer(kBackwardCapacity, kForwardCapacity)),
-      deferred_(false),
-      defer_strategy_(kReadThenDefer),
+    : deferred_(false),
+      defer_strategy_(strategy),
       completed_(false),
       range_requested_(false),
       range_supported_(false),
@@ -79,9 +124,15 @@ BufferedResourceLoader::BufferedResourceLoader(
       first_offset_(0),
       last_offset_(0),
       keep_test_loader_(false),
-      bitrate_(0),
-      playback_rate_(0.0),
+      bitrate_(bitrate),
+      playback_rate_(playback_rate),
       media_log_(media_log) {
+
+  size_t backward_capacity;
+  size_t forward_capacity;
+  ComputeTargetBufferWindow(
+      playback_rate_, bitrate_, &backward_capacity, &forward_capacity);
+  buffer_.reset(new media::SeekableBuffer(backward_capacity, forward_capacity));
 }
 
 BufferedResourceLoader::~BufferedResourceLoader() {
@@ -232,7 +283,7 @@ void BufferedResourceLoader::Read(int64 position,
       saved_forward_capacity_ = buffer_->forward_capacity();
       buffer_->set_forward_capacity(read_size_);
     }
-   return;
+    return;
   }
 
   // Make a callback to report failure.
@@ -498,6 +549,12 @@ void BufferedResourceLoader::UpdateDeferStrategy(DeferStrategy strategy) {
 
 void BufferedResourceLoader::SetPlaybackRate(float playback_rate) {
   playback_rate_ = playback_rate;
+
+  // This is a pause so don't bother updating the buffer window as we'll likely
+  // get unpaused in the future.
+  if (playback_rate_ == 0.0)
+    return;
+
   UpdateBufferWindow();
 }
 
@@ -510,49 +567,10 @@ void BufferedResourceLoader::SetBitrate(int bitrate) {
 /////////////////////////////////////////////////////////////////////////////
 // Helper methods.
 
-// Computes the suggested backward and forward capacity for the buffer
-// if one wants to play at |playback_rate| * the natural playback speed.
-// Use a value of 0 for |bitrate| if it is unknown.
-static void ComputeTargetBufferWindow(float playback_rate, int bitrate,
-                                      size_t* out_backward_capacity,
-                                      size_t* out_forward_capacity) {
-  DCHECK_GE(bitrate, 0);
-  DCHECK_NE(playback_rate, 0.0);
-  static const size_t kDefaultBitrate = kMegabyte;
-  static const size_t kMaxBitrate = 50 * kMegabyte;
-  static const float kMaxPlaybackRate = 25.0;
-  static const size_t kTargetSecondsBufferedAhead = 10;
-  static const size_t kTargetSecondsBufferedBehind = 2;
-
-  if (bitrate <= 0)
-    bitrate = kDefaultBitrate;
-
-  bool backward_playback = playback_rate < 0.0;
-  if (backward_playback)
-    playback_rate *= -1.0;
-
-  // Cap playback rate and bitrate to prevent overflow.
-  playback_rate = std::min(kMaxPlaybackRate, playback_rate);
-  bitrate  = std::min(kMaxBitrate, static_cast<size_t>(bitrate));
-
-  size_t bytes_per_second = static_cast<size_t>(playback_rate * bitrate / 8.0);
-
-  *out_forward_capacity = std::min(
-      kTargetSecondsBufferedAhead * bytes_per_second, kMaxBufferCapacity);
-  *out_backward_capacity = std::min(
-      kTargetSecondsBufferedBehind * bytes_per_second, kMaxBufferCapacity);
-  if (backward_playback)
-    std::swap(*out_forward_capacity, *out_backward_capacity);
-}
-
 void BufferedResourceLoader::UpdateBufferWindow() {
   if (!buffer_.get())
     return;
 
-  // Don't adjust buffer window if video is paused.
-  if (playback_rate_ == 0.0)
-    return;
-
   size_t backward_capacity;
   size_t forward_capacity;
   ComputeTargetBufferWindow(
diff --git a/webkit/glue/media/buffered_resource_loader.h b/webkit/glue/media/buffered_resource_loader.h
index a9a1520..1b6d55f 100644
--- a/webkit/glue/media/buffered_resource_loader.h
+++ b/webkit/glue/media/buffered_resource_loader.h
@@ -54,9 +54,15 @@ class BufferedResourceLoader
   // |kPositionNotSpecified| for not specified.
   // |last_byte_position| - Last byte to be loaded,
   // |kPositionNotSpecified| for not specified.
+  // |strategy| is the initial loading strategy to use.
+  // |bitrate| is the bitrate of the media, 0 if unknown.
+  // |playback_rate| is the current playback rate of the media.
   BufferedResourceLoader(const GURL& url,
                          int64 first_byte_position,
                          int64 last_byte_position,
+                         DeferStrategy strategy,
+                         int bitrate,
+                         float playback_rate,
                          media::MediaLog* media_log);
 
   // Start the resource loading with the specified URL and range.
@@ -167,6 +173,7 @@ class BufferedResourceLoader
   virtual ~BufferedResourceLoader();
 
  private:
+  friend class BufferedDataSourceTest2;
   friend class BufferedResourceLoaderTest;
 
   // Updates the |buffer_|'s forward and backward capacities.
diff --git a/webkit/glue/media/buffered_resource_loader_unittest.cc b/webkit/glue/media/buffered_resource_loader_unittest.cc
index 39a62a9..ef11e19 100644
--- a/webkit/glue/media/buffered_resource_loader_unittest.cc
+++ b/webkit/glue/media/buffered_resource_loader_unittest.cc
@@ -22,22 +22,10 @@
 #include "webkit/mocks/mock_webframeclient.h"
 #include "webkit/mocks/mock_weburlloader.h"
 
-using ::testing::_;
-using ::testing::Assign;
-using ::testing::AtLeast;
-using ::testing::DeleteArg;
-using ::testing::DoAll;
 using ::testing::InSequence;
-using ::testing::Invoke;
-using ::testing::InvokeWithoutArgs;
-using ::testing::NotNull;
 using ::testing::Return;
-using ::testing::ReturnRef;
-using ::testing::SetArgumentPointee;
-using ::testing::StrictMock;
 using ::testing::Truly;
 using ::testing::NiceMock;
-using ::testing::WithArgs;
 
 using WebKit::WebString;
 using WebKit::WebURLError;
@@ -100,9 +88,10 @@ class BufferedResourceLoaderTest : public testing::Test {
     last_position_ = last_position;
 
     url_loader_ = new NiceMock<MockWebURLLoader>();
-    loader_ = new BufferedResourceLoader(gurl_,
-                                         first_position_, last_position_,
-                                         new media::MediaLog());
+    loader_ = new BufferedResourceLoader(
+        gurl_, first_position_, last_position_,
+        BufferedResourceLoader::kThresholdDefer, 0, 0,
+        new media::MediaLog());
     loader_->SetURLLoaderForTest(url_loader_);
   }
 
@@ -236,6 +225,11 @@ class BufferedResourceLoaderTest : public testing::Test {
     EXPECT_EQ(0, memcmp(buffer, data_ + pos, size));
   }
 
+  void ConfirmLoaderBufferBackwardCapacity(size_t expected_backward_capacity) {
+    EXPECT_EQ(loader_->buffer_->backward_capacity(),
+              expected_backward_capacity);
+  }
+
   void ConfirmLoaderBufferForwardCapacity(size_t expected_forward_capacity) {
     EXPECT_EQ(loader_->buffer_->forward_capacity(), expected_forward_capacity);
   }
@@ -244,42 +238,17 @@ class BufferedResourceLoaderTest : public testing::Test {
     EXPECT_EQ(loader_->deferred_, expectedVal);
   }
 
-  void CheckBufferWindowIsNotTooBig() {
-    // An arbitrarily chosen, reasonable limit.
-    static const size_t kMaxBufferCapacity = 20 * 1024 * 1024;
-    EXPECT_LE(loader_->buffer_->forward_capacity(), kMaxBufferCapacity);
-    EXPECT_LE(loader_->buffer_->backward_capacity(), kMaxBufferCapacity);
-  }
-
-  void CheckBufferWindowIsNotTooSmall() {
-    EXPECT_GT(loader_->buffer_->forward_capacity(), 0u);
-    EXPECT_GT(loader_->buffer_->backward_capacity(), 0u);
-  }
-
   // Makes sure the |loader_| buffer window is in a reasonable range.
   void CheckBufferWindowBounds() {
-    CheckBufferWindowIsNotTooSmall();
-    CheckBufferWindowIsNotTooBig();
-  }
+    // Corresponds to value defined in buffered_resource_loader.cc.
+    static const size_t kMinBufferCapacity = 2 * 1024 * 1024;
+    EXPECT_GE(loader_->buffer_->forward_capacity(), kMinBufferCapacity);
+    EXPECT_GE(loader_->buffer_->backward_capacity(), kMinBufferCapacity);
 
-  // Updates the |loader_|'s |playback_rate| and |bitrate|, then returns via the
-  // output parameters the resultant change in the forward and backward capacity
-  // of |loader_|'s buffer window.
-  void InvokeChangeInBufferWindow(float playback_rate, int bitrate,
-                                  int* out_forward_capacity_delta,
-                                  int* out_backward_capacity_delta) {
-    CheckBufferWindowBounds();
-    size_t old_forward_capacity = loader_->buffer_->forward_capacity();
-    size_t old_backward_capacity = loader_->buffer_->backward_capacity();
-
-    loader_->SetPlaybackRate(playback_rate);
-    loader_->SetBitrate(bitrate);
-    CheckBufferWindowBounds();
-
-    *out_forward_capacity_delta =
-        loader_->buffer_->forward_capacity() - old_forward_capacity;
-    *out_backward_capacity_delta =
-        loader_->buffer_->backward_capacity() - old_backward_capacity;
+    // Corresponds to value defined in buffered_resource_loader.cc.
+    static const size_t kMaxBufferCapacity = 20 * 1024 * 1024;
+    EXPECT_LE(loader_->buffer_->forward_capacity(), kMaxBufferCapacity);
+    EXPECT_LE(loader_->buffer_->backward_capacity(), kMaxBufferCapacity);
   }
 
   MOCK_METHOD1(StartCallback, void(int error));
@@ -445,12 +414,15 @@ TEST_F(BufferedResourceLoaderTest, ReadExtendBuffer) {
   Start();
   PartialResponse(10, 0x014FFFFFF, 0x01500000);
 
+  // Don't test for network callbacks (covered by *Strategy tests).
+  EXPECT_CALL(*this, NetworkCallback())
+      .WillRepeatedly(Return());
+
   uint8 buffer[20];
   InSequence s;
 
   // Write more than forward capacity and read it back. Ensure forward capacity
   // gets reset.
-  EXPECT_CALL(*this, NetworkCallback());
   WriteLoader(10, 20);
   EXPECT_CALL(*this, ReadCallback(20));
   ReadLoader(10, 20, buffer);
@@ -460,14 +432,12 @@ TEST_F(BufferedResourceLoaderTest, ReadExtendBuffer) {
 
   // Make and outstanding read request larger than forward capacity. Ensure
   // forward capacity gets extended.
-  EXPECT_CALL(*this, NetworkCallback());
   ReadLoader(30, 20, buffer);
 
   ConfirmLoaderBufferForwardCapacity(20);
 
   // Fulfill outstanding request. Ensure forward capacity gets reset.
   EXPECT_CALL(*this, ReadCallback(20));
-  EXPECT_CALL(*this, NetworkCallback());
   WriteLoader(30, 20);
 
   VerifyBuffer(buffer, 30, 20);
@@ -475,7 +445,6 @@ TEST_F(BufferedResourceLoaderTest, ReadExtendBuffer) {
 
   // Try to read further ahead than kForwardWaitThreshold allows. Ensure
   // forward capacity is not changed.
-  EXPECT_CALL(*this, NetworkCallback());
   EXPECT_CALL(*this, ReadCallback(net::ERR_CACHE_MISS));
   ReadLoader(0x00300000, 1, buffer);
 
@@ -493,7 +462,6 @@ TEST_F(BufferedResourceLoaderTest, ReadExtendBuffer) {
 
 TEST_F(BufferedResourceLoaderTest, ReadOutsideBuffer) {
   Initialize(kHttpUrl, 10, 0x00FFFFFF);
-  loader_->UpdateDeferStrategy(BufferedResourceLoader::kThresholdDefer);
   Start();
   PartialResponse(10, 0x00FFFFFF, 0x01000000);
 
@@ -694,167 +662,92 @@ TEST_F(BufferedResourceLoaderTest, HasSingleOrigin) {
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_Bitrate_Set) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_Default) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-  loader_->SetBitrate(1024 * 8);
-  CheckBufferWindowBounds();
+
+  // Test ensures that default construction of a BufferedResourceLoader has sane
+  // values.
+  //
+  // Please do not change these values in order to make a test pass! Instead,
+  // start a conversation on what the default buffer window capacities should
+  // be.
+  ConfirmLoaderBufferBackwardCapacity(2 * 1024 * 1024);
+  ConfirmLoaderBufferForwardCapacity(2 * 1024 * 1024);
+
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_Bitrate_Increase) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_Bitrate_Unknown) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-
-  static const int kBitrate = 1024 * 8;
-  static const float kPlaybackRate = 1.0;
-  loader_->SetBitrate(kBitrate);
-  loader_->SetPlaybackRate(kPlaybackRate);
-
-  static const int kNewBitrate = 1024 * 1024 * 8;
-  int forward_capacity_delta;
-  int backward_capacity_delta;
-  InvokeChangeInBufferWindow(kPlaybackRate, kNewBitrate,
-                             &forward_capacity_delta, &backward_capacity_delta);
-
-  EXPECT_GT(forward_capacity_delta, 0);
-  EXPECT_GT(backward_capacity_delta, 0);
-
+  loader_->SetBitrate(0);
+  CheckBufferWindowBounds();
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_Bitrate_Decrease) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_Bitrate_BelowLowerBound) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-
-  static const int kBitrate = 1024 * 1024 * 8;
-  static const float kPlaybackRate = 1.0;
-  loader_->SetBitrate(kBitrate);
-  loader_->SetPlaybackRate(kPlaybackRate);
-
-  static const int kNewBitrate = 1024 * 8;
-  int forward_capacity_delta;
-  int backward_capacity_delta;
-  InvokeChangeInBufferWindow(kPlaybackRate, kNewBitrate,
-                             &forward_capacity_delta, &backward_capacity_delta);
-
-  EXPECT_LT(forward_capacity_delta, 0);
-  EXPECT_LT(backward_capacity_delta, 0);
-
+  loader_->SetBitrate(1024 * 8);  // 1 Kbps.
+  CheckBufferWindowBounds();
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_SetVeryLarge) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_Bitrate_WithinBounds) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-
-  loader_->SetPlaybackRate(1.0);
-  loader_->SetBitrate(100 * 1024 * 1024);
-  CheckBufferWindowBounds();
-
-  loader_->SetBitrate(1024 * 1024 * 8);
-  loader_->SetPlaybackRate(10000.0);
-  CheckBufferWindowBounds();
-
-  loader_->SetPlaybackRate(-10000.0);
+  loader_->SetBitrate(2 * 1024 * 1024 * 8);  // 2 Mbps.
   CheckBufferWindowBounds();
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_SetVerySmall) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_Bitrate_AboveUpperBound) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-  loader_->SetBitrate(1);
+  loader_->SetBitrate(100 * 1024 * 1024 * 8);  // 100 Mbps.
   CheckBufferWindowBounds();
-
-  // Only check for too big of buffer size in very small playback case; a buffer
-  // window of 0 is reasonable if playback is crawling.
-  loader_->SetPlaybackRate(-0.01f);
-  CheckBufferWindowIsNotTooBig();
-  loader_->SetPlaybackRate(0.01f);
-  CheckBufferWindowIsNotTooBig();
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_SetUnknownBitrate) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_PlaybackRate_Negative) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-  loader_->SetBitrate(0);
+  loader_->SetPlaybackRate(-10);
   CheckBufferWindowBounds();
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_PlaybackRate_Increase) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_PlaybackRate_Zero) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-
-  static const int kBitrate = 1024 * 8;
-  static const float kPlaybackRate = 1.0;
-  loader_->SetBitrate(kBitrate);
-  loader_->SetPlaybackRate(kPlaybackRate);
-
-  static const float kNewPlaybackRate = 1.5;
-  int forward_capacity_delta;
-  int backward_capacity_delta;
-  InvokeChangeInBufferWindow(kNewPlaybackRate, kBitrate,
-                             &forward_capacity_delta, &backward_capacity_delta);
-
-  EXPECT_GT(forward_capacity_delta, 0);
-  EXPECT_GT(backward_capacity_delta, 0);
-
+  loader_->SetPlaybackRate(0);
+  CheckBufferWindowBounds();
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_PlaybackRate_Decrease) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_PlaybackRate_BelowLowerBound) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-
-  static const int kBitrate = 1024 * 8;
-  static const float kPlaybackRate = 1.0;
-  loader_->SetBitrate(kBitrate);
-  loader_->SetPlaybackRate(kPlaybackRate);
-
-  static const float kNewPlaybackRate = 0.5;
-  int forward_capacity_delta;
-  int backward_capacity_delta;
-  InvokeChangeInBufferWindow(kNewPlaybackRate, kBitrate,
-                             &forward_capacity_delta, &backward_capacity_delta);
-
-  EXPECT_LT(forward_capacity_delta, 0);
-  EXPECT_LT(backward_capacity_delta, 0);
-
+  loader_->SetPlaybackRate(0.1f);
+  CheckBufferWindowBounds();
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_PlaybackRate_Backwards) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_PlaybackRate_WithinBounds) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-  loader_->SetPlaybackRate(-1.0);
+  loader_->SetPlaybackRate(10);
   CheckBufferWindowBounds();
   StopWhenLoad();
 }
 
-TEST_F(BufferedResourceLoaderTest, BufferWindow_PlayPause) {
+TEST_F(BufferedResourceLoaderTest, BufferWindow_PlaybackRate_AboveUpperBound) {
   Initialize(kHttpUrl, -1, -1);
   Start();
-
-  static const int kBitrate = 1024 * 8;
-  static const float kPlaybackRate = 1.0;
-  loader_->SetBitrate(kBitrate);
-  loader_->SetPlaybackRate(kPlaybackRate);
-
-  static const float kPausedPlaybackRate = 0.0;
-  int forward_capacity_delta;
-  int backward_capacity_delta;
-  InvokeChangeInBufferWindow(kPausedPlaybackRate, kBitrate,
-                             &forward_capacity_delta, &backward_capacity_delta);
-
-  EXPECT_EQ(forward_capacity_delta, 0);
-  EXPECT_EQ(backward_capacity_delta, 0);
-
+  loader_->SetPlaybackRate(100);
+  CheckBufferWindowBounds();
   StopWhenLoad();
 }
 
-// TODO(hclam): add unit test for defer loading.
-
 }  // namespace webkit_glue