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 bcd730abd9f2d092e1ea040d4f37b8c95a5945de..cbdd66e0e76fe29754dee0cbf4764d71ba9b6898 100644
--- a/content/browser/loader/mojo_async_resource_handler_unittest.cc
+++ b/content/browser/loader/mojo_async_resource_handler_unittest.cc
@@ -15,6 +15,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
 #include "base/run_loop.h"
+#include "base/test/test_simple_task_runner.h"
 #include "content/browser/loader/mock_resource_loader.h"
 #include "content/browser/loader/resource_controller.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
@@ -44,6 +45,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 +58,37 @@ namespace {
 
 constexpr int kSizeMimeSnifferRequiresForFirstOnWillRead = 2048;
 
+class DummyUploadDataStream : public net::UploadDataStream {
+ public:
+  DummyUploadDataStream() : UploadDataStream(false, 0) {}
+
+  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(); }

[mmenke, 2017/01/19 16:06:49]

nit:  include base/logging.h

[tzik, 2017/01/20 04:42:03]

Done.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DummyUploadDataStream);

[mmenke, 2017/01/19 16:06:49]

include base/macros.h

[tzik, 2017/01/20 04:42:03]

Done.

+};
+
+class FakeUploadProgressTracker : public UploadProgressTracker {
+ public:
+  using UploadProgressTracker::UploadProgressTracker;
+
+  net::UploadProgress GetUploadProgress() const override {
+    return upload_progress_;
+  }
+  base::TimeTicks GetCurrentTime() const override { return current_time_; }
+
+  net::UploadProgress upload_progress_;
+  base::TimeTicks current_time_;

[mmenke, 2017/01/19 16:06:49]

private:
 DISALLOW_COPY_AND_ASSIGN?

[tzik, 2017/01/20 04:42:03]

Done.

+};
+
 class TestResourceDispatcherHostDelegate final
     : public ResourceDispatcherHostDelegate {
  public:
@@ -186,7 +219,8 @@ class MojoAsyncResourceHandlerWithStubOperations
       : MojoAsyncResourceHandler(request,
                                  rdh,
                                  std::move(mojo_request),
-                                 std::move(url_loader_client)) {}
+                                 std::move(url_loader_client)),
+        task_runner_(new base::TestSimpleTaskRunner) {}
   ~MojoAsyncResourceHandlerWithStubOperations() override {}
 
   void ResetBeginWriteExpectation() { is_begin_write_expectation_set_ = false; }
@@ -204,6 +238,15 @@ class MojoAsyncResourceHandlerWithStubOperations
     metadata_ = std::move(metadata);
   }
 
+  FakeUploadProgressTracker* upload_progress_tracker() const {
+    return upload_progress_tracker_;
+  }
+
+  void PollUploadProgress() {
+    task_runner_->RunPendingTasks();
+    base::RunLoop().RunUntilIdle();
+  }
+
  private:
   MojoResult BeginWrite(void** data, uint32_t* available) override {
     if (is_begin_write_expectation_set_)
@@ -224,6 +267,17 @@ class MojoAsyncResourceHandlerWithStubOperations
     has_received_bad_message_ = true;
   }
 
+  std::unique_ptr<UploadProgressTracker> CreateUploadProgressTracker(
+      const tracked_objects::Location& from_here,
+      UploadProgressTracker::UploadProgressReportCallback callback) override {
+    DCHECK(!upload_progress_tracker_);
+
+    auto upload_progress_tracker = base::MakeUnique<FakeUploadProgressTracker>(
+        from_here, std::move(callback), request(), task_runner_);
+    upload_progress_tracker_ = upload_progress_tracker.get();
+    return std::move(upload_progress_tracker);
+  }
+
   bool is_begin_write_expectation_set_ = false;
   bool is_end_write_expectation_set_ = false;
   bool has_received_bad_message_ = false;
@@ -231,6 +285,9 @@ class MojoAsyncResourceHandlerWithStubOperations
   MojoResult end_write_expectation_ = MOJO_RESULT_UNKNOWN;
   scoped_refptr<net::IOBufferWithSize> metadata_;
 
