Don't share ResourceDispatcherHostImpl's timer for reporting upload progress.

content/browser/loader/resource_loader_unittest.cc

 // Copyright (c) 2013 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 "content/browser/loader/resource_loader.h"
 
 #include "base/files/file.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/run_loop.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/loader/redirect_to_file_resource_handler.h"
 #include "content/browser/loader/resource_loader_delegate.h"
 #include "content/public/browser/client_certificate_delegate.h"
 #include "content/public/browser/resource_request_info.h"
+#include "content/public/common/content_paths.h"
 #include "content/public/common/resource_response.h"
 #include "content/public/test/mock_resource_context.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_renderer_host.h"
 #include "content/test/test_content_browser_client.h"
 #include "content/test/test_web_contents.h"
 #include "ipc/ipc_message.h"
+#include "net/base/chunked_upload_data_stream.h"
 #include "net/base/io_buffer.h"
 #include "net/base/mock_file_stream.h"
 #include "net/base/net_errors.h"
 #include "net/base/request_priority.h"
+#include "net/base/upload_bytes_element_reader.h"
 #include "net/cert/x509_certificate.h"
 #include "net/ssl/client_cert_store.h"
 #include "net/ssl/ssl_cert_request_info.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_job_factory.h"
 #include "net/url_request/url_request_job_factory_impl.h"
 #include "net/url_request/url_request_test_job.h"
 #include "net/url_request/url_request_test_util.h"
 #include "storage/browser/blob/shareable_file_reference.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using storage::ShareableFileReference;
 
@@ skipping 126 matching lines @@
   explicit ResourceHandlerStub(net::URLRequest* request)
       : ResourceHandler(request),
         read_buffer_(new net::IOBuffer(kReadBufSize)),
         defer_request_on_will_start_(false),
         expect_reads_(true),
         cancel_on_read_completed_(false),
         defer_eof_(false),
         received_on_will_read_(false),
         received_eof_(false),
         received_response_completed_(false),
-        total_bytes_downloaded_(0) {
+        total_bytes_downloaded_(0),
+        upload_position_(0) {
   }
 
   // If true, defers the resource load in OnWillStart.
   void set_defer_request_on_will_start(bool defer_request_on_will_start) {
     defer_request_on_will_start_ = defer_request_on_will_start;
   }
 
   // If true, expect OnWillRead / OnReadCompleted pairs for handling
   // data. Otherwise, expect OnDataDownloaded.
   void set_expect_reads(bool expect_reads) { expect_reads_ = expect_reads; }
@@ skipping 11 matching lines @@
   bool received_response_completed() const {
     return received_response_completed_;
   }
   const net::URLRequestStatus& status() const { return status_; }
   int total_bytes_downloaded() const { return total_bytes_downloaded_; }
 
   void Resume() {
     controller()->Resume();
   }
 
+  // Waits until OnUploadProgress is called and returns the upload position.
+  uint64 WaitForUploadProgress() {
+    wait_for_progress_loop_.reset(new base::RunLoop());
+    wait_for_progress_loop_->Run();
+    wait_for_progress_loop_.reset();
+    return upload_position_;
+  }
+
   // ResourceHandler implementation:
   bool OnUploadProgress(uint64 position, uint64 size) override {
-    NOTREACHED();
+    EXPECT_LE(position, size);
+    EXPECT_GE(position, upload_position_);

[mmenke, 2015/05/22 15:15:08]

This should be GT, right?  We never call it with the same value, and we also
don't call it when position is 0 (Since ResourceLoader starts with a
last_upload_position_ of 0).

[Andre, 2015/05/22 17:24:45]

You're right, fixed.

+    upload_position_ = position;
+    if (wait_for_progress_loop_)
+      wait_for_progress_loop_->Quit();
     return true;
   }
 
   bool OnRequestRedirected(const net::RedirectInfo& redirect_info,
                            ResourceResponse* response,
                            bool* defer) override {
     NOTREACHED();
     return true;
   }
 
@@ skipping 71 matching lines @@
   bool cancel_on_read_completed_;
   bool defer_eof_;
 
   GURL start_url_;
   scoped_refptr<ResourceResponse> response_;
   bool received_on_will_read_;
   bool received_eof_;
   bool received_response_completed_;
   net::URLRequestStatus status_;
   int total_bytes_downloaded_;
