Fix URLFetcherDownloadProgressTest to work with remote test server

net/url_request/url_fetcher_impl_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 "net/url_request/url_fetcher_impl.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/file_util.h"
@@ skipping 43 matching lines @@
 class ThrottlingTestURLRequestContextGetter
     : public TestURLRequestContextGetter {
  public:
   ThrottlingTestURLRequestContextGetter(
       base::MessageLoopProxy* io_message_loop_proxy,
       TestURLRequestContext* request_context)
       : TestURLRequestContextGetter(io_message_loop_proxy),
         context_(request_context) {
   }
 
+  // TestURLRequestContextGetter:
   virtual TestURLRequestContext* GetURLRequestContext() OVERRIDE {
     return context_;
   }
 
  protected:
   virtual ~ThrottlingTestURLRequestContextGetter() {}
 
   TestURLRequestContext* const context_;
 };
 
 }  // namespace
 
 class URLFetcherTest : public testing::Test,
                        public URLFetcherDelegate {
  public:
   URLFetcherTest()
       : fetcher_(NULL),
         context_(new ThrottlingTestURLRequestContext()) {
   }
 
   static int GetNumFetcherCores() {
     return URLFetcherImpl::GetNumFetcherCores();
   }
 
   // Creates a URLFetcher, using the program's main thread to do IO.
   virtual void CreateFetcher(const GURL& url);
 
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   // Subclasses that override this should either call this function or
   // CleanupAfterFetchComplete() at the end of their processing, depending on
   // whether they want to check for a non-empty HTTP 200 response or not.
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 
   // Deletes |fetcher| and terminates the message loop.
   void CleanupAfterFetchComplete();
 
   scoped_refptr<base::MessageLoopProxy> io_message_loop_proxy() {
     return io_message_loop_proxy_;
   }
 
   TestURLRequestContext* request_context() {
     return context_.get();
   }
 
  protected:
+  // testing::Test:
   virtual void SetUp() OVERRIDE {
     testing::Test::SetUp();
 
     io_message_loop_proxy_ = base::MessageLoopProxy::current();
 
 #if defined(USE_NSS)
     crypto::EnsureNSSInit();
     EnsureNSSHttpIOInit();
 #endif
   }
@@ skipping 40 matching lines @@
   io_message_loop_proxy()->PostTask(FROM_HERE, MessageLoop::QuitClosure());
   // If the current message loop is not the IO loop, it will be shut down when
   // the main loop returns and this thread subsequently goes out of scope.
 }
 
 namespace {
 
 // Version of URLFetcherTest that does a POST instead
 class URLFetcherPostTest : public URLFetcherTest {
  public:
-  // URLFetcherTest override.
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
 
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 };
 
 // Version of URLFetcherTest that does a POST instead with empty upload body
 class URLFetcherEmptyPostTest : public URLFetcherTest {
  public:
-  // URLFetcherTest override.
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
 
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 };
 
 // Version of URLFetcherTest that tests download progress reports.
 class URLFetcherDownloadProgressTest : public URLFetcherTest {
  public:
-  // URLFetcherTest override.
+  URLFetcherDownloadProgressTest()
+      : previous_progress_(0),
+        expected_total_(0) {
+  }
+
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
 
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchDownloadProgress(const URLFetcher* source,
-                                          int64 current, int64 total) OVERRIDE;
+                                          int64 current,
+                                          int64 total) OVERRIDE;
+
  protected:
+  // Download progress returned by the previous callback.
   int64 previous_progress_;
+  // Size of the file being downloaded, known in advance (provided by each test
+  // case).
   int64 expected_total_;
 };
 
