Fix BufferedResourceLoader's kReadThenDefer strategy.

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

3 files changed, 44 insertions, 7 deletions

diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index bfcafd1..dad922c 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -58,8 +58,7 @@ BufferedDataSource::BufferedDataSource(
       using_range_request_(true) {
 }
 
-BufferedDataSource::~BufferedDataSource() {
-}
+BufferedDataSource::~BufferedDataSource() {}
 
 // A factory method to create BufferedResourceLoader using the read parameters.
 // This method can be overrided to inject mock BufferedResourceLoader object
@@ -148,6 +147,7 @@ void BufferedDataSource::SetPreload(media::Preload preload) {
 // media::DataSource implementation.
 void BufferedDataSource::Read(int64 position, size_t size, uint8* data,
                               media::DataSource::ReadCallback* read_callback) {
+  VLOG(1) << "Read: " << position << " offset, " << size << " bytes";
   DCHECK(read_callback);
 
   {
@@ -353,6 +353,8 @@ void BufferedDataSource::ReadInternal() {
 // Method to report the results of the current read request. Also reset all
 // the read parameters.
 void BufferedDataSource::DoneRead_Locked(int error) {
+  VLOG(1) << "DoneRead: " << error << " bytes";
+
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(read_callback_.get());
   lock_.AssertAcquired();
diff --git a/webkit/glue/media/buffered_resource_loader.cc b/webkit/glue/media/buffered_resource_loader.cc
index 883f8a5..eb14351 100644
--- a/webkit/glue/media/buffered_resource_loader.cc
+++ b/webkit/glue/media/buffered_resource_loader.cc
@@ -272,6 +272,7 @@ void BufferedResourceLoader::didSendData(
 void BufferedResourceLoader::didReceiveResponse(
     WebURLLoader* loader,
     const WebURLResponse& response) {
+  VLOG(1) << "didReceiveResponse: " << response.httpStatusCode();
 
   // The loader may have been stopped and |start_callback| is destroyed.
   // In this case we shouldn't do anything.
@@ -336,6 +337,8 @@ void BufferedResourceLoader::didReceiveData(
     const char* data,
     int data_length,
     int encoded_data_length) {
+  VLOG(1) << "didReceiveData: " << data_length << " bytes";
+
   DCHECK(!completed_);
   DCHECK_GT(data_length, 0);
 
@@ -382,6 +385,8 @@ void BufferedResourceLoader::didReceiveCachedMetadata(
 void BufferedResourceLoader::didFinishLoading(
     WebURLLoader* loader,
     double finishTime) {
+  VLOG(1) << "didFinishLoading";
+
   DCHECK(!completed_);
   completed_ = true;
 
@@ -421,6 +426,8 @@ void BufferedResourceLoader::didFinishLoading(
 void BufferedResourceLoader::didFail(
     WebURLLoader* loader,
     const WebURLError& error) {
+  VLOG(1) << "didFail: " << error.reason;
+
   DCHECK(!completed_);
   completed_ = true;
 
@@ -499,11 +506,9 @@ bool BufferedResourceLoader::ShouldDisableDefer() {
 
     // We have an outstanding read request, and we have not buffered enough
     // yet to fulfill the request; disable defer to get more data.
-    case kReadThenDefer: {
-      size_t amount_buffered = buffer_->forward_bytes();
-      size_t amount_to_read = static_cast<size_t>(read_size_);
-      return read_callback_.get() && amount_buffered < amount_to_read;
-    }
+    case kReadThenDefer:
+      return read_callback_.get() &&
+          last_offset_ > static_cast<int>(buffer_->forward_bytes());
 
     // We have less than half the capacity of our threshold, so
     // disable defer to get more data.
diff --git a/webkit/glue/media/buffered_resource_loader_unittest.cc b/webkit/glue/media/buffered_resource_loader_unittest.cc
index 29f1a64..f23bd89 100644
--- a/webkit/glue/media/buffered_resource_loader_unittest.cc
+++ b/webkit/glue/media/buffered_resource_loader_unittest.cc
@@ -475,6 +475,36 @@ TEST_F(BufferedResourceLoaderTest, ReadThenDeferStrategy) {
   ConfirmLoaderDeferredState(true);
   VerifyBuffer(buffer, 10, 10);
 
+  // Read again which should disable deferring since there should be nothing
+  // left in our internal buffer.
+  EXPECT_CALL(*this, NetworkCallback());
+  ReadLoader(20, 10, buffer);
+
+  ConfirmLoaderDeferredState(false);
+
+  // Over-fulfill requested bytes, then deferring should be enabled again.
+  EXPECT_CALL(*this, NetworkCallback());
+  EXPECT_CALL(*this, ReadCallback(10));
+  WriteLoader(20, 40);
+
+  ConfirmLoaderDeferredState(true);
+  VerifyBuffer(buffer, 20, 10);
+
+  // Read far ahead, which should disable deferring. In this case we still have
+  // bytes in our internal buffer.
+  EXPECT_CALL(*this, NetworkCallback());
+  ReadLoader(80, 10, buffer);
+
+  ConfirmLoaderDeferredState(false);
+
+  // Fulfill requested bytes, then deferring should be enabled again.
+  EXPECT_CALL(*this, NetworkCallback());
+  EXPECT_CALL(*this, ReadCallback(10));
+  WriteLoader(60, 40);
+
+  ConfirmLoaderDeferredState(true);
+  VerifyBuffer(buffer, 80, 10);
+
   StopWhenLoad();
 }