Merge 121274 - Return net::ERR_FAILED when BufferedResourceLoader::didFail() is called.

The documentation of BufferedResourceLoader's start/read callbacks states that it'll return only a tiny subset of the many error codes listed under net/base/net_error_list.h

It's possible to receive an error code of 0 for didFail() that maps neatly to net::OK. The end result is we trick callees into thinking the operation succeeded when, in fact, it did not.

This is a short term fix for bug 112833 until we can replace our use of net::CompletionCallback (see bug 110120).

BUG=112833

Review URL: https://chromiumcodereview.appspot.com/9375005

TBR=scherkus@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9372041

git-svn-id: svn://svn.chromium.org/chrome/branches/1025/src@121356 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 34 insertions, 8 deletions

diff --git a/webkit/media/buffered_resource_loader.cc b/webkit/media/buffered_resource_loader.cc
index 4bb6d80..eea8f57 100644
--- a/webkit/media/buffered_resource_loader.cc
+++ b/webkit/media/buffered_resource_loader.cc
@@ -221,7 +221,12 @@ void BufferedResourceLoader::Read(
   read_size_ = read_size;
   read_buffer_ = buffer;
 
-  // If read position is beyond the instance size, we cannot read there.
+  // If we're attempting to read past the end of the file, return a zero
+  // indicating EOF.
+  //
+  // This can happen with callees that read in fixed-sized amounts for parsing
+  // or at the end of chunked 200 responses when we discover the actual length
+  // of the file.
   if (instance_size_ != kPositionNotSpecified &&
       instance_size_ <= read_position_) {
     DoneRead(0);
@@ -354,7 +359,7 @@ void BufferedResourceLoader::didSendData(
 void BufferedResourceLoader::didReceiveResponse(
     WebURLLoader* loader,
     const WebURLResponse& response) {
-  VLOG(1) << "didReceiveResponse: " << response.httpStatusCode();
+  DVLOG(1) << "didReceiveResponse: " << response.httpStatusCode();
   DCHECK(active_loader_.get());
 
   // The loader may have been stopped and |start_callback| is destroyed.
@@ -419,7 +424,7 @@ void BufferedResourceLoader::didReceiveData(
     const char* data,
     int data_length,
     int encoded_data_length) {
-  VLOG(1) << "didReceiveData: " << data_length << " bytes";
+  DVLOG(1) << "didReceiveData: " << data_length << " bytes";
   DCHECK(active_loader_.get());
   DCHECK_GT(data_length, 0);
 
@@ -467,7 +472,7 @@ void BufferedResourceLoader::didReceiveCachedMetadata(
 void BufferedResourceLoader::didFinishLoading(
     WebURLLoader* loader,
     double finishTime) {
-  VLOG(1) << "didFinishLoading";
+  DVLOG(1) << "didFinishLoading";
   DCHECK(active_loader_.get());
 
   // We're done with the loader.
@@ -507,7 +512,8 @@ void BufferedResourceLoader::didFinishLoading(
 void BufferedResourceLoader::didFail(
     WebURLLoader* loader,
     const WebURLError& error) {
-  VLOG(1) << "didFail: " << error.reason;
+  DVLOG(1) << "didFail: reason=" << error.reason
+           << " isCancellation=" << error.isCancellation;
   DCHECK(active_loader_.get());
 
   // We don't need to continue loading after failure.
@@ -520,13 +526,13 @@ void BufferedResourceLoader::didFail(
   if (!start_callback_.is_null()) {
     DCHECK(read_callback_.is_null())
         << "Shouldn't have a read callback during start";
-    DoneStart(error.reason);
+    DoneStart(net::ERR_FAILED);
     return;
   }
 
   // Don't leave read callbacks hanging around.
   if (HasPendingRead()) {
-    DoneRead(error.reason);
+    DoneRead(net::ERR_FAILED);
   }
 }
 
diff --git a/webkit/media/buffered_resource_loader_unittest.cc b/webkit/media/buffered_resource_loader_unittest.cc
index 212b90b..97d1090 100644
--- a/webkit/media/buffered_resource_loader_unittest.cc
+++ b/webkit/media/buffered_resource_loader_unittest.cc
@@ -527,11 +527,31 @@ TEST_F(BufferedResourceLoaderTest, RequestFailedWhenRead) {
   uint8 buffer[10];
   InSequence s;
 
+  // We should convert any error we receive to net::ERR_FAILED.
   ReadLoader(10, 10, buffer);
   EXPECT_CALL(*this, NetworkCallback());
   EXPECT_CALL(*this, ReadCallback(net::ERR_FAILED));
   WebURLError error;
-  error.reason = net::ERR_FAILED;
+  error.reason = net::ERR_TIMED_OUT;
+  error.isCancellation = false;
+  loader_->didFail(url_loader_, error);
+}
+
+TEST_F(BufferedResourceLoaderTest, RequestCancelledWhenRead) {
+  Initialize(kHttpUrl, 10, 29);
+  Start();
+  PartialResponse(10, 29, 30);
+
+  uint8 buffer[10];
+  InSequence s;
+
+  // We should convert any error we receive to net::ERR_FAILED.
+  ReadLoader(10, 10, buffer);
+  EXPECT_CALL(*this, NetworkCallback());
+  EXPECT_CALL(*this, ReadCallback(net::ERR_FAILED));
+  WebURLError error;
+  error.reason = 0;
+  error.isCancellation = true;
   loader_->didFail(url_loader_, error);
 }