Split BufferedResourceLoader's network callback into separate loading state and progress callbacks.

webkit/media/buffered_data_source_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "media/base/media_log.h"
 #include "media/base/mock_callback.h"
 #include "media/base/mock_data_source_host.h"
 #include "media/base/mock_filters.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebURLResponse.h"
 #include "webkit/media/buffered_data_source.h"
 #include "webkit/mocks/mock_webframeclient.h"
 #include "webkit/mocks/mock_weburlloader.h"
 #include "webkit/media/test_response_generator.h"
 
 using ::testing::_;
 using ::testing::Assign;
 using ::testing::Invoke;
+using ::testing::NiceMock;
 using ::testing::StrictMock;
-using ::testing::NiceMock;
 
 using WebKit::WebFrame;
 using WebKit::WebString;
 using WebKit::WebURLLoader;
 using WebKit::WebURLResponse;
 using WebKit::WebView;
 
 using webkit_glue::MockWebFrameClient;
 using webkit_glue::MockWebURLLoader;
 
@@ skipping 64 matching lines @@
                                               view_->mainFrame());
     data_source_->set_host(&host_);
   }
 
   virtual ~BufferedDataSourceTest() {
     view_->close();
   }
 
   void Initialize(media::PipelineStatus expected) {
     ExpectCreateResourceLoader();
+
+    EXPECT_FALSE(data_source_->downloading());
     data_source_->Initialize(response_generator_.gurl(),
                              BufferedResourceLoader::kUnspecified,
                              media::NewExpectedStatusCB(expected));
     message_loop_.RunAllPending();
+    EXPECT_TRUE(data_source_->downloading());
   }
 
   // Helper to initialize tests with a valid 206 response.
   void InitializeWith206Response() {
     Initialize(media::PIPELINE_OK);
 
     EXPECT_CALL(host_, SetTotalBytes(response_generator_.content_length()));
     Respond(response_generator_.Generate206(0));
   }
 
@@ skipping 16 matching lines @@
         .WillOnce(Invoke(data_source_.get(),
                          &MockBufferedDataSource::CreateMockResourceLoader));
     message_loop_.RunAllPending();
   }
 
   void Respond(const WebURLResponse& response) {
     loader()->didReceiveResponse(url_loader(), response);
     message_loop_.RunAllPending();
   }
 
-  void FinishRead() {
-    loader()->didReceiveData(url_loader(), data_, kDataSize, kDataSize);
+  void ReceiveData(int size) {
+    scoped_array<char> data(new char[size]);
+    memset(data.get(), 0xA5, size);  // Arbitrary non-zero value.
+
+    loader()->didReceiveData(url_loader(), data.get(), size, size);
     message_loop_.RunAllPending();
   }
 
   void FinishLoading() {
     data_source_->set_loading(false);
     loader()->didFinishLoading(url_loader(), 0);
     message_loop_.RunAllPending();
   }
 
   MOCK_METHOD1(ReadCallback, void(int size));
@@ skipping 28 matching lines @@
   MockWebFrameClient client_;
   WebView* view_;
 
   StrictMock<media::MockDataSourceHost> host_;
   MessageLoop message_loop_;
 
  private:
   // Used for calling BufferedDataSource::Read().
   uint8 buffer_[kDataSize];
 
-  // Used for calling BufferedResourceLoader::didReceiveData().
-  char data_[kDataSize];
-
   DISALLOW_COPY_AND_ASSIGN(BufferedDataSourceTest);
 };
 
 TEST_F(BufferedDataSourceTest, Range_Supported) {
   Initialize(media::PIPELINE_OK);
 
   EXPECT_CALL(host_, SetTotalBytes(response_generator_.content_length()));
   Respond(response_generator_.Generate206(0));
 
   EXPECT_TRUE(data_source_->loading());
@@ skipping 220 matching lines @@
   EXPECT_TRUE(data_source_->loading());
   EXPECT_CALL(*this, ReadCallback(media::DataSource::kReadError));
   Stop();
 }
 
 TEST_F(BufferedDataSourceTest, Read) {
   InitializeWith206Response();
 
   ReadAt(0);
 
-  // When the read completes we'll update our network status.
+  // Receive first half of the read.
+  EXPECT_CALL(host_, AddBufferedByteRange(0, (kDataSize / 2) - 1));
+  ReceiveData(kDataSize / 2);
+
+  // Receive last half of the read.
+  EXPECT_CALL(*this, ReadCallback(kDataSize));
   EXPECT_CALL(host_, AddBufferedByteRange(0, kDataSize - 1));
-  EXPECT_CALL(*this, ReadCallback(kDataSize));
-  FinishRead();
-  EXPECT_TRUE(data_source_->downloading());
-
-  // During teardown we'll also report our final network status.
-  EXPECT_CALL(host_, AddBufferedByteRange(0, kDataSize - 1));
+  ReceiveData(kDataSize / 2);
 
   EXPECT_TRUE(data_source_->downloading());
   Stop();
-  EXPECT_FALSE(data_source_->downloading());
+  // XXXXXXXXXXXX Should we report something on didFail? The idea here is that
+  // by calling Stop() on the object no more CBs should execute
+  EXPECT_TRUE(data_source_->downloading());

[scherkus (not reviewing), 2012/07/09 20:35:48]

ATTN!

[Ami GONE FROM CHROMIUM, 2012/07/10 02:40:14]

This change seems wrong.  Why are we still downloading() after a Stop()/didFail?

[Ami GONE FROM CHROMIUM, 2012/07/10 02:40:14]

Shouldn't BDS be firing a ReadCB(kReadError) in case of BRL::didFail()?

[scherkus (not reviewing), 2012/07/10 03:22:20]

It does and we cover that case somewhat indirectly in the
SetBitrate/SetPlaybackRate tests above. This test doesn't have a pending read
when the test ends (see line 448 where we expect our read to be fulfilled)

[scherkus (not reviewing), 2012/07/10 03:22:20]

We're not really "downloading" per se -- it's our cached value of downloading_
that we're inspecting here.

Should the API contract when calling data_source_->Stop() be that
downloading_cb_.Run(false) is executed?

Being the people that called data_source_->Stop() in the first place I'd assume
that we know we're not downloading anymore.

 }
 
 }  // namespace webkit_media