Only LOAD_VALIDATE_CACHE for validated resources.

When fetching an URL with LOAD_VALIDATE_CACHE, if the URL is in the
cache but does not have validation headers (e.g. ETag, Last-Modified),
it will be refetched in its entirety. We don't want that.

Instead, for resource fetches, we fetch the URL first with
LOAD_ONLY_FROM_CACHE, and then confirm that either it is missing or it
has strong validators before re-requesting the URL with
LOAD_VALIDATE_CACHE.

For config and manifest fetches, we fetch the URL normally (without the
LOAD_VALIDATE_CACHE flag), as we don't benefit from revalidation.

In order to effect the above, this change introduces a small state
machine, controlled by the FetchStage enum.

In order to test the above, this change introduces a custom URLFetcherFactory.
This is because FakeURLFetcherFactory lacks a way to fake more than one
response for the same URL.

For test, each of the requests need to be customized in a different way.
Because the URLFetcherFactory API doesn't create an URLFetcher until the URL is
requested, this requires passing around a lambda specifying how to modify it at
create time.

BUG=309216

Review URL: https://codereview.chromium.org/1289683004

Cr-Commit-Position: refs/heads/master@{#344978}

4 files changed, 351 insertions, 101 deletions

diff --git a/components/precache/core/DEPS b/components/precache/core/DEPS
index dfc6c2b..9eafbf5 100644
--- a/components/precache/core/DEPS
+++ b/components/precache/core/DEPS
@@ -1,5 +1,6 @@
 include_rules = [
   "+net/base",
+  "+net/http",
   "+net/url_request",
   "+sql",
 ]
diff --git a/components/precache/core/precache_fetcher.cc b/components/precache/core/precache_fetcher.cc
index 5fd1a13..97f1eef 100644
--- a/components/precache/core/precache_fetcher.cc
+++ b/components/precache/core/precache_fetcher.cc
@@ -20,8 +20,7 @@
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_fetcher_delegate.h"
+#include "net/http/http_response_headers.h"
 #include "net/url_request/url_fetcher_response_writer.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
@@ -30,6 +29,13 @@ using net::URLFetcher;
 
 namespace precache {
 
+// LOAD_DO_NOT_*_COOKIES is for privacy reasons. If a user clears their
+// cookies, but a tracking beacon is prefetched and the beacon specifies its
+// source URL in a URL param, the beacon site would be able to rebuild a
+// profile of the user.
+const int kNoCookies =
+    net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DO_NOT_SEND_COOKIES;
+
 namespace {
 
 // The maximum for the Precache.Fetch.ResponseBytes.* histograms. We set this to
@@ -127,58 +133,80 @@ class URLFetcherNullWriter : public net::URLFetcherResponseWriter {
 
 }  // namespace
 
-// Class that fetches a URL, and runs the specified callback when the fetch is
-// complete. This class exists so that a different method can be run in
-// response to different kinds of fetches, e.g. OnConfigFetchComplete when
-// configuration settings are fetched, OnManifestFetchComplete when a manifest
-// is fetched, etc.
-class PrecacheFetcher::Fetcher : public net::URLFetcherDelegate {
- public:
-  // Construct a new Fetcher. This will create and start a new URLFetcher for
-  // the specified URL using the specified request context.
-  Fetcher(net::URLRequestContextGetter* request_context,
-          const GURL& url,
-          const base::Callback<void(const URLFetcher&)>& callback,
-          bool ignore_response_body);
-  ~Fetcher() override {}
-  void OnURLFetchComplete(const URLFetcher* source) override;
-  int64 response_bytes() { return response_bytes_; }
-  int64 network_response_bytes() { return network_response_bytes_; }
-
- private:
-  const base::Callback<void(const URLFetcher&)> callback_;
-  scoped_ptr<URLFetcher> url_fetcher_;
-  int64 response_bytes_;
-  int64 network_response_bytes_;
-
-  DISALLOW_COPY_AND_ASSIGN(Fetcher);
-};
-
 PrecacheFetcher::Fetcher::Fetcher(
     net::URLRequestContextGetter* request_context,
     const GURL& url,
     const base::Callback<void(const URLFetcher&)>& callback,
-    bool ignore_response_body)
-    : callback_(callback), response_bytes_(0), network_response_bytes_(0) {
-  url_fetcher_ = URLFetcher::Create(url, URLFetcher::GET, this);
-  url_fetcher_->SetRequestContext(request_context);
-  // LOAD_VALIDATE_CACHE allows us to refresh the cache by updating Date
-  // headers, without costing network bytes, as cache hits will return 304s.
-  // LOAD_DO_NOT_*_COOKIES is for privacy reasons. If a user clears their
-  // cookies, but a tracking beacon is prefetched and the beacon specifies its
-  // source URL in a URL param, the beacon site would be able to rebuild a
-  // profile of the user.
-  url_fetcher_->SetLoadFlags(net::LOAD_VALIDATE_CACHE |
-                             net::LOAD_DO_NOT_SAVE_COOKIES |
-                             net::LOAD_DO_NOT_SEND_COOKIES);
-  if (ignore_response_body) {
+    bool is_resource_request)
+    : request_context_(request_context),
+      url_(url),
+      callback_(callback),
+      is_resource_request_(is_resource_request),
+      response_bytes_(0),
+      network_response_bytes_(0) {
+  if (is_resource_request_)
+    LoadFromCache();
+  else
+    LoadFromNetwork();
+}
+
+PrecacheFetcher::Fetcher::~Fetcher() {}
+
+void PrecacheFetcher::Fetcher::LoadFromCache() {
+  fetch_stage_ = FetchStage::CACHE;
+  url_fetcher_cache_ = URLFetcher::Create(url_, URLFetcher::GET, this);
+  url_fetcher_cache_->SetRequestContext(request_context_);
+  url_fetcher_cache_->SetLoadFlags(net::LOAD_ONLY_FROM_CACHE | kNoCookies);
+  scoped_ptr<URLFetcherNullWriter> null_writer(new URLFetcherNullWriter);
+  url_fetcher_cache_->SaveResponseWithWriter(null_writer.Pass());
+  url_fetcher_cache_->Start();
+}
+
+void PrecacheFetcher::Fetcher::LoadFromNetwork() {
+  fetch_stage_ = FetchStage::NETWORK;
+  url_fetcher_network_ = URLFetcher::Create(url_, URLFetcher::GET, this);
+  url_fetcher_network_->SetRequestContext(request_context_);
+  if (is_resource_request_) {
+    // LOAD_VALIDATE_CACHE allows us to refresh Date headers for resources
+    // already in the cache. The Date headers are updated from 304s as well as
+    // 200s.
+    url_fetcher_network_->SetLoadFlags(net::LOAD_VALIDATE_CACHE | kNoCookies);
+    // We don't need a copy of the response body for resource requests. The
+    // request is issued only to populate the browser cache.
     scoped_ptr<URLFetcherNullWriter> null_writer(new URLFetcherNullWriter);
-    url_fetcher_->SaveResponseWithWriter(null_writer.Pass());
+    url_fetcher_network_->SaveResponseWithWriter(null_writer.Pass());
+  } else {
+    // Config and manifest requests do not need to be revalidated. It's okay if
+    // they expire from the cache minutes after we request them.
+    url_fetcher_network_->SetLoadFlags(kNoCookies);
   }
-  url_fetcher_->Start();
+  url_fetcher_network_->Start();
 }
 
 void PrecacheFetcher::Fetcher::OnURLFetchComplete(const URLFetcher* source) {
+  if (fetch_stage_ == FetchStage::CACHE &&
+      (source->GetStatus().error() == net::ERR_CACHE_MISS ||
+       (source->GetResponseHeaders() &&
+        source->GetResponseHeaders()->HasStrongValidators()))) {
+    // If the resource was not found in the cache, request it from the
+    // network.
+    //
+    // If the resource was found in the cache, but contains validators,
+    // request a refresh. The presence of validators increases the chance that
+    // we get a 304 response rather than a full one, thus allowing us to
+    // refresh the cache with minimal network load.
+    //
+    // TODO(twifkak): Add support for weak validators, which should be just as
+    // likely a guarantee that the response will be a 304.
+    LoadFromNetwork();
+    return;
+  }
+
+  // If any of:
+  // - The request was for a config or manifest.
+  // - The resource was a cache hit without validators.
+  // - The response came from the network.
+  // Then Fetcher is done with this URL and can return control to the caller.
   response_bytes_ = source->GetReceivedResponseContentLength();
   network_response_bytes_ = source->GetTotalReceivedBytes();
   callback_.Run(*source);
@@ -246,7 +274,7 @@ void PrecacheFetcher::Start() {
   fetcher_.reset(new Fetcher(request_context_.get(), config_url,
                              base::Bind(&PrecacheFetcher::OnConfigFetchComplete,
                                         base::Unretained(this)),
-                             false /* ignore_response_body */));
+                             false /* is_resource_request */));
 }
 
 void PrecacheFetcher::StartNextFetch() {
@@ -259,7 +287,7 @@ void PrecacheFetcher::StartNextFetch() {
         new Fetcher(request_context_.get(), resource_urls_to_fetch_.front(),
                     base::Bind(&PrecacheFetcher::OnResourceFetchComplete,
                                base::Unretained(this)),
-                    true /* ignore_response_body */));
+                    true /* is_resource_request */));
 
     resource_urls_to_fetch_.pop_front();
     return;
@@ -271,7 +299,7 @@ void PrecacheFetcher::StartNextFetch() {
         new Fetcher(request_context_.get(), manifest_urls_to_fetch_.front(),
                     base::Bind(&PrecacheFetcher::OnManifestFetchComplete,
                                base::Unretained(this)),
-                    false /* ignore_response_body */));
+                    false /* is_resource_request */));
 
     manifest_urls_to_fetch_.pop_front();
     return;
