Add MultiRangeHttpFetcherOverFileFetcherTest to HttpFetcherTest.
am: 642b32b723

Change-Id: I6d40a0b30326544e51fdd43ddcb9d43f108f06e6
diff --git a/common/http_fetcher_unittest.cc b/common/http_fetcher_unittest.cc
index 66767fb..00ea128 100644
--- a/common/http_fetcher_unittest.cc
+++ b/common/http_fetcher_unittest.cc
@@ -19,6 +19,7 @@
 #include <sys/socket.h>
 #include <unistd.h>
 
+#include <algorithm>
 #include <memory>
 #include <string>
 #include <utility>
@@ -218,6 +219,7 @@
   virtual bool IsMock() const = 0;
   virtual bool IsMulti() const = 0;
   virtual bool IsHttpSupported() const = 0;
+  virtual bool IsFileFetcher() const = 0;
 
   virtual void IgnoreServerAborting(HttpServer* server) const {}
 
@@ -251,6 +253,7 @@
   bool IsMock() const override { return true; }
   bool IsMulti() const override { return false; }
   bool IsHttpSupported() const override { return true; }
+  bool IsFileFetcher() const override { return false; }
 
   HttpServer* CreateServer() override {
     return new NullHttpServer;
@@ -292,6 +295,7 @@
   bool IsMock() const override { return false; }
   bool IsMulti() const override { return false; }
   bool IsHttpSupported() const override { return true; }
+  bool IsFileFetcher() const override { return false; }
 
   void IgnoreServerAborting(HttpServer* server) const override {
     // Nothing to do.
@@ -342,6 +346,17 @@
   }
 
   string BigUrl(in_port_t port) const override {
+    static string big_contents = []() {
+      string buf;
+      buf.reserve(kBigLength);
+      constexpr const char* kBigUrlContent = "abcdefghij";
+      for (size_t i = 0; i < kBigLength; i += strlen(kBigUrlContent)) {
+        buf.append(kBigUrlContent,
+                   std::min(kBigLength - i, strlen(kBigUrlContent)));
+      }
+      return buf;
+    }();
+    test_utils::WriteFileString(temp_file_.path(), big_contents);
     return "file://" + temp_file_.path();
   }
   string SmallUrl(in_port_t port) const override {
@@ -355,6 +370,7 @@
   bool IsMock() const override { return false; }
   bool IsMulti() const override { return false; }
   bool IsHttpSupported() const override { return false; }
+  bool IsFileFetcher() const override { return true; }
 
   void IgnoreServerAborting(HttpServer* server) const override {}
 
@@ -364,6 +380,31 @@
   test_utils::ScopedTempFile temp_file_{"ue_file_fetcher.XXXXXX"};
 };
 
+class MultiRangeHttpFetcherOverFileFetcherTest : public FileFetcherTest {
+ public:
+  // Necessary to unhide the definition in the base class.
+  using AnyHttpFetcherTest::NewLargeFetcher;
+  HttpFetcher* NewLargeFetcher(ProxyResolver* /* proxy_resolver */) override {
+    MultiRangeHttpFetcher* ret = new MultiRangeHttpFetcher(new FileFetcher());
+    ret->ClearRanges();
+    // FileFetcher doesn't support range with unspecified length.
+    ret->AddRange(0, 1);
+    // Speed up test execution.
+    ret->set_idle_seconds(1);
+    ret->set_retry_seconds(1);
+    fake_hardware_.SetIsOfficialBuild(false);
+    return ret;
+  }
+
+  // Necessary to unhide the definition in the base class.
+  using AnyHttpFetcherTest::NewSmallFetcher;
+  HttpFetcher* NewSmallFetcher(ProxyResolver* proxy_resolver) override {
+    return NewLargeFetcher(proxy_resolver);
+  }
+
+  bool IsMulti() const override { return true; }
+};
+
 //
 // Infrastructure for type tests of HTTP fetcher.
 // See: http://code.google.com/p/googletest/wiki/AdvancedGuide#Typed_Tests
@@ -401,7 +442,8 @@
 typedef ::testing::Types<LibcurlHttpFetcherTest,
                          MockHttpFetcherTest,
                          MultiRangeHttpFetcherTest,
-                         FileFetcherTest>
+                         FileFetcherTest,
+                         MultiRangeHttpFetcherOverFileFetcherTest>
     HttpFetcherTestTypes;
 TYPED_TEST_CASE(HttpFetcherTest, HttpFetcherTestTypes);
 
@@ -1160,6 +1202,26 @@
 
   vector<pair<off_t, off_t>> ranges;
   ranges.push_back(make_pair(0, 25));
+  ranges.push_back(make_pair(99, 17));
+  MultiTest(this->test_.NewLargeFetcher(),
+            this->test_.fake_hardware(),
+            this->test_.BigUrl(server->GetPort()),
+            ranges,
+            "abcdefghijabcdefghijabcdejabcdefghijabcdef",
+            25 + 17,
+            this->test_.IsFileFetcher() ? kHttpResponseOk
+                                        : kHttpResponsePartialContent);
+}
+
+TYPED_TEST(HttpFetcherTest, MultiHttpFetcherUnspecifiedEndTest) {
+  if (!this->test_.IsMulti() || this->test_.IsFileFetcher())
+    return;
+
+  unique_ptr<HttpServer> server(this->test_.CreateServer());
+  ASSERT_TRUE(server->started_);
+
+  vector<pair<off_t, off_t>> ranges;
+  ranges.push_back(make_pair(0, 25));
   ranges.push_back(make_pair(99, 0));
   MultiTest(this->test_.NewLargeFetcher(),
             this->test_.fake_hardware(),
@@ -1185,11 +1247,12 @@
             ranges,
             "abcdefghijabcdefghijabcd",
             24,
-            kHttpResponsePartialContent);
+            this->test_.IsFileFetcher() ? kHttpResponseOk
+                                        : kHttpResponsePartialContent);
 }
 
 TYPED_TEST(HttpFetcherTest, MultiHttpFetcherMultiEndTest) {
-  if (!this->test_.IsMulti())
+  if (!this->test_.IsMulti() || this->test_.IsFileFetcher())
     return;
 
   unique_ptr<HttpServer> server(this->test_.CreateServer());
@@ -1235,7 +1298,7 @@
 // (1) successful recovery: The offset fetch will fail twice but succeed with
 // the third proxy.
 TYPED_TEST(HttpFetcherTest, MultiHttpFetcherErrorIfOffsetRecoverableTest) {
-  if (!this->test_.IsMulti())
+  if (!this->test_.IsMulti() || this->test_.IsFileFetcher())
     return;
 
   unique_ptr<HttpServer> server(this->test_.CreateServer());
@@ -1258,7 +1321,7 @@
 // (2) unsuccessful recovery: The offset fetch will fail repeatedly.  The
 // fetcher will signal a (failed) completed transfer to the delegate.
 TYPED_TEST(HttpFetcherTest, MultiHttpFetcherErrorIfOffsetUnrecoverableTest) {
-  if (!this->test_.IsMulti())
+  if (!this->test_.IsMulti() || this->test_.IsFileFetcher())
     return;
 
   unique_ptr<HttpServer> server(this->test_.CreateServer());
@@ -1290,15 +1353,17 @@
                      size_t length) override {
     LOG(INFO) << "ReceivedBytes, " << length << " bytes.";
     EXPECT_EQ(fetcher, fetcher_.get());
+    bool should_terminate = false;
     if (bytes_downloaded_ < terminate_trigger_bytes_ &&
         bytes_downloaded_ + length >= terminate_trigger_bytes_) {
       MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(&HttpFetcher::TerminateTransfer,
                      base::Unretained(fetcher_.get())));
+      should_terminate = true;
     }
     bytes_downloaded_ += length;
-    return true;
+    return !should_terminate;
   }
 
   void TransferComplete(HttpFetcher* fetcher, bool successful) override {
diff --git a/common/multi_range_http_fetcher.cc b/common/multi_range_http_fetcher.cc
index d39b7f9..230106d 100644
--- a/common/multi_range_http_fetcher.cc
+++ b/common/multi_range_http_fetcher.cc
@@ -99,10 +99,13 @@
                          range.length() - bytes_received_this_range_);
   }
   LOG_IF(WARNING, next_size <= 0) << "Asked to write length <= 0";
+  // bytes_received_this_range_ needs to be updated regardless of the delegate_
+  // result, because it will be used to determine a successful transfer in
+  // TransferEnded().
+  bytes_received_this_range_ += length;
   if (delegate_ && !delegate_->ReceivedBytes(this, bytes, next_size))
     return false;
 
-  bytes_received_this_range_ += length;
   if (range.HasLength() && bytes_received_this_range_ >= range.length()) {
     // Terminates the current fetcher. Waits for its TransferTerminated
     // callback before starting the next range so that we don't end up