-/// Version of URLFetcherTest that tests progress reports at cancellation.
+// Version of URLFetcherTest that tests progress reports at cancellation.
 class URLFetcherDownloadProgressCancelTest : public URLFetcherTest {
  public:
-  // URLFetcherTest override.
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
 
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
   virtual void OnURLFetchDownloadProgress(const URLFetcher* source,
-                                          int64 current, int64 total) OVERRIDE;
+                                          int64 current,
+                                          int64 total) OVERRIDE;
  protected:
   bool cancelled_;
 };
 
 // Version of URLFetcherTest that tests upload progress reports.
 class URLFetcherUploadProgressTest : public URLFetcherTest {
  public:
-  virtual void CreateFetcher(const GURL& url);
+  // URLFetcherTest:
+  virtual void CreateFetcher(const GURL& url) OVERRIDE;
 
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchUploadProgress(const URLFetcher* source,
-                                        int64 current, int64 total);
+                                        int64 current,
+                                        int64 total) OVERRIDE;
  protected:
   int64 previous_progress_;
   std::string chunk_;
   int64 number_of_chunks_added_;
 };
 
 // Version of URLFetcherTest that tests headers.
 class URLFetcherHeadersTest : public URLFetcherTest {
  public:
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 };
 
 // Version of URLFetcherTest that tests SocketAddress.
 class URLFetcherSocketAddressTest : public URLFetcherTest {
  public:
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
  protected:
   std::string expected_host_;
   uint16 expected_port_;
 };
 
 // Version of URLFetcherTest that tests stopping on a redirect.
 class URLFetcherStopOnRedirectTest : public URLFetcherTest {
  public:
   URLFetcherStopOnRedirectTest();
   virtual ~URLFetcherStopOnRedirectTest();
 
-  // URLFetcherTest override.
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
-  // URLFetcherDelegate
+
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 
  protected:
   // The URL we should be redirected to.
   static const char* kRedirectTarget;
 
   bool callback_called_;  // Set to true in OnURLFetchComplete().
 };
 
 // Version of URLFetcherTest that tests overload protection.
 class URLFetcherProtectTest : public URLFetcherTest {
  public:
-  // URLFetcherTest override.
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
-  // URLFetcherDelegate
+
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
  private:
   Time start_time_;
 };
 
 // Version of URLFetcherTest that tests overload protection, when responses
 // passed through.
 class URLFetcherProtectTestPassedThrough : public URLFetcherTest {
  public:
-  // URLFetcherTest override.
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
-  // URLFetcherDelegate
+
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
  private:
   Time start_time_;
 };
 
 // Version of URLFetcherTest that tests bad HTTPS requests.
 class URLFetcherBadHTTPSTest : public URLFetcherTest {
  public:
   URLFetcherBadHTTPSTest();
 
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 
  private:
   FilePath cert_dir_;
 };
 
 // Version of URLFetcherTest that tests request cancellation on shutdown.
 class URLFetcherCancelTest : public URLFetcherTest {
  public:
-  // URLFetcherTest override.
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
-  // URLFetcherDelegate
+
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 
   void CancelRequest();
 };
 
 // Version of TestURLRequestContext that posts a Quit task to the IO
 // thread once it is deleted.
 class CancelTestURLRequestContext : public ThrottlingTestURLRequestContext {
  public:
   explicit CancelTestURLRequestContext() {
@@ skipping 11 matching lines @@
     : public TestURLRequestContextGetter {
  public:
   CancelTestURLRequestContextGetter(
       base::MessageLoopProxy* io_message_loop_proxy,
       const GURL& throttle_for_url)
       : TestURLRequestContextGetter(io_message_loop_proxy),
         io_message_loop_proxy_(io_message_loop_proxy),
         context_created_(false, false),
         throttle_for_url_(throttle_for_url) {
   }
+
+  // TestURLRequestContextGetter:
   virtual TestURLRequestContext* GetURLRequestContext() OVERRIDE {
     if (!context_.get()) {
       context_.reset(new CancelTestURLRequestContext());
       DCHECK(context_->throttler_manager());
 
       // Registers an entry for test url. The backoff time is calculated by:
       //     new_backoff = 2.0 * old_backoff + 0
       // The initial backoff is 2 seconds and maximum backoff is 4 seconds.
       // Maximum retries allowed is set to 2.
       scoped_refptr<URLRequestThrottlerEntry> entry(
           new URLRequestThrottlerEntry(
               context_->throttler_manager(),
               "", 200, 3, 2000, 2.0, 0.0, 4000));
       context_->throttler_manager()->OverrideEntryForTests(
           throttle_for_url_, entry);
 
       context_created_.Signal();
     }
     return context_.get();
   }
+
   virtual scoped_refptr<base::MessageLoopProxy> GetIOMessageLoopProxy() const {
     return io_message_loop_proxy_;
   }
+
   void WaitForContextCreation() {
     context_created_.Wait();
   }
 
  protected:
   virtual ~CancelTestURLRequestContextGetter() {}
 
  private:
   scoped_ptr<TestURLRequestContext> context_;
   scoped_refptr<base::MessageLoopProxy> io_message_loop_proxy_;
   base::WaitableEvent context_created_;
   GURL throttle_for_url_;
 };
 
 // Version of URLFetcherTest that tests retying the same request twice.
 class URLFetcherMultipleAttemptTest : public URLFetcherTest {
  public:
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
  private:
   std::string data_;
 };
 
 class URLFetcherFileTest : public URLFetcherTest {
  public:
   URLFetcherFileTest() : take_ownership_of_file_(false),
                          expected_file_error_(base::PLATFORM_FILE_OK) {}
 
   void CreateFetcherForFile(const GURL& url, const FilePath& file_path);
   void CreateFetcherForTempFile(const GURL& url);
 
-  // URLFetcherDelegate
+  // URLFetcherDelegate:
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
 
  protected:
   FilePath expected_file_;
   FilePath file_path_;
 
   // Set by the test. Used in OnURLFetchComplete() to decide if
   // the URLFetcher should own the temp file, so that we can test
   // disowning prevents the file from being deleted.
   bool take_ownership_of_file_;
@@ skipping 37 matching lines @@
 
   CleanupAfterFetchComplete();
   // Do not call the super class method URLFetcherTest::OnURLFetchComplete,
   // since it expects a non-empty response.
 }
 
 void URLFetcherDownloadProgressTest::CreateFetcher(const GURL& url) {
   fetcher_ = new URLFetcherImpl(url, URLFetcher::GET, this);
   fetcher_->SetRequestContext(new ThrottlingTestURLRequestContextGetter(
       io_message_loop_proxy(), request_context()));
-  previous_progress_ = 0;
   fetcher_->Start();
 }
 
 void URLFetcherDownloadProgressTest::OnURLFetchDownloadProgress(
-    const URLFetcher* source, int64 current, int64 total) {
+    const URLFetcher* source, int64 progress, int64 total) {

[wtc, 2012/11/29 19:35:25]

I agree that |progress| seems more informative than |current|.
The problem with renaming the argument only here is that all
the other OnURLFetchDownloadProgress declarations and
definitions still use the old argument name, so this change
created an inconsistency. Consistency is valued in our
Style Guide.

Can you look into if it is worth the trouble to rename that
argument globally in the source tree? Thanks.

   // Increasing between 0 and total.
-  EXPECT_LE(0, current);
-  EXPECT_GE(total, current);
-  EXPECT_LE(previous_progress_, current);
-  previous_progress_ = current;
+  EXPECT_LE(0, progress);
+  EXPECT_GE(total, progress);
+  EXPECT_LE(previous_progress_, progress);
   EXPECT_EQ(expected_total_, total);
+  previous_progress_ = progress;
 }
 
 void URLFetcherDownloadProgressCancelTest::CreateFetcher(const GURL& url) {
   fetcher_ = new URLFetcherImpl(url, URLFetcher::GET, this);
   fetcher_->SetRequestContext(new ThrottlingTestURLRequestContextGetter(
       io_message_loop_proxy(), request_context()));
   cancelled_ = false;
   fetcher_->Start();
 }
 
@@ skipping 379 matching lines @@
 
 TEST_F(URLFetcherDownloadProgressTest, Basic) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
   ASSERT_TRUE(test_server.Start());
 
   // Get a file large enough to require more than one read into
   // URLFetcher::Core's IOBuffer.
   static const char kFileToFetch[] = "animate1.gif";
-  file_util::GetFileSize(test_server.document_root().AppendASCII(kFileToFetch),
-                         &expected_total_);
+  // Hardcoded file size - it cannot be easily fetched when a remote http server
+  // is used for testing.
+  static const int64 kFileSize = 19021;
+
+  expected_total_ = kFileSize;
+
   CreateFetcher(test_server.GetURL(
       std::string(kTestServerFilePrefix) + kFileToFetch));
 
   MessageLoop::current()->Run();
 }
 
 TEST_F(URLFetcherDownloadProgressCancelTest, CancelWhileProgressReport) {
   TestServer test_server(TestServer::TYPE_HTTP,
                          TestServer::kLocalhost,
                          FilePath(kDocRoot));
@@ skipping 375 matching lines @@
   MessageLoop::current()->Run();  // OnURLFetchComplete() will Quit().
 
   MessageLoop::current()->RunUntilIdle();
   ASSERT_FALSE(file_util::PathExists(file_path_))
       << file_path_.value() << " not removed.";
 }
 
 }  // namespace
 
 }  // namespace net