diff --git a/components/precache/core/precache_fetcher.h b/components/precache/core/precache_fetcher.h
index 646efb3..d572801 100644
--- a/components/precache/core/precache_fetcher.h
+++ b/components/precache/core/precache_fetcher.h
@@ -10,18 +10,24 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time/time.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_fetcher_delegate.h"
 #include "url/gurl.h"
 
 namespace net {
-class URLFetcher;
 class URLRequestContextGetter;
 }
 
 namespace precache {
 
+// Visible for testing.
+extern const int kNoCookies;
+
 // Public interface to code that fetches resources that the user is likely to
 // want to fetch in the future, putting them in the network stack disk cache.
 // Precaching is intended to be done when Chrome is not actively in use, likely
@@ -69,6 +75,9 @@ class PrecacheFetcher {
     virtual void OnDone() = 0;
   };
 
+  // Visible for testing.
+  class Fetcher;
+
   // Constructs a new PrecacheFetcher. The |starting_hosts| parameter is a
   // prioritized list of hosts that the user commonly visits. These hosts are
   // used by a server side component to construct a list of resource URLs that
@@ -87,8 +96,6 @@ class PrecacheFetcher {
   void Start();
 
  private:
-  class Fetcher;
-
   // Fetches the next resource or manifest URL, if any remain. Fetching is done
   // sequentially and depth-first: all resources are fetched for a manifest
   // before the next manifest is fetched. This is done to limit the length of
@@ -149,6 +156,45 @@ class PrecacheFetcher {
   DISALLOW_COPY_AND_ASSIGN(PrecacheFetcher);
 };
 
+// Class that fetches a URL, and runs the specified callback when the fetch is
+// complete. This class exists so that a different method can be run in
+// response to different kinds of fetches, e.g. OnConfigFetchComplete when
+// configuration settings are fetched, OnManifestFetchComplete when a manifest
+// is fetched, etc.
+class PrecacheFetcher::Fetcher : public net::URLFetcherDelegate {
+ public:
+  // Construct a new Fetcher. This will create and start a new URLFetcher for
+  // the specified URL using the specified request context.
+  Fetcher(net::URLRequestContextGetter* request_context,
+          const GURL& url,
+          const base::Callback<void(const net::URLFetcher&)>& callback,
+          bool is_resource_request);
+  ~Fetcher() override;
+  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  int64 response_bytes() const { return response_bytes_; }
+  int64 network_response_bytes() const { return network_response_bytes_; }
+
+ private:
+  enum class FetchStage { CACHE, NETWORK };
+
+  void LoadFromCache();
+  void LoadFromNetwork();
+
+  net::URLRequestContextGetter* const request_context_;
+  const GURL url_;
+  const base::Callback<void(const net::URLFetcher&)> callback_;
+  const bool is_resource_request_;
+
+  FetchStage fetch_stage_;
+  // The url_fetcher_cache_ is kept alive until Fetcher destruction for testing.
+  scoped_ptr<net::URLFetcher> url_fetcher_cache_;
+  scoped_ptr<net::URLFetcher> url_fetcher_network_;
+  int64 response_bytes_;
+  int64 network_response_bytes_;
+
+  DISALLOW_COPY_AND_ASSIGN(Fetcher);
+};
+
 }  // namespace precache
 
 #endif  // COMPONENTS_PRECACHE_CORE_PRECACHE_FETCHER_H_
diff --git a/components/precache/core/precache_fetcher_unittest.cc b/components/precache/core/precache_fetcher_unittest.cc
index 848194d..cac75bf 100644
--- a/components/precache/core/precache_fetcher_unittest.cc
+++ b/components/precache/core/precache_fetcher_unittest.cc
@@ -13,15 +13,20 @@
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
 #include "base/thread_task_runner_handle.h"
 #include "components/precache/core/precache_switches.h"
 #include "components/precache/core/proto/precache.pb.h"
+#include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
@@ -29,6 +34,26 @@ namespace precache {
 
 namespace {
 
+using ::testing::_;
+
+const char kConfigURL[] = "http://config-url.com";
+const char kManifestURLPrefix[] = "http://manifest-url-prefix.com/";
+const char kCustomConfigURL[] = "http://custom-config-url.com";
+const char kCustomManifestURLPrefix[] =
+    "http://custom-manifest-url-prefix.com/";
+const char kManifestFetchFailureURL[] =
+    "http://manifest-url-prefix.com/manifest-fetch-failure.com";
+const char kBadManifestURL[] =
+    "http://manifest-url-prefix.com/bad-manifest.com";
+const char kGoodManifestURL[] =
+    "http://manifest-url-prefix.com/good-manifest.com";
+const char kCustomGoodManifestURL[] =
+    "http://custom-manifest-url-prefix.com/good-manifest.com";
+const char kResourceFetchFailureURL[] = "http://resource-fetch-failure.com";
+const char kGoodResourceURL[] = "http://good-resource.com";
+const char kForcedStartingURLManifestURL[] =
+    "http://manifest-url-prefix.com/forced-starting-url.com";
+
 class TestURLFetcherCallback {
  public:
   TestURLFetcherCallback() : total_response_bytes_(0) {}
@@ -40,15 +65,9 @@ class TestURLFetcherCallback {
     scoped_ptr<net::FakeURLFetcher> fetcher(new net::FakeURLFetcher(
         url, delegate, response_data, response_code, status));
 
-    if (response_code == net::HTTP_OK) {
-      scoped_refptr<net::HttpResponseHeaders> download_headers =
-          new net::HttpResponseHeaders("");
-      download_headers->AddHeader("Content-Type: text/html");
-      fetcher->set_response_headers(download_headers);
-    }
-
     total_response_bytes_ += response_data.size();
     requested_urls_.insert(url);
+
     return fetcher.Pass();
   }
 
@@ -78,6 +97,186 @@ class TestPrecacheDelegate : public PrecacheFetcher::PrecacheDelegate {
   bool was_on_done_called_;
 };
 
+class MockURLFetcherFactory : public net::URLFetcherFactory {
+ public:
+  typedef net::URLFetcher* DoURLFetcher(
+      int id,
+      const GURL& url,
+      net::URLFetcher::RequestType request_type,
+      net::URLFetcherDelegate* delegate);
+
+  scoped_ptr<net::URLFetcher> CreateURLFetcher(
+      int id,
+      const GURL& url,
+      net::URLFetcher::RequestType request_type,
+      net::URLFetcherDelegate* delegate) override {
+    return make_scoped_ptr(DoCreateURLFetcher(id, url, request_type, delegate));
+  }
+
+  // The method to mock out, instead of CreateURLFetcher. This is necessary
+  // because gmock can't handle move-only types such as scoped_ptr.
+  MOCK_METHOD4(DoCreateURLFetcher, DoURLFetcher);
+
+  // A fake successful response. When the action runs, it saves off a pointer to
+  // the FakeURLFetcher in its output parameter for later inspection.
+  testing::Action<DoURLFetcher> RespondWith(const std::string& body,
+                                            net::FakeURLFetcher** fetcher) {
+    return RespondWith(body, [](net::FakeURLFetcher* fetcher) {
+      fetcher->set_response_code(net::HTTP_OK);
+    }, fetcher);
+  }
+
+  // A fake custom response. When the action runs, it runs the given modifier to
+  // customize the FakeURLFetcher, and then saves off a pointer to the
+  // FakeURLFetcher in its output parameter for later inspection. The modifier
+  // should be a functor that takes a FakeURLFetcher* and returns void.
+  template <typename F>
+  testing::Action<DoURLFetcher> RespondWith(const std::string& body,
+                                            F modifier,
+                                            net::FakeURLFetcher** fetcher) {
+    return testing::MakeAction(
+        new FakeResponseAction<F>(body, modifier, fetcher));
+  }
+
+ private:
+  template <typename F>
+  class FakeResponseAction : public testing::ActionInterface<DoURLFetcher> {
+   public:
+    FakeResponseAction(const std::string& body,
+                       F modifier,
+                       net::FakeURLFetcher** fetcher)
+        : body_(body), modifier_(modifier), fetcher_(fetcher) {}
+
+    net::URLFetcher* Perform(
+        const testing::tuple<int,
+                             const GURL&,
+                             net::URLFetcher::RequestType,
+                             net::URLFetcherDelegate*>& args) {
+      *fetcher_ = new net::FakeURLFetcher(
+          testing::get<1>(args), testing::get<3>(args), body_, net::HTTP_OK,
+          net::URLRequestStatus::SUCCESS);
+      modifier_(*fetcher_);
+      return *fetcher_;
+    }
+
+   private:
+    std::string body_;
+    F modifier_;
+    net::FakeURLFetcher** fetcher_;
+  };
+};
+
+class PrecacheFetcherFetcherTest : public testing::Test {
+ public:
+  PrecacheFetcherFetcherTest()
+      : request_context_(new net::TestURLRequestContextGetter(
+            base::ThreadTaskRunnerHandle::Get())),
+        scoped_url_fetcher_factory_(&factory_),
+        callback_(base::Bind(&PrecacheFetcherFetcherTest::Callback,
+                             base::Unretained(this))),
+        callback_called_(false) {}
+
+  void Callback(const net::URLFetcher&) { callback_called_ = true; }
+
+ protected:
+  base::MessageLoopForUI loop_;
+  scoped_refptr<net::TestURLRequestContextGetter> request_context_;
+  MockURLFetcherFactory factory_;
+  net::ScopedURLFetcherFactory scoped_url_fetcher_factory_;
+  base::Callback<void(const net::URLFetcher&)> callback_;
+  bool callback_called_;
+};
+
+TEST_F(PrecacheFetcherFetcherTest, Config) {
+  GURL url(kConfigURL);
+
+  net::FakeURLFetcher* fetcher = nullptr;
+  EXPECT_CALL(factory_, DoCreateURLFetcher(_, url, net::URLFetcher::GET, _))
+      .WillOnce(factory_.RespondWith("", &fetcher));
+
+  PrecacheFetcher::Fetcher precache_fetcher(
+      request_context_.get(), url, callback_, false /* is_resource_request */);
+
+  loop_.RunUntilIdle();
+
+  ASSERT_NE(nullptr, fetcher);
+  EXPECT_EQ(kNoCookies, fetcher->GetLoadFlags());
+
+  EXPECT_EQ(true, callback_called_);
+}
+
+TEST_F(PrecacheFetcherFetcherTest, ResourceNotInCache) {
+  GURL url(kGoodResourceURL);
+
+  net::FakeURLFetcher *fetcher1 = nullptr, *fetcher2 = nullptr;
+  EXPECT_CALL(factory_, DoCreateURLFetcher(_, url, net::URLFetcher::GET, _))
+      .WillOnce(factory_.RespondWith(
+          "",
+          [](net::FakeURLFetcher* fetcher) {
+            fetcher->set_status(net::URLRequestStatus(
+                net::URLRequestStatus::FAILED, net::ERR_CACHE_MISS));
+          },
+          &fetcher1))
+      .WillOnce(factory_.RespondWith("", &fetcher2));
+
+  PrecacheFetcher::Fetcher precache_fetcher(
+      request_context_.get(), url, callback_, true /* is_resource_request */);
+
+  loop_.RunUntilIdle();
+
+  ASSERT_NE(nullptr, fetcher1);
+  EXPECT_EQ(net::LOAD_ONLY_FROM_CACHE | kNoCookies, fetcher1->GetLoadFlags());
+  ASSERT_NE(nullptr, fetcher2);
+  EXPECT_EQ(net::LOAD_VALIDATE_CACHE | kNoCookies, fetcher2->GetLoadFlags());
+
+  EXPECT_EQ(true, callback_called_);
+}
+
+TEST_F(PrecacheFetcherFetcherTest, ResourceHasStrongValidators) {
+  GURL url(kGoodResourceURL);
+
+  net::FakeURLFetcher *fetcher1 = nullptr, *fetcher2 = nullptr;
+  EXPECT_CALL(factory_, DoCreateURLFetcher(_, url, net::URLFetcher::GET, _))
+      .WillOnce(factory_.RespondWith(
+          "",
+          [](net::FakeURLFetcher* fetcher) {
+            std::string raw_headers("HTTP/1.1 200 OK\0ETag: foo\0\0", 27);
+            fetcher->set_response_headers(
+                make_scoped_refptr(new net::HttpResponseHeaders(raw_headers)));
+          },
+          &fetcher1))
+      .WillOnce(factory_.RespondWith("", &fetcher2));
+
+  PrecacheFetcher::Fetcher precache_fetcher(
+      request_context_.get(), url, callback_, true /* is_resource_request */);
+
+  loop_.RunUntilIdle();
+
+  ASSERT_NE(nullptr, fetcher1);
+  EXPECT_EQ(net::LOAD_ONLY_FROM_CACHE | kNoCookies, fetcher1->GetLoadFlags());
+  ASSERT_NE(nullptr, fetcher2);
+  EXPECT_EQ(net::LOAD_VALIDATE_CACHE | kNoCookies, fetcher2->GetLoadFlags());
+
+  EXPECT_EQ(true, callback_called_);
+}
+
+TEST_F(PrecacheFetcherFetcherTest, ResourceHasNoValidators) {
+  GURL url(kGoodResourceURL);
+
+  net::FakeURLFetcher* fetcher;
+  EXPECT_CALL(factory_, DoCreateURLFetcher(_, url, net::URLFetcher::GET, _))
+      .WillOnce(factory_.RespondWith("", &fetcher));
+
+  PrecacheFetcher::Fetcher precache_fetcher(
+      request_context_.get(), url, callback_, true /* is_resource_request */);
+
+  loop_.RunUntilIdle();
+
+  EXPECT_EQ(net::LOAD_ONLY_FROM_CACHE | kNoCookies, fetcher->GetLoadFlags());
+
+  EXPECT_EQ(true, callback_called_);
+}
+
 class PrecacheFetcherTest : public testing::Test {
  public:
   PrecacheFetcherTest()
@@ -89,6 +288,13 @@ class PrecacheFetcherTest : public testing::Test {
         expected_total_response_bytes_(0) {}
 
  protected:
+  void SetDefaultFlags() {
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kPrecacheConfigSettingsURL, kConfigURL);
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kPrecacheManifestURLPrefix, kManifestURLPrefix);
+  }
+
   base::MessageLoopForUI loop_;
   scoped_refptr<net::TestURLRequestContextGetter> request_context_;
   TestURLFetcherCallback url_callback_;
@@ -97,29 +303,8 @@ class PrecacheFetcherTest : public testing::Test {
   int expected_total_response_bytes_;
 };
 
-const char kConfigURL[] = "http://config-url.com";
-const char kManifestURLPrefix[] = "http://manifest-url-prefix.com/";
-const char kCustomConfigURL[] = "http://custom-config-url.com";
-const char kCustomManifestURLPrefix[] =
-    "http://custom-manifest-url-prefix.com/";
-const char kManifestFetchFailureURL[] =
-    "http://manifest-url-prefix.com/manifest-fetch-failure.com";
-const char kBadManifestURL[] =
-    "http://manifest-url-prefix.com/bad-manifest.com";
-const char kGoodManifestURL[] =
-    "http://manifest-url-prefix.com/good-manifest.com";
-const char kCustomGoodManifestURL[] =
-    "http://custom-manifest-url-prefix.com/good-manifest.com";
-const char kResourceFetchFailureURL[] = "http://resource-fetch-failure.com";
-const char kGoodResourceURL[] = "http://good-resource.com";
-const char kForcedStartingURLManifestURL[] =
-    "http://manifest-url-prefix.com/forced-starting-url.com";
-
 TEST_F(PrecacheFetcherTest, FullPrecache) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheConfigSettingsURL, kConfigURL);
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheManifestURLPrefix, kManifestURLPrefix);
+  SetDefaultFlags();
 
   std::vector<std::string> starting_hosts;
   starting_hosts.push_back("manifest-fetch-failure.com");
@@ -165,7 +350,7 @@ TEST_F(PrecacheFetcherTest, FullPrecache) {
                                      &precache_delegate_);
     precache_fetcher.Start();
 
-    base::MessageLoop::current()->RunUntilIdle();
+    loop_.RunUntilIdle();
 
     // Destroy the PrecacheFetcher after it has finished, to record metrics.
   }
@@ -190,10 +375,7 @@ TEST_F(PrecacheFetcherTest, FullPrecache) {
 }
 
 TEST_F(PrecacheFetcherTest, CustomURLs) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheConfigSettingsURL, kConfigURL);
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheManifestURLPrefix, kManifestURLPrefix);
+  SetDefaultFlags();
 
   std::vector<std::string> starting_hosts;
   starting_hosts.push_back("good-manifest.com");