+  FakeUploadProgressTracker* upload_progress_tracker_ = nullptr;
+  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+
   DISALLOW_COPY_AND_ASSIGN(MojoAsyncResourceHandlerWithStubOperations);
 };
 
@@ -273,7 +330,8 @@ class TestURLLoaderFactory final : public mojom::URLLoaderFactory {
 
 class MojoAsyncResourceHandlerTestBase {
  public:
-  MojoAsyncResourceHandlerTestBase()
+  explicit MojoAsyncResourceHandlerTestBase(
+      std::unique_ptr<net::UploadDataStream> upload_stream)
       : thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP),
         browser_context_(new TestBrowserContext()) {
     MojoAsyncResourceHandler::SetAllocationSizeForTesting(32 * 1024);
@@ -285,6 +343,7 @@ class MojoAsyncResourceHandlerTestBase {
         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
@@ -330,15 +389,17 @@ class MojoAsyncResourceHandlerTestBase {
   }
 
   // Returns false if something bad happens.
-  bool CallOnWillStartAndOnResponseStarted() {
-    rdh_delegate_.set_num_on_response_started_calls_expectation(1);
+  bool CallOnWillStart() {
     MockResourceLoader::Status result =
         mock_loader_->OnWillStart(request_->url());
     EXPECT_EQ(MockResourceLoader::Status::IDLE, result);
-    if (result != MockResourceLoader::Status::IDLE)
-      return false;
+    return result == MockResourceLoader::Status::IDLE;
+  }
 
-    result = mock_loader_->OnResponseStarted(
+  // Returns false if something bad happens.
+  bool CallOnResponseStarted() {
+    rdh_delegate_.set_num_on_response_started_calls_expectation(1);
+    MockResourceLoader::Status result = mock_loader_->OnResponseStarted(
         make_scoped_refptr(new ResourceResponse()));
     EXPECT_EQ(MockResourceLoader::Status::IDLE, result);
     if (result != MockResourceLoader::Status::IDLE)
@@ -352,6 +413,18 @@ class MojoAsyncResourceHandlerTestBase {
     return true;
   }
 
+  // Returns false if something bad happens.
+  bool CallOnWillStartAndOnResponseStarted() {
+    return CallOnWillStart() && CallOnResponseStarted();
+  }
+
+  void set_upload_progress(const net::UploadProgress& upload_progress) {
+    handler_->upload_progress_tracker()->upload_progress_ = upload_progress;
+  }
+  void AdvanceCurrentTime(const base::TimeDelta& delta) {
+    handler_->upload_progress_tracker()->current_time_ += delta;
+  }
+
   TestBrowserThreadBundle thread_bundle_;
   TestResourceDispatcherHostDelegate rdh_delegate_;
   ResourceDispatcherHostImpl rdh_;
@@ -368,7 +441,10 @@ class MojoAsyncResourceHandlerTestBase {
 };
 
 class MojoAsyncResourceHandlerTest : public MojoAsyncResourceHandlerTestBase,
-                                     public ::testing::Test {};
+                                     public ::testing::Test {
+ protected:
+  MojoAsyncResourceHandlerTest() : MojoAsyncResourceHandlerTestBase(nullptr) {}
+};
 
 // This test class is parameterized with MojoAsyncResourceHandler's allocation
 // size.
@@ -376,11 +452,21 @@ class MojoAsyncResourceHandlerWithAllocationSizeTest
     : public MojoAsyncResourceHandlerTestBase,
       public ::testing::TestWithParam<size_t> {
  protected:
-  MojoAsyncResourceHandlerWithAllocationSizeTest() {
+  MojoAsyncResourceHandlerWithAllocationSizeTest()
+      : MojoAsyncResourceHandlerTestBase(nullptr) {
     MojoAsyncResourceHandler::SetAllocationSizeForTesting(GetParam());
   }
 };
 
+class MojoAsyncResourceHandlerUploadTest
+    : public MojoAsyncResourceHandlerTestBase,
+      public ::testing::Test {
+ protected:
+  MojoAsyncResourceHandlerUploadTest()
+      : MojoAsyncResourceHandlerTestBase(
+            base::MakeUnique<DummyUploadDataStream>()) {}
+};
+
 TEST_F(MojoAsyncResourceHandlerTest, InFlightRequests) {
   EXPECT_EQ(0, rdh_.num_in_flight_requests_for_testing());
   handler_ = nullptr;
@@ -428,6 +514,8 @@ TEST_F(MojoAsyncResourceHandlerTest, OnResponseStarted) {
 
   url_loader_client_.RunUntilCachedMetadataReceived();
   EXPECT_EQ("hello", url_loader_client_.cached_metadata());
+
+  EXPECT_FALSE(url_loader_client_.has_received_upload_progress());
 }
 
 TEST_F(MojoAsyncResourceHandlerTest, OnWillReadAndInFlightRequests) {
@@ -798,6 +886,59 @@ TEST_F(MojoAsyncResourceHandlerTest,
   EXPECT_EQ(net::ERR_FAILED, mock_loader_->error_code());
 }
 
+TEST_F(MojoAsyncResourceHandlerUploadTest, UploadProgressHandling) {
+  ASSERT_TRUE(CallOnWillStart());
+
+  // Expect no report for no progress.
+  set_upload_progress(net::UploadProgress(0, 1000));
+  handler_->PollUploadProgress();
+  EXPECT_FALSE(url_loader_client_.has_received_upload_progress());
+  EXPECT_EQ(0, url_loader_client_.current_upload_position());
+  EXPECT_EQ(0, url_loader_client_.total_upload_size());
+
+  // Expect a upload progress report for a good amount of progress.
+  url_loader_client_.reset_has_received_upload_progress();
+  set_upload_progress(net::UploadProgress(100, 1000));
+  handler_->PollUploadProgress();
+  EXPECT_TRUE(url_loader_client_.has_received_upload_progress());
+  EXPECT_EQ(100, url_loader_client_.current_upload_position());
+  EXPECT_EQ(1000, url_loader_client_.total_upload_size());
+
+  // Expect a upload progress report for the passed time.
+  url_loader_client_.reset_has_received_upload_progress();
+  set_upload_progress(net::UploadProgress(101, 1000));
+  AdvanceCurrentTime(base::TimeDelta::FromSeconds(5));
+  handler_->PollUploadProgress();
+  EXPECT_TRUE(url_loader_client_.has_received_upload_progress());
+  EXPECT_EQ(101, url_loader_client_.current_upload_position());
+  EXPECT_EQ(1000, url_loader_client_.total_upload_size());
+
+  // A redirect rewinds the upload progress. Expect no report for the rewound
+  // progress.

[mmenke, 2017/01/19 16:06:49]

Oops, sorry...When I said rewind, I actually mean a check for for both the
OnResponseStarted and OnResponseCompleted OnUploadCompleted() calls (So you will
need another test, or another upload in this test, to cover them body).  Not
that I object to the extra coverage in this test - looks great!

+  url_loader_client_.reset_has_received_upload_progress();
+  set_upload_progress(net::UploadProgress(0, 1000));
+  AdvanceCurrentTime(base::TimeDelta::FromSeconds(5));
+  handler_->PollUploadProgress();
+  EXPECT_FALSE(url_loader_client_.has_received_upload_progress());
+
+  // Set the progress to almost-finished state to prepare for the completion
+  // report below.
+  url_loader_client_.reset_has_received_upload_progress();
+  set_upload_progress(net::UploadProgress(999, 1000));
+  handler_->PollUploadProgress();
+  EXPECT_TRUE(url_loader_client_.has_received_upload_progress());
+  EXPECT_EQ(999, url_loader_client_.current_upload_position());
+  EXPECT_EQ(1000, url_loader_client_.total_upload_size());
+
+  // Expect a upload progress report for the upload completion.
+  url_loader_client_.reset_has_received_upload_progress();
+  set_upload_progress(net::UploadProgress(1000, 1000));
+  ASSERT_TRUE(CallOnResponseStarted());
+  EXPECT_TRUE(url_loader_client_.has_received_upload_progress());
+  EXPECT_EQ(1000, url_loader_client_.current_upload_position());
+  EXPECT_EQ(1000, url_loader_client_.total_upload_size());
+}
+
 TEST_P(MojoAsyncResourceHandlerWithAllocationSizeTest,
        OnWillReadWithLongContents) {
   ASSERT_TRUE(CallOnWillStartAndOnResponseStarted());