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 168 matching lines @@
   // URLFetcherTest override.
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
 
   // 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),
+        previous_total_(0),
+        first_call_(true) {
+  }
+
+  // URLFetcherTest:
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
-
-  // URLFetcherDelegate

[wtc, 2012/11/21 00:48:01]

We should either leave these comments unchanged, or fix all
instances of these comments. Fixing these comments only in
this class creates an inconsistency with the other copies
of these comments above.

[ppi, 2012/11/22 09:57:18]

Thanks, I have fixed the comments throughout the entire file (patch set 3).

   virtual void OnURLFetchDownloadProgress(const URLFetcher* source,
                                           int64 current, int64 total) OVERRIDE;
+
  protected:
   int64 previous_progress_;
-  int64 expected_total_;
+  int64 previous_total_;
+  // Indicates that |previous_progress_| and |previous_total_| are not yet set.
+  bool first_call_;
 };
 
-/// 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.
   virtual void CreateFetcher(const GURL& url) OVERRIDE;
 
   // URLFetcherDelegate
   virtual void OnURLFetchComplete(const URLFetcher* source) OVERRIDE;
   virtual void OnURLFetchDownloadProgress(const URLFetcher* source,
                                           int64 current, int64 total) OVERRIDE;
  protected:
@@ skipping 226 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) {
-  // Increasing between 0 and total.
+  // Progress has to be between 0 and total size.
   EXPECT_LE(0, current);
   EXPECT_GE(total, current);
-  EXPECT_LE(previous_progress_, current);
+
+  // Progress has to be non-decreasing, total size has to be constant.
+  if (!first_call_) {
+    EXPECT_LE(previous_progress_, current);
+    EXPECT_EQ(previous_total_, total);
+  }
+
   previous_progress_ = current;
-  EXPECT_EQ(expected_total_, total);
+  previous_total_ = total;
+  first_call_ = false;
 }
 
 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_);

[wtc, 2012/11/21 00:48:01]

Removing this file_util::GetFileSize call weakens the test
because we no longer know the expected total size.

Is this file_util::GetFileSize call problematic for a remote
test server? I guess the file to fetch is not on the device?

I wonder if the fix should be to hardcode the size of the
"animate1.gif" file here.

[ppi, 2012/11/22 09:57:18]

You are right, thanks. I didn't feel comfortable about hardcoding the file size
in the test, so I made it being verified only on non-Android platforms (patch
set 4).

   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()->RunAllPending();
   ASSERT_FALSE(file_util::PathExists(file_path_))
       << file_path_.value() << " not removed.";
 }
 
 }  // namespace
 
 }  // namespace net