@@ -216,7 +398,7 @@ TEST_F(PrecacheFetcherTest, CustomURLs) {
       kCustomManifestURLPrefix, &precache_delegate_);
   precache_fetcher.Start();
 
-  base::MessageLoop::current()->RunUntilIdle();
+  loop_.RunUntilIdle();
 
   std::multiset<GURL> expected_requested_urls;
   expected_requested_urls.insert(GURL(kCustomConfigURL));
@@ -229,10 +411,7 @@ TEST_F(PrecacheFetcherTest, CustomURLs) {
 }
 
 TEST_F(PrecacheFetcherTest, ConfigFetchFailure) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheConfigSettingsURL, kConfigURL);
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheManifestURLPrefix, kManifestURLPrefix);
+  SetDefaultFlags();
 
   std::vector<std::string> starting_hosts(1, "good-manifest.com");
 
@@ -246,7 +425,7 @@ TEST_F(PrecacheFetcherTest, ConfigFetchFailure) {
                                    GURL(), std::string(), &precache_delegate_);
   precache_fetcher.Start();
 
-  base::MessageLoop::current()->RunUntilIdle();
+  loop_.RunUntilIdle();
 
   std::multiset<GURL> expected_requested_urls;
   expected_requested_urls.insert(GURL(kConfigURL));
