Implement upload progress handling in Mojo loading

content/browser/loader/mojo_async_resource_handler_unittest.cc

Index: content/browser/loader/mojo_async_resource_handler_unittest.cc
diff --git a/content/browser/loader/mojo_async_resource_handler_unittest.cc b/content/browser/loader/mojo_async_resource_handler_unittest.cc
index fa69c8d39c892f63e8b07f152b017287d7dacb0f..17716bef0df110b91af679f2f1c4a0494b1af4d7 100644
--- a/content/browser/loader/mojo_async_resource_handler_unittest.cc
+++ b/content/browser/loader/mojo_async_resource_handler_unittest.cc
@@ -44,6 +44,7 @@
 #include "net/http/http_status_code.h"
 #include "net/http/http_util.h"
 #include "net/ssl/client_cert_store.h"
+#include "net/test/url_request/url_request_mock_data_job.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_status.h"
@@ -56,6 +57,33 @@ namespace {
 
 constexpr int kSizeMimeSnifferRequiresForFirstOnWillRead = 2048;
 
+class FakeUploadDataStream : public net::UploadDataStream {
+ public:
+  FakeUploadDataStream(int64_t position, int64_t size)
+      : UploadDataStream(false, 0), position_(position), size_(size) {}
+
+  void set_position(int64_t position) { position_ = position; }
+  int InitInternal(const net::NetLogWithSource& net_log) override {
+    NOTREACHED();
+    return 0;
+  }
+  int ReadInternal(net::IOBuffer* buf, int buf_len) override {
+    NOTREACHED();
+    return 0;
+  }
+  void ResetInternal() override { NOTREACHED(); }
+
+  net::UploadProgress GetUploadProgress() const override {
+    return net::UploadProgress(position_, size_);
+  }
+
+ private:
+  int64_t position_;
+  int64_t size_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeUploadDataStream);
+};
+
 class TestResourceDispatcherHostDelegate final
     : public ResourceDispatcherHostDelegate {
  public:
@@ -278,13 +306,19 @@ class MojoAsyncResourceHandlerTestBase {
         browser_context_(new TestBrowserContext()) {
     MojoAsyncResourceHandler::SetAllocationSizeForTesting(32 * 1024);
     rdh_.SetDelegate(&rdh_delegate_);
+  }
+
+  void SetUpRequest() { SetUpRequestWithUpload(nullptr); }
 
+  void SetUpRequestWithUpload(
+      std::unique_ptr<net::UploadDataStream> upload_stream) {
     // Create and initialize |request_|.  None of this matters, for these tests,
     // just need something non-NULL.
     net::URLRequestContext* request_context =
         browser_context_->GetResourceContext()->GetRequestContext();
     request_ = request_context->CreateRequest(
         GURL("http://foo/"), net::DEFAULT_PRIORITY, &url_request_delegate_);
+    request_->set_upload(std::move(upload_stream));
     ResourceRequestInfo::AllocateForTesting(
         request_.get(),                          // request
         RESOURCE_TYPE_XHR,                       // resource_type
@@ -382,17 +416,20 @@ class MojoAsyncResourceHandlerWithAllocationSizeTest
 };
 
 TEST_F(MojoAsyncResourceHandlerTest, InFlightRequests) {
+  SetUpRequest();

[mmenke, 2017/01/18 17:00:46]

Can we do this in the constructor instead, and make SetUpRequestWithUpload just
overwrite the default objects?

[tzik, 2017/01/19 11:53:09]

Done.

   EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
   handler_ = nullptr;
   EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnWillStart) {
+  SetUpRequest();
   EXPECT_EQ(MockResourceLoader::Status::IDLE,
             mock_loader_->OnWillStart(request_->url()));
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnResponseStarted) {
+  SetUpRequest();
   rdh_delegate_.set_num_on_response_started_calls_expectation(1);
   scoped_refptr<net::IOBufferWithSize> metadata = new net::IOBufferWithSize(5);
   memcpy(metadata->data(), "hello", 5);
@@ -431,6 +468,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OnResponseStarted) {
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnWillReadAndInFlightRequests) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
@@ -440,6 +478,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OnWillReadAndInFlightRequests) {
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnWillReadWithInsufficientResource) {
+  SetUpRequest();
   rdh_.set_max_num_in_flight_requests_per_process(0);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
@@ -451,6 +490,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OnWillReadWithInsufficientResource) {
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnWillReadAndOnReadCompleted) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   // The buffer size that the mime sniffer requires implicitly.
@@ -481,6 +521,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OnWillReadAndOnReadCompleted) {
 
 TEST_F(MojoAsyncResourceHandlerTest,
        OnWillReadAndOnReadCompletedWithInsufficientInitialCapacity) {
+  SetUpRequest();
   MojoAsyncResourceHandler::SetAllocationSizeForTesting(2);
 
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
@@ -517,6 +558,7 @@ TEST_F(MojoAsyncResourceHandlerTest,
 
 TEST_F(MojoAsyncResourceHandlerTest,
        IOBufferFromOnWillReadShouldRemainValidEvenIfHandlerIsGone) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   // The io_buffer size that the mime sniffer requires implicitly.
@@ -532,6 +574,7 @@ TEST_F(MojoAsyncResourceHandlerTest,
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnResponseCompleted) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
   ResourceRequestInfoImpl::ForRequest(request_.get())
@@ -555,6 +598,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OnResponseCompleted) {
 
 // This test case sets different status values from OnResponseCompleted.
 TEST_F(MojoAsyncResourceHandlerTest, OnResponseCompleted2) {
+  SetUpRequest();
   rdh_.SetDelegate(nullptr);
   // Don't use CallOnWillStartAndOnResponseStarted as this test case manually
   // sets the null delegate.
@@ -588,6 +632,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OnResponseCompleted2) {
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnResponseCompletedWithCanceledTimedOut) {
+  SetUpRequest();
   net::URLRequestStatus status(net::URLRequestStatus::CANCELED,
                                net::ERR_TIMED_OUT);
 
@@ -602,6 +647,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OnResponseCompletedWithCanceledTimedOut) {
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnResponseCompletedWithFailedTimedOut) {
+  SetUpRequest();
   net::URLRequestStatus status(net::URLRequestStatus::FAILED,
                                net::ERR_TIMED_OUT);
 
@@ -616,6 +662,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OnResponseCompletedWithFailedTimedOut) {
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, ResponseCompletionShouldCloseDataPipe) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
@@ -644,6 +691,7 @@ TEST_F(MojoAsyncResourceHandlerTest, ResponseCompletionShouldCloseDataPipe) {
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OutOfBandCancelDuringBodyTransmission) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
@@ -683,6 +731,7 @@ TEST_F(MojoAsyncResourceHandlerTest, OutOfBandCancelDuringBodyTransmission) {
 
 // In this case, an error is notified after OnWillRead, before OnReadCompleted.
 TEST_F(MojoAsyncResourceHandlerTest, ResponseErrorDuringBodyTransmission2) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
@@ -710,12 +759,14 @@ TEST_F(MojoAsyncResourceHandlerTest, ResponseErrorDuringBodyTransmission2) {
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, BeginWriteFailsOnWillRead) {
+  SetUpRequest();
   handler_->set_begin_write_expectation(MOJO_RESULT_UNKNOWN);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::CANCELED, mock_loader_->OnWillRead(-1));
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, BeginWriteReturnsShouldWaitOnWillRead) {
+  SetUpRequest();
   handler_->set_begin_write_expectation(MOJO_RESULT_SHOULD_WAIT);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
@@ -723,6 +774,7 @@ TEST_F(MojoAsyncResourceHandlerTest, BeginWriteReturnsShouldWaitOnWillRead) {
 
 TEST_F(MojoAsyncResourceHandlerTest,
        BeginWriteReturnsShouldWaitOnWillReadAndThenReturnsOK) {
+  SetUpRequest();
   handler_->set_begin_write_expectation(MOJO_RESULT_SHOULD_WAIT);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   size_t written = 0;
@@ -760,6 +812,7 @@ TEST_F(MojoAsyncResourceHandlerTest,
 
 TEST_F(MojoAsyncResourceHandlerTest,
        EndWriteFailsOnWillReadWithInsufficientInitialCapacity) {
+  SetUpRequest();
   MojoAsyncResourceHandler::SetAllocationSizeForTesting(2);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   handler_->set_end_write_expectation(MOJO_RESULT_UNKNOWN);
@@ -767,6 +820,7 @@ TEST_F(MojoAsyncResourceHandlerTest,
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, EndWriteFailsOnReadCompleted) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
 
@@ -778,6 +832,7 @@ TEST_F(MojoAsyncResourceHandlerTest, EndWriteFailsOnReadCompleted) {
 
 TEST_F(MojoAsyncResourceHandlerTest,
        EndWriteFailsOnReadCompletedWithInsufficientInitialCapacity) {
+  SetUpRequest();
   MojoAsyncResourceHandler::SetAllocationSizeForTesting(2);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
@@ -790,6 +845,7 @@ TEST_F(MojoAsyncResourceHandlerTest,
 
 TEST_F(MojoAsyncResourceHandlerTest,
        EndWriteFailsOnResumeWithInsufficientInitialCapacity) {
+  SetUpRequest();
   MojoAsyncResourceHandler::SetAllocationSizeForTesting(8);
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
@@ -824,8 +880,30 @@ TEST_F(MojoAsyncResourceHandlerTest,
   EXPECT_EQ(net::ERR_FAILED, mock_loader_->error_code());
 }
 
+TEST_F(MojoAsyncResourceHandlerTest, UploadProgressHandling) {
+  net::URLRequestMockDataJob::AddUrlHandlerForHostname("foo");
+  FakeUploadDataStream* upload = new FakeUploadDataStream(0, 1000);
+  SetUpRequestWithUpload(base::WrapUnique(upload));
+  request_->Start();
+
+  net::UploadProgress progress = request_->GetUploadProgress();
+  EXPECT_EQ(0u, progress.position());
+  EXPECT_EQ(1000u, progress.size());
+
+  EXPECT_EQ(0, url_loader_client_.current_upload_position());
+  EXPECT_EQ(0, url_loader_client_.total_upload_size());
+
+  upload->set_position(1000);
+  ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());

[mmenke, 2017/01/18 17:00:46]

So there are three paths to update the status, so I suggest three tests total,
one for each path.  We of course don't need to fully test the
UploadProgressTracker, but we should test the three places it's hooked up.

One where there's a redirect, make sure we get the new position without calling
ResponseStarted.

One where enough time passes that we get an update without the ResponseStarted
call (Could set the position to something less than 1000 in that one).

[tzik, 2017/01/19 11:53:09]

Done.

+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_EQ(1000, url_loader_client_.current_upload_position());
+  EXPECT_EQ(1000, url_loader_client_.total_upload_size());
+}
+
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        OnWillReadWithLongContents) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   std::string expected;
@@ -874,6 +952,7 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        BeginWriteFailsOnReadCompleted) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
 
@@ -885,6 +964,7 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        BeginWriteReturnsShouldWaitOnReadCompleted) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
 
@@ -896,6 +976,7 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        BeginWriteFailsOnResume) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnWillRead(-1));
   ASSERT_EQ(MockResourceLoader::Status::IDLE, mock_loader_->OnReadCompleted(0));
@@ -927,6 +1008,7 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
 }
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest, CancelWhileWaiting) {
+  SetUpRequest();
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());
 
   while (true) {
@@ -968,6 +1050,7 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest, CancelWhileWaiting) {
 }
 
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest, RedirectHandling) {
+  SetUpRequest();
   rdh_delegate_.set_num_on_response_started_calls_expectation(1);
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
@@ -1033,6 +1116,7 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest, RedirectHandling) {
 // redirect, despite the fact that no redirect has been received yet.
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        MalformedFollowRedirectRequest) {
+  SetUpRequest();
   handler_->FollowRedirect();
 
   EXPECT_TRUE(handler_->has_received_bad_message());
@@ -1042,6 +1126,7 @@ TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
 TEST_P(
     MojoAsyncResourceHandlerWithAllocationSizeTest,
     OnWillStartThenOnResponseStartedThenOnWillReadThenOnReadCompletedThenOnResponseCompleted) {
+  SetUpRequest();
   rdh_delegate_.set_num_on_response_started_calls_expectation(1);
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE,
@@ -1092,6 +1177,7 @@ TEST_P(
 TEST_P(
     MojoAsyncResourceHandlerWithAllocationSizeTest,
     OnWillStartThenOnWillReadThenOnResponseStartedThenOnReadCompletedThenOnResponseCompleted) {
+  SetUpRequest();
   rdh_delegate_.set_num_on_response_started_calls_expectation(1);
 
   ASSERT_EQ(MockResourceLoader::Status::IDLE,