Added fallback code for servers that don't support Range requests properly

This change issues a request without a Range header if the first request with
a Range header fails. This allows us to play files from servers that don't
properly implement responses to Range requests.

Patch by acolwell@chromium.org:
http://codereview.chromium.org/3796007/show

BUG=39048
TEST=BufferedDataSourceTest.MissingContentRange

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

3 files changed, 66 insertions, 18 deletions

diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index 4feb0e6..3456e3a 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -560,7 +560,8 @@ BufferedDataSource::BufferedDataSource(
       render_loop_(render_loop),
       stop_signal_received_(false),
       stopped_on_render_loop_(false),
-      media_is_paused_(true) {
+      media_is_paused_(true),
+      using_range_request_(true) {
 }
 
 BufferedDataSource::~BufferedDataSource() {
@@ -879,6 +880,17 @@ void BufferedDataSource::HttpInitialStartCallback(int error) {
     loader_->Stop();
   }
 
+  if (error == net::ERR_INVALID_RESPONSE && using_range_request_) {
+    // Assuming that the Range header was causing the problem. Retry without
+    // the Range header.
+    using_range_request_ = false;
+    loader_ = CreateResourceLoader(-1, -1);
+    loader_->Start(
+        NewCallback(this, &BufferedDataSource::HttpInitialStartCallback),
+        NewCallback(this, &BufferedDataSource::NetworkEventCallback));
+    return;
+  }
+
   // We need to prevent calling to filter host and running the callback if
   // we have received the stop signal. We need to lock down the whole callback
   // method to prevent bad things from happening. The reason behind this is
diff --git a/webkit/glue/media/buffered_data_source.h b/webkit/glue/media/buffered_data_source.h
index d07eb01..e7b2e47 100644
--- a/webkit/glue/media/buffered_data_source.h
+++ b/webkit/glue/media/buffered_data_source.h
@@ -406,6 +406,10 @@ class BufferedDataSource : public media::DataSource {
   // the message loop doesn't hold a reference for the watch dog task.
   base::RepeatingTimer<BufferedDataSource> watch_dog_timer_;
 
+  // Keeps track of whether we used a Range header in the initialization
+  // request.
+  bool using_range_request_;
+
   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 2ba0087..f032595 100644
--- a/webkit/glue/media/buffered_data_source_unittest.cc
+++ b/webkit/glue/media/buffered_data_source_unittest.cc
@@ -29,6 +29,7 @@ using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SetArgumentPointee;
 using ::testing::StrictMock;
+using ::testing::NiceMock;
 using ::testing::WithArgs;
 
 namespace {
@@ -597,6 +598,17 @@ class BufferedDataSourceTest : public testing::Test {
     }
   }
 
+  void ExpectCreateAndStartResourceLoader(int start_error) {
+    EXPECT_CALL(*data_source_, CreateResourceLoader(_, _))
+        .WillOnce(Return(loader_.get()));
+
+    EXPECT_CALL(*loader_, Start(NotNull(), NotNull()))
+        .WillOnce(
+            DoAll(Assign(&error_, start_error),
+                  Invoke(this,
+                         &BufferedDataSourceTest::InvokeStartCallback)));
+  }
+
   void InitializeDataSource(const char* url, int error,
                             bool partial_response, int64 instance_size,
                             NetworkState networkState) {
@@ -613,21 +625,34 @@ class BufferedDataSourceTest : public testing::Test {
     // There is no need to provide a message loop to data source.
     data_source_->set_host(&host_);
 
+    scoped_refptr<NiceMock<MockBufferedResourceLoader> > first_loader =
+        new NiceMock<MockBufferedResourceLoader>();
+
     // Creates the mock loader to be injected.
-    loader_ = new StrictMock<MockBufferedResourceLoader>();
+    loader_ = first_loader;
 
+    bool initialized_ok = (error == net::OK);
     bool loaded = networkState == LOADED;
     {
       InSequence s;
-      EXPECT_CALL(*data_source_, CreateResourceLoader(_, _))
-          .WillOnce(Return(loader_.get()));
-
-      // The initial response loader will be started.
-      EXPECT_CALL(*loader_, Start(NotNull(), NotNull()))
-          .WillOnce(
-              DoAll(Assign(&error_, error),
-                    Invoke(this,
-                           &BufferedDataSourceTest::InvokeStartCallback)));
+      ExpectCreateAndStartResourceLoader(error);
+
+      // In the case of an invalid partial response we expect a second loader
+      // to be created.
+      if (partial_response && (error == net::ERR_INVALID_RESPONSE)) {
+        // Verify that the initial loader is stopped.
+        EXPECT_CALL(*loader_, Stop());
+
+        // Replace loader_ with a new instance.
+        loader_ = new NiceMock<MockBufferedResourceLoader>();
+
+        // Create and start Make sure Start() is called the new loader.
+        ExpectCreateAndStartResourceLoader(net::OK);
+
+        // Update initialization variable since we know the second loader will
+        // return OK.
+        initialized_ok = true;
+      }
     }
 
     StrictMock<media::MockFilterCallback> callback;
@@ -635,7 +660,7 @@ class BufferedDataSourceTest : public testing::Test {
         .WillRepeatedly(Return(instance_size));
     EXPECT_CALL(*loader_, partial_response())
         .WillRepeatedly(Return(partial_response));
-    if (error == net::OK) {
+    if (initialized_ok) {
       // Expected loaded or not.
       EXPECT_CALL(host_, SetLoaded(loaded));
 
@@ -663,7 +688,7 @@ class BufferedDataSourceTest : public testing::Test {
     data_source_->Initialize(url, callback.NewCallback());
     message_loop_->RunAllPending();
 
-    if (error == net::OK) {
+    if (initialized_ok) {
       // Verify the size of the data source.
       int64 size;
       if (instance_size != -1 && (loaded || partial_response)) {
@@ -746,8 +771,8 @@ class BufferedDataSourceTest : public testing::Test {
     }
 
     // 2. Then the current loader will be stop and destroyed.
-    StrictMock<MockBufferedResourceLoader> *new_loader =
-        new StrictMock<MockBufferedResourceLoader>();
+    NiceMock<MockBufferedResourceLoader> *new_loader =
+        new NiceMock<MockBufferedResourceLoader>();
     EXPECT_CALL(*data_source_, CreateResourceLoader(position, -1))
         .WillOnce(Return(new_loader));
 
@@ -810,8 +835,8 @@ class BufferedDataSourceTest : public testing::Test {
     }
 
     // 2. Then the current loader will be stop and destroyed.
-    StrictMock<MockBufferedResourceLoader> *new_loader =
-        new StrictMock<MockBufferedResourceLoader>();
+    NiceMock<MockBufferedResourceLoader> *new_loader =
+        new NiceMock<MockBufferedResourceLoader>();
     EXPECT_CALL(*data_source_, CreateResourceLoader(position, -1))
         .WillOnce(Return(new_loader));
 
@@ -853,7 +878,7 @@ class BufferedDataSourceTest : public testing::Test {
 
   scoped_ptr<StrictMock<MockMediaResourceLoaderBridgeFactory> >
       bridge_factory_;
-  scoped_refptr<StrictMock<MockBufferedResourceLoader> > loader_;
+  scoped_refptr<NiceMock<MockBufferedResourceLoader> > loader_;
   scoped_refptr<MockBufferedDataSource> data_source_;
   scoped_refptr<media::FilterFactory> factory_;
 
@@ -889,6 +914,13 @@ TEST_F(BufferedDataSourceTest, RangeRequestNotSupported) {
   StopDataSource();
 }
 
+// Test the case where we get a 206 response, but no Content-Range header.
+TEST_F(BufferedDataSourceTest, MissingContentRange) {
+  InitializeDataSource(kHttpUrl, net::ERR_INVALID_RESPONSE, true, 1024,
+                       LOADING);
+  StopDataSource();
+}
+
 TEST_F(BufferedDataSourceTest,
        MissingContentLengthAndRangeRequestNotSupported) {
   InitializeDataSource(kHttpUrl, net::OK, false, -1, LOADING);