Merge 83942 - Partial revert of 82061 so we keep the initial unbounded range request.

Also loosened restrictions for detecting servers that support range requests. Sending "Accept-Ranges: bytes" is optional so if we attempt a range request and a server replies correctly we should assume that it does indeed support range requests.

BUG=80187
TEST=test_shell_tests --gtest_filter=Buffered*

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

TBR=scherkus@chromium.org
Review URL: http://codereview.chromium.org/6926002

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

3 files changed, 60 insertions, 13 deletions

diff --git a/webkit/glue/media/buffered_resource_loader.cc b/webkit/glue/media/buffered_resource_loader.cc
index b19fc92..8d7f1dd 100644
--- a/webkit/glue/media/buffered_resource_loader.cc
+++ b/webkit/glue/media/buffered_resource_loader.cc
@@ -103,7 +103,7 @@ void BufferedResourceLoader::Start(net::CompletionCallback* start_callback,
   WebURLRequest request(url_);
   request.setTargetType(WebURLRequest::TargetIsMedia);
 
-  if (!IsWholeFileRange()) {
+  if (IsRangeRequest()) {
     range_requested_ = true;
     request.setHTTPHeaderField(WebString::fromUTF8("Range"),
                                WebString::fromUTF8(GenerateHeaders(
@@ -295,8 +295,11 @@ void BufferedResourceLoader::didReceiveResponse(
 
     if (range_requested_) {
       // If we have verified the partial response and it is correct, we will
-      // return net::OK.
-      if (!partial_response || !VerifyPartialResponse(response))
+      // return net::OK. It's also possible for a server to support range
+      // requests without advertising Accept-Ranges: bytes.
+      if (partial_response && VerifyPartialResponse(response))
+        range_supported_ = true;
+      else
         error = net::ERR_INVALID_RESPONSE;
     } else if (response.httpStatusCode() != kHttpOK) {
       // We didn't request a range but server didn't reply with "200 OK".
@@ -643,10 +646,8 @@ void BufferedResourceLoader::NotifyNetworkEvent() {
     event_callback_->Run();
 }
 
-bool BufferedResourceLoader::IsWholeFileRange() const {
-  return ((first_byte_position_ == kPositionNotSpecified ||
-           first_byte_position_ == 0) &&
-          last_byte_position_ == kPositionNotSpecified);
+bool BufferedResourceLoader::IsRangeRequest() const {
+  return first_byte_position_ != kPositionNotSpecified;
 }
 
 }  // namespace webkit_glue
diff --git a/webkit/glue/media/buffered_resource_loader.h b/webkit/glue/media/buffered_resource_loader.h
index 2d43fb8..39e89d8 100644
--- a/webkit/glue/media/buffered_resource_loader.h
+++ b/webkit/glue/media/buffered_resource_loader.h
@@ -207,9 +207,8 @@ class BufferedResourceLoader :
 
   bool HasPendingRead() { return read_callback_.get() != NULL; }
 
-  // Helper function that returns true if a range is not provided
-  // or a 0- range is specified.
-  bool IsWholeFileRange() const;
+  // Helper function that returns true if a range request was specified.
+  bool IsRangeRequest() const;
 
   // A sliding window of buffer.
   scoped_ptr<media::SeekableBuffer> buffer_;
diff --git a/webkit/glue/media/buffered_resource_loader_unittest.cc b/webkit/glue/media/buffered_resource_loader_unittest.cc
index 3e7dccd..03fd360d 100644
--- a/webkit/glue/media/buffered_resource_loader_unittest.cc
+++ b/webkit/glue/media/buffered_resource_loader_unittest.cc
@@ -131,23 +131,49 @@ class BufferedResourceLoaderTest : public testing::Test {
 
   void PartialResponse(int64 first_position, int64 last_position,
                        int64 instance_size) {
+    PartialResponse(first_position, last_position, instance_size, false, true);
+  }
+
+  void PartialResponse(int64 first_position, int64 last_position,
+                       int64 instance_size, bool chunked, bool accept_ranges) {
     EXPECT_CALL(*this, StartCallback(net::OK));
-    int64 content_length = last_position - first_position + 1;
 
     WebURLResponse response(gurl_);
-    response.setHTTPHeaderField(WebString::fromUTF8("Accept-Ranges"),
-                                WebString::fromUTF8("bytes"));
     response.setHTTPHeaderField(WebString::fromUTF8("Content-Range"),
                                 WebString::fromUTF8(base::StringPrintf("bytes "
                                             "%" PRId64 "-%" PRId64 "/%" PRId64,
                                             first_position,
                                             last_position,
                                             instance_size)));
+
+    // HTTP 1.1 doesn't permit Content-Length with Transfer-Encoding: chunked.
+    int64 content_length = -1;
+    if (chunked) {
+      response.setHTTPHeaderField(WebString::fromUTF8("Transfer-Encoding"),
+                                  WebString::fromUTF8("chunked"));
+    } else {
+      content_length = last_position - first_position + 1;
+    }
     response.setExpectedContentLength(content_length);
+
+    // A server isn't required to return Accept-Ranges even though it might.
+    if (accept_ranges) {
+      response.setHTTPHeaderField(WebString::fromUTF8("Accept-Ranges"),
+                                  WebString::fromUTF8("bytes"));
+    }
+
     response.setHTTPStatusCode(kHttpPartialContent);
     loader_->didReceiveResponse(url_loader_, response);
+
+    // XXX: what's the difference between these two? For example in the chunked
+    // range request case, Content-Length is unspecified (because it's chunked)
+    // but Content-Range: a-b/c can be returned, where c == Content-Length
+    //
+    // Can we eliminate one?
     EXPECT_EQ(content_length, loader_->content_length());
     EXPECT_EQ(instance_size, loader_->instance_size());
+
+    // A valid partial response should always result in this being true.
     EXPECT_TRUE(loader_->range_supported());
   }
 
@@ -257,6 +283,27 @@ TEST_F(BufferedResourceLoaderTest, PartialResponse) {
   StopWhenLoad();
 }
 
+TEST_F(BufferedResourceLoaderTest, PartialResponse_Chunked) {
+  Initialize(kHttpUrl, 100, 200);
+  Start();
+  PartialResponse(100, 200, 1024, true, true);
+  StopWhenLoad();
+}
+
+TEST_F(BufferedResourceLoaderTest, PartialResponse_NoAcceptRanges) {
+  Initialize(kHttpUrl, 100, 200);
+  Start();
+  PartialResponse(100, 200, 1024, false, false);
+  StopWhenLoad();
+}
+
+TEST_F(BufferedResourceLoaderTest, PartialResponse_ChunkedNoAcceptRanges) {
+  Initialize(kHttpUrl, 100, 200);
+  Start();
+  PartialResponse(100, 200, 1024, true, false);
+  StopWhenLoad();
+}
+
 // Tests that an invalid partial response is received.
 TEST_F(BufferedResourceLoaderTest, InvalidPartialResponse) {
   Initialize(kHttpUrl, 0, 10);