@@ -257,10 +436,7 @@ TEST_F(PrecacheFetcherTest, ConfigFetchFailure) {
 }
 
 TEST_F(PrecacheFetcherTest, BadConfig) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheConfigSettingsURL, kConfigURL);
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheManifestURLPrefix, kManifestURLPrefix);
+  SetDefaultFlags();
 
   std::vector<std::string> starting_hosts(1, "good-manifest.com");
 
@@ -273,7 +449,7 @@ TEST_F(PrecacheFetcherTest, BadConfig) {
                                    GURL(), std::string(), &precache_delegate_);
   precache_fetcher.Start();
 
-  base::MessageLoop::current()->RunUntilIdle();
+  loop_.RunUntilIdle();
 
   std::multiset<GURL> expected_requested_urls;
   expected_requested_urls.insert(GURL(kConfigURL));
@@ -284,8 +460,7 @@ TEST_F(PrecacheFetcherTest, BadConfig) {
 }
 
 TEST_F(PrecacheFetcherTest, Cancel) {
-  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kPrecacheConfigSettingsURL, kConfigURL);
+  SetDefaultFlags();
 
   std::vector<std::string> starting_hosts(1, "starting-url.com");
 
@@ -307,7 +482,7 @@ TEST_F(PrecacheFetcherTest, Cancel) {
     // This should not cause OnDone to be called on the precache delegate.
   }
 
-  base::MessageLoop::current()->RunUntilIdle();
+  loop_.RunUntilIdle();
 
   std::multiset<GURL> expected_requested_urls;
   expected_requested_urls.insert(GURL(kConfigURL));
@@ -338,7 +513,7 @@ TEST_F(PrecacheFetcherTest, PrecacheUsingDefaultConfigSettingsURL) {
                                    GURL(), std::string(), &precache_delegate_);
   precache_fetcher.Start();
 
-  base::MessageLoop::current()->RunUntilIdle();
+  loop_.RunUntilIdle();
 
   std::multiset<GURL> expected_requested_urls;
   expected_requested_urls.insert(GURL(PRECACHE_CONFIG_SETTINGS_URL));
@@ -373,7 +548,7 @@ TEST_F(PrecacheFetcherTest, PrecacheUsingDefaultManifestURLPrefix) {
                                    GURL(), std::string(), &precache_delegate_);
   precache_fetcher.Start();
 
-  base::MessageLoop::current()->RunUntilIdle();
+  loop_.RunUntilIdle();
 
   std::multiset<GURL> expected_requested_urls;
   expected_requested_urls.insert(GURL(kConfigURL));