+  scoped_ptr<base::RunLoop> wait_for_progress_loop_;
+  uint64 upload_position_;
 };
 
 // Test browser client that captures calls to SelectClientCertificates and
 // records the arguments of the most recent call for later inspection.
 class SelectCertificateBrowserClient : public TestContentBrowserClient {
  public:
   SelectCertificateBrowserClient() : call_count_(0) {}
 
   void SelectClientCertificate(
       WebContents* web_contents,
@@ skipping 30 matching lines @@
   }
 
   void SetClientCertStore(scoped_ptr<net::ClientCertStore> store) {
     dummy_cert_store_ = store.Pass();
   }
 
  private:
   scoped_ptr<net::ClientCertStore> dummy_cert_store_;
 };
 
+// Wraps a ChunkedUploadDataStream to behave as non-chunked to enable upload
+// progress reporting.
+class NonChunkedUploadDataStream : public net::UploadDataStream {

[mmenke, 2015/05/22 15:15:08]

Great idea!

+ public:
+  explicit NonChunkedUploadDataStream(uint64 size)
+      : net::UploadDataStream(false, 0), stream_(0), size_(size) {}
+
+  void AppendData(const char* data) {
+    stream_.AppendData(data, strlen(data), false);
+  }
+
+ private:
+  int InitInternal() override {
+    SetSize(size_);
+    stream_.Init(base::Bind(&NonChunkedUploadDataStream::OnInitCompleted,
+                            base::Unretained(this)));
+    return net::OK;
+  }
+
+  int ReadInternal(net::IOBuffer* buf, int buf_len) override {
+    return stream_.Read(buf, buf_len,
+                        base::Bind(&NonChunkedUploadDataStream::OnReadCompleted,
+                                   base::Unretained(this)));
+  }
+
+  void ResetInternal() override { stream_.Reset(); }
+
+  net::ChunkedUploadDataStream stream_;
+  uint64 size_;
+
+  DISALLOW_COPY_AND_ASSIGN(NonChunkedUploadDataStream);
+};
+
 // Fails to create a temporary file with the given error.
 void CreateTemporaryError(
     base::File::Error error,
     const CreateTemporaryFileStreamCallback& callback) {
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(callback, error, base::Passed(scoped_ptr<net::FileStream>()),
                  scoped_refptr<ShareableFileReference>()));
 }
 
 }  // namespace
 
 class ResourceLoaderTest : public testing::Test,
                            public ResourceLoaderDelegate {
  protected:
   ResourceLoaderTest()
     : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
       resource_context_(&test_url_request_context_),
       raw_ptr_resource_handler_(NULL),
       raw_ptr_to_request_(NULL) {
     test_url_request_context_.set_job_factory(&job_factory_);
   }
 
   GURL test_url() const { return net::URLRequestTestJob::test_url_1(); }
 
   std::string test_data() const {
     return net::URLRequestTestJob::test_data_1();
   }
 
+  // Waits until upload progress reaches |target_position|
+  void WaitForUploadProgress(uint64 target_position) {
+    while (true) {
+      uint64 position = raw_ptr_resource_handler_->WaitForUploadProgress();
+      EXPECT_LE(position, target_position);
+      loader_->OnUploadProgressACK();
+      if (position == target_position)
+        break;
+    }
+  }
+
   virtual net::URLRequestJobFactory::ProtocolHandler* CreateProtocolHandler() {
     return net::URLRequestTestJob::CreateProtocolHandler();
   }
 
   virtual scoped_ptr<ResourceHandler> WrapResourceHandler(
       scoped_ptr<ResourceHandlerStub> leaf_handler,
       net::URLRequest* request) {
     return leaf_handler.Pass();
   }
 
-  void SetUp() override {
-    job_factory_.SetProtocolHandler("test", CreateProtocolHandler());
+  // Replaces loader_ with a new one for |request|.
+  void SetUpResourceLoader(scoped_ptr<net::URLRequest> request) {
+    raw_ptr_to_request_ = request.get();
 
-    browser_context_.reset(new TestBrowserContext());
-    scoped_refptr<SiteInstance> site_instance =
-        SiteInstance::Create(browser_context_.get());
-    web_contents_.reset(
-        TestWebContents::Create(browser_context_.get(), site_instance.get()));
     RenderFrameHost* rfh = web_contents_->GetMainFrame();
-
-    scoped_ptr<net::URLRequest> request(
-        resource_context_.GetRequestContext()->CreateRequest(
-            test_url(),
-            net::DEFAULT_PRIORITY,
-            NULL /* delegate */));
-    raw_ptr_to_request_ = request.get();
     ResourceRequestInfo::AllocateForTesting(
         request.get(), RESOURCE_TYPE_MAIN_FRAME, &resource_context_,
         rfh->GetProcess()->GetID(), rfh->GetRenderViewHost()->GetRoutingID(),
         rfh->GetRoutingID(), true /* is_main_frame */,
         false /* parent_is_main_frame */, true /* allow_download */,
         false /* is_async */);
     scoped_ptr<ResourceHandlerStub> resource_handler(
         new ResourceHandlerStub(request.get()));
     raw_ptr_resource_handler_ = resource_handler.get();
     loader_.reset(new ResourceLoader(
         request.Pass(),
         WrapResourceHandler(resource_handler.Pass(), raw_ptr_to_request_),
         this));
   }
 
+  void SetUp() override {
+    job_factory_.SetProtocolHandler("test", CreateProtocolHandler());
+
+    browser_context_.reset(new TestBrowserContext());
+    scoped_refptr<SiteInstance> site_instance =
+        SiteInstance::Create(browser_context_.get());
+    web_contents_.reset(
+        TestWebContents::Create(browser_context_.get(), site_instance.get()));
+
+    scoped_ptr<net::URLRequest> request(
+        resource_context_.GetRequestContext()->CreateRequest(
+            test_url(),
+            net::DEFAULT_PRIORITY,
+            nullptr /* delegate */));
+    SetUpResourceLoader(request.Pass());
+  }
+
   void TearDown() override {
     // Destroy the WebContents and pump the event loop before destroying
     // |rvh_test_enabler_| and |thread_bundle_|. This lets asynchronous cleanup
     // tasks complete.
     web_contents_.reset();
     base::RunLoop().RunUntilIdle();
   }
 
   // ResourceLoaderDelegate:
   ResourceDispatcherHostLoginDelegate* CreateLoginDelegate(
@@ skipping 199 matching lines @@
   EXPECT_FALSE(raw_ptr_resource_handler_->received_response_completed());
 
   raw_ptr_resource_handler_->Resume();
   base::RunLoop().RunUntilIdle();
 
   EXPECT_TRUE(raw_ptr_resource_handler_->received_response_completed());
   EXPECT_EQ(net::URLRequestStatus::SUCCESS,
             raw_ptr_resource_handler_->status().status());
 }
 
+// Tests that progress is reported correctly while uploading.
+// TODO(andresantoso): Add test for the redirect case.
+TEST_F(ResourceLoaderTest, UploadProgress) {
+  // Set up a test server.
+  net::test_server::EmbeddedTestServer server;
+  ASSERT_TRUE(server.InitializeAndWaitUntilReady());
+  base::FilePath path;
+  PathService::Get(content::DIR_TEST_DATA, &path);
+  server.ServeFilesFromDirectory(path);
+
+  scoped_ptr<net::URLRequest> request(
+      resource_context_.GetRequestContext()->CreateRequest(
+          server.GetURL("/title1.html"),
+          net::DEFAULT_PRIORITY,
+          nullptr /* delegate */));
+
+  // Start an upload.
+  auto stream = new NonChunkedUploadDataStream(10);
+  request->set_upload(make_scoped_ptr(stream));
+
+  SetUpResourceLoader(request.Pass());
+  loader_->StartRequest();
+
+  stream->AppendData("xx");
+  WaitForUploadProgress(2);
+
+  stream->AppendData("yyy");
+  WaitForUploadProgress(5);
+
+  stream->AppendData("zzzzz");
+  WaitForUploadProgress(10);
+}
+
 class ResourceLoaderRedirectToFileTest : public ResourceLoaderTest {
  public:
   ResourceLoaderRedirectToFileTest()
       : file_stream_(NULL),
         redirect_to_file_resource_handler_(NULL) {
   }
 
   base::FilePath temp_path() const { return temp_path_; }
   ShareableFileReference* deletable_file() const {
     return deletable_file_.get();
@@ skipping 231 matching lines @@
   ASSERT_TRUE(base::ReadFileToString(temp_path(), &contents));
   EXPECT_EQ(test_data(), contents);
 
   // Release the loader. The file should be gone now.
   ReleaseLoader();
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(base::PathExists(temp_path()));
 }
 
 }  // namespace content