Fix URLFetcherDownloadProgressTest to work with remote test server

net/url_request/url_fetcher_impl_unittest.cc

Index: net/url_request/url_fetcher_impl_unittest.cc
diff --git a/net/url_request/url_fetcher_impl_unittest.cc b/net/url_request/url_fetcher_impl_unittest.cc
index 6435baf3d7639289d9ad13e48ce61d2be5f77d5b..8ab9d69e3f4aac9f670a5c9a4106a018001cd162 100644
--- a/net/url_request/url_fetcher_impl_unittest.cc
+++ b/net/url_request/url_fetcher_impl_unittest.cc
@@ -186,18 +186,25 @@ class URLFetcherEmptyPostTest : public URLFetcherTest {
 // Version of URLFetcherTest that tests download progress reports.
 class URLFetcherDownloadProgressTest : public URLFetcherTest {
  public:
-  // URLFetcherTest override.
-  virtual void CreateFetcher(const GURL& url) OVERRIDE;
+  URLFetcherDownloadProgressTest()
+      : previous_progress_(0),
+        previous_total_(0),
+        first_call_(true) {
+  }
 
-  // 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).

+  // URLFetcherTest:
+  virtual void CreateFetcher(const GURL& url) OVERRIDE;
   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.
@@ -444,18 +451,24 @@ 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) {
@@ -855,8 +868,6 @@ TEST_F(URLFetcherDownloadProgressTest, Basic) {
   // 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));