Make SDCH dictionaries persistent across browser restart.

net/url_request/sdch_dictionary_fetcher_unittest.cc

Index: net/url_request/sdch_dictionary_fetcher_unittest.cc
diff --git a/net/url_request/sdch_dictionary_fetcher_unittest.cc b/net/url_request/sdch_dictionary_fetcher_unittest.cc
index 1826ddb1681509bc7ffb8e47a84fbd59baa254d1..f0a54f9b204f39fc2379151fae0ae465d0974551 100644
--- a/net/url_request/sdch_dictionary_fetcher_unittest.cc
+++ b/net/url_request/sdch_dictionary_fetcher_unittest.cc
@@ -5,13 +5,18 @@
 #include "net/url_request/sdch_dictionary_fetcher.h"
 
 #include <string>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/logging.h"
+#include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/thread_task_runner_handle.h"
+#include "net/base/load_flags.h"
 #include "net/base/sdch_manager.h"
+#include "net/http/http_response_headers.h"
 #include "net/url_request/url_request_data_job.h"
 #include "net/url_request/url_request_filter.h"
 #include "net/url_request/url_request_interceptor.h"
@@ -27,10 +32,17 @@ const char kTestDomain[] = "top.domain.test";
 
 class URLRequestSpecifiedResponseJob : public URLRequestSimpleJob {
  public:
-  URLRequestSpecifiedResponseJob(URLRequest* request,
-                                 NetworkDelegate* network_delegate,
-                                 const base::Closure& destruction_callback)
+  // Called on destruction with load flags used for this request.
+  typedef base::Callback<void(int)> DestructionCallback;
+
+  URLRequestSpecifiedResponseJob(
+      URLRequest* request,
+      NetworkDelegate* network_delegate,
+      const HttpResponseInfo& response_info_to_return,
+      const DestructionCallback& destruction_callback)
       : URLRequestSimpleJob(request, network_delegate),
+        response_info_to_return_(response_info_to_return),
+        last_load_flags_seen_(request->load_flags()),
         destruction_callback_(destruction_callback) {
     DCHECK(!destruction_callback.is_null());
   }
@@ -42,8 +54,15 @@ class URLRequestSpecifiedResponseJob : public URLRequestSimpleJob {
                               url.spec().c_str());
   }
 
+  // URLRequestJob
+  void GetResponseInfo(HttpResponseInfo* info) override {
+    *info = response_info_to_return_;
+  }
+
  private:
-  ~URLRequestSpecifiedResponseJob() override { destruction_callback_.Run(); }
+  ~URLRequestSpecifiedResponseJob() override {
+    destruction_callback_.Run(last_load_flags_seen_);
+  }
 
   // URLRequestSimpleJob implementation:
   int GetData(std::string* mime_type,
@@ -55,19 +74,32 @@ class URLRequestSpecifiedResponseJob : public URLRequestSimpleJob {
     return OK;
   }
 
-  const base::Closure destruction_callback_;
+  const HttpResponseInfo response_info_to_return_;
+  int last_load_flags_seen_;
+  const DestructionCallback destruction_callback_;
+  DISALLOW_COPY_AND_ASSIGN(URLRequestSpecifiedResponseJob);
 };
 
 class SpecifiedResponseJobInterceptor : public URLRequestInterceptor {
  public:
   // A callback to be called whenever a URLRequestSpecifiedResponseJob is
-  // created or destroyed.  The argument will be the change in number of
+  // created or destroyed.  The first argument will be the change in number of
   // jobs (i.e. +1 for created, -1 for destroyed).
-  typedef base::Callback<void(int outstanding_job_delta)> LifecycleCallback;
-
-  explicit SpecifiedResponseJobInterceptor(
-      const LifecycleCallback& lifecycle_callback)
-      : lifecycle_callback_(lifecycle_callback), factory_(this) {
+  // The second argument will be undefined if the job is being created,
+  // and will contain the load flags passed to the request the
+  // job was created for if the job is being destroyed.
+  typedef base::Callback<void(int outstanding_job_delta,
+                              int destruction_load_flags)> LifecycleCallback;
+
+  // |*info| will be returned from all child URLRequestSpecifiedResponseJobs.
+  // Note that: a) this pointer is shared with the caller, and the caller must
+  // guarantee that |*info| outlives the SpecifiedResponseJobInterceptor, and
+  // b) |*info| is mutable, and changes to should propagate to
+  // URLRequestSpecifiedResponseJobs created after any change.
+  SpecifiedResponseJobInterceptor(HttpResponseInfo* http_response_info,
+                                  const LifecycleCallback& lifecycle_callback)
+      : http_response_info_(http_response_info),
+        lifecycle_callback_(lifecycle_callback) {
     DCHECK(!lifecycle_callback_.is_null());
   }
   ~SpecifiedResponseJobInterceptor() override {}
@@ -75,19 +107,21 @@ class SpecifiedResponseJobInterceptor : public URLRequestInterceptor {
   URLRequestJob* MaybeInterceptRequest(
       URLRequest* request,
       NetworkDelegate* network_delegate) const override {
-    if (!lifecycle_callback_.is_null())
-      lifecycle_callback_.Run(1);
+    lifecycle_callback_.Run(1, 0);
 
     return new URLRequestSpecifiedResponseJob(
-        request, network_delegate, base::Bind(lifecycle_callback_, -1));
+        request, network_delegate, *http_response_info_,
+        base::Bind(lifecycle_callback_, -1));
   }
 
-  // The caller must ensure that the callback is valid to call for the
-  // lifetime of the SpecifiedResponseJobInterceptor (i.e. until
-  // Unregister() is called).
-  static void RegisterWithFilter(const LifecycleCallback& lifecycle_callback) {
+  // The caller must ensure that both |*http_response_info| and the
+  // callback remain valid for the lifetime of the
+  // SpecifiedResponseJobInterceptor (i.e. until Unregister() is called).
+  static void RegisterWithFilter(HttpResponseInfo* http_response_info,
+                                 const LifecycleCallback& lifecycle_callback) {
     scoped_ptr<SpecifiedResponseJobInterceptor> interceptor(
-        new SpecifiedResponseJobInterceptor(lifecycle_callback));
+        new SpecifiedResponseJobInterceptor(http_response_info,
+                                            lifecycle_callback));
 
     net::URLRequestFilter::GetInstance()->AddHostnameInterceptor(
         "http", kTestDomain, interceptor.Pass());
@@ -99,26 +133,33 @@ class SpecifiedResponseJobInterceptor : public URLRequestInterceptor {
   }
 
  private:
-  void OnSpecfiedResponseJobDestruction() const {
-    if (!lifecycle_callback_.is_null())
-      lifecycle_callback_.Run(-1);
-  }
-
+  HttpResponseInfo* http_response_info_;
   LifecycleCallback lifecycle_callback_;
-  mutable base::WeakPtrFactory<SpecifiedResponseJobInterceptor> factory_;
+  DISALLOW_COPY_AND_ASSIGN(SpecifiedResponseJobInterceptor);
 };
 
 // Local test infrastructure
 // * URLRequestSpecifiedResponseJob: A URLRequestJob that returns
 //   a different but derivable response for each URL (used for all
-//   url requests in this file).  The class provides interfaces to
-//   signal whenever the total number of jobs transitions to zero.
-// * SdchDictionaryFetcherTest: Registers a callback with the above
-//   class, and provides blocking interfaces for a transition to zero jobs.
-//   Contains an SdchDictionaryFetcher, and tracks fetcher dictionary
-//   addition callbacks.
-// Most tests schedule a dictionary fetch, wait for no jobs outstanding,
-// then test that the fetch results are as expected.
+//   url requests in this file).  This class is initialized with
+//   the HttpResponseInfo to return (if any), as well as a callback
+//   that is called when the class is destroyed.  That callback
+//   takes as arguemnt the load flags used for the request the
+//   job was created for.
+// * SpecifiedResponseJobInterceptor: This class is a
+//   URLRequestInterceptor that generates the class above.  It is constructed
+//   with a pointer to the (mutable) resposne info that should be
+//   returned from the URLRequestSpecifiedResponseJob children, as well as
+//   a callback that is run when URLRequestSpecifiedResponseJobs are
+//   created or destroyed.
+// * SdchDictionaryFetcherTest: This class registers the above interceptor,
+//   tracks the number of jobs requested and the subset of those
+//   that are still outstanding.  It exports an interface to wait until there
+//   are no jobs outstanding.  It shares an HttpResponseInfo structure
+//   with the SpecifiedResponseJobInterceptor to control the response
+//   information returned by the jbos.
+// The standard pattern for tests is to schedule a dictionary fetch, wait
+// for no jobs outstanding, then test that the fetch results are as expected.
 
 class SdchDictionaryFetcherTest : public ::testing::Test {
  public:
@@ -134,13 +175,14 @@ class SdchDictionaryFetcherTest : public ::testing::Test {
   SdchDictionaryFetcherTest()
       : jobs_requested_(0),
         jobs_outstanding_(0),
+        last_load_flags_seen_(LOAD_NORMAL),
         context_(new TestURLRequestContext),
-        fetcher_(new SdchDictionaryFetcher(
-            context_.get(),
-            base::Bind(&SdchDictionaryFetcherTest::OnDictionaryFetched,
-                       base::Unretained(this)))),
+        fetcher_(new SdchDictionaryFetcher(context_.get())),
         factory_(this) {
+    response_info_to_return_.request_time = base::Time::Now();
+    response_info_to_return_.response_time = base::Time::Now();
     SpecifiedResponseJobInterceptor::RegisterWithFilter(
+        &response_info_to_return_,
         base::Bind(&SdchDictionaryFetcherTest::OnNumberJobsChanged,
                    factory_.GetWeakPtr()));
   }
@@ -152,15 +194,15 @@ class SdchDictionaryFetcherTest : public ::testing::Test {
   void OnDictionaryFetched(const std::string& dictionary_text,
                            const GURL& dictionary_url,
                            const BoundNetLog& net_log) {
-    dictionary_additions.push_back(
+    dictionary_additions_.push_back(
         DictionaryAdditions(dictionary_text, dictionary_url));
   }
 
   // Return (in |*out|) all dictionary additions since the last time
   // this function was called.
   void GetDictionaryAdditions(std::vector<DictionaryAdditions>* out) {
-    out->swap(dictionary_additions);
-    dictionary_additions.clear();
+    out->swap(dictionary_additions_);
+    dictionary_additions_.clear();
   }
 
   SdchDictionaryFetcher* fetcher() { return fetcher_.get(); }
@@ -168,7 +210,7 @@ class SdchDictionaryFetcherTest : public ::testing::Test {
   // May not be called outside the SetUp()/TearDown() interval.
   int jobs_requested() const { return jobs_requested_; }
 
-  GURL PathToGurl(const char* path) {
+  GURL PathToGurl(const char* path) const {
     std::string gurl_string("http://");
     gurl_string += kTestDomain;
     gurl_string += "/";
@@ -186,10 +228,25 @@ class SdchDictionaryFetcherTest : public ::testing::Test {
     run_loop_.reset();
   }
 
+  HttpResponseInfo* response_info_to_return() {
+    return &response_info_to_return_;
+  }
+
+  int last_load_flags_seen() const { return last_load_flags_seen_; }
+
+  const SdchDictionaryFetcher::OnDictionaryFetchedCallback
+      GetDefaultCallback() {
+    return base::Bind(&SdchDictionaryFetcherTest::OnDictionaryFetched,
+                      base::Unretained(this));
+  }
+
  private:
-  void OnNumberJobsChanged(int outstanding_jobs_delta) {
+  void OnNumberJobsChanged(int outstanding_jobs_delta, int load_flags) {
+    DCHECK_NE(0, outstanding_jobs_delta);
     if (outstanding_jobs_delta > 0)
       jobs_requested_ += outstanding_jobs_delta;
+    else
+      last_load_flags_seen_ = load_flags;
     jobs_outstanding_ += outstanding_jobs_delta;
     if (jobs_outstanding_ == 0 && run_loop_)
       run_loop_->Quit();
@@ -197,17 +254,27 @@ class SdchDictionaryFetcherTest : public ::testing::Test {
 
   int jobs_requested_;
   int jobs_outstanding_;
+  // Last load flags seen by the interceptor installed in
+  // SdchDictionaryFetcherTest(). These are available to test bodies and
+  // currently used for ensuring that certain loads are marked only-from-cache.
+  int last_load_flags_seen_;
   scoped_ptr<base::RunLoop> run_loop_;
   scoped_ptr<TestURLRequestContext> context_;
   scoped_ptr<SdchDictionaryFetcher> fetcher_;
-  std::vector<DictionaryAdditions> dictionary_additions;
+  std::vector<DictionaryAdditions> dictionary_additions_;
+  // The request_time and response_time fields are filled in by the constructor
+  // for SdchDictionaryFetcherTest. Tests can fill the other fields of this
+  // member in to alter the HttpResponseInfo returned by the fetcher's
+  // URLRequestJob.
+  HttpResponseInfo response_info_to_return_;
   base::WeakPtrFactory<SdchDictionaryFetcherTest> factory_;
+  DISALLOW_COPY_AND_ASSIGN(SdchDictionaryFetcherTest);
 };
 
 // Schedule a fetch and make sure it happens.
 TEST_F(SdchDictionaryFetcherTest, Basic) {
   GURL dictionary_url(PathToGurl("dictionary"));
-  fetcher()->Schedule(dictionary_url);
+  fetcher()->Schedule(dictionary_url, GetDefaultCallback());
   WaitForNoJobs();
 
   EXPECT_EQ(1, jobs_requested());
@@ -217,14 +284,15 @@ TEST_F(SdchDictionaryFetcherTest, Basic) {
   EXPECT_EQ(
       URLRequestSpecifiedResponseJob::ExpectedResponseForURL(dictionary_url),
       additions[0].dictionary_text);
+  EXPECT_FALSE(last_load_flags_seen() & LOAD_ONLY_FROM_CACHE);
 }
 
 // Multiple fetches of the same URL should result in only one request.
 TEST_F(SdchDictionaryFetcherTest, Multiple) {
   GURL dictionary_url(PathToGurl("dictionary"));
-  fetcher()->Schedule(dictionary_url);
-  fetcher()->Schedule(dictionary_url);
-  fetcher()->Schedule(dictionary_url);
+  EXPECT_TRUE(fetcher()->Schedule(dictionary_url, GetDefaultCallback()));
+  EXPECT_FALSE(fetcher()->Schedule(dictionary_url, GetDefaultCallback()));
+  EXPECT_FALSE(fetcher()->Schedule(dictionary_url, GetDefaultCallback()));
   WaitForNoJobs();
 
   EXPECT_EQ(1, jobs_requested());
@@ -242,9 +310,9 @@ TEST_F(SdchDictionaryFetcherTest, Cancel) {
   GURL dictionary_url_2(PathToGurl("dictionary_2"));
   GURL dictionary_url_3(PathToGurl("dictionary_3"));
 
-  fetcher()->Schedule(dictionary_url_1);
-  fetcher()->Schedule(dictionary_url_2);
-  fetcher()->Schedule(dictionary_url_3);
+  fetcher()->Schedule(dictionary_url_1, GetDefaultCallback());
+  fetcher()->Schedule(dictionary_url_2, GetDefaultCallback());
+  fetcher()->Schedule(dictionary_url_3, GetDefaultCallback());
   fetcher()->Cancel();
   WaitForNoJobs();
 
@@ -257,8 +325,8 @@ TEST_F(SdchDictionaryFetcherTest, Cancel) {
 TEST_F(SdchDictionaryFetcherTest, LoopRace) {
   GURL dictionary0_url(PathToGurl("dictionary0"));
   GURL dictionary1_url(PathToGurl("dictionary1"));
-  fetcher()->Schedule(dictionary0_url);
-  fetcher()->Schedule(dictionary1_url);
+  fetcher()->Schedule(dictionary0_url, GetDefaultCallback());
+  fetcher()->Schedule(dictionary1_url, GetDefaultCallback());
   WaitForNoJobs();
 
   ASSERT_EQ(2, jobs_requested());
@@ -273,6 +341,82 @@ TEST_F(SdchDictionaryFetcherTest, LoopRace) {
       additions[1].dictionary_text);
 }
 
+TEST_F(SdchDictionaryFetcherTest, ScheduleReloadLoadFlags) {
+  GURL dictionary_url(PathToGurl("dictionary"));
+  fetcher()->ScheduleReload(dictionary_url, GetDefaultCallback());
+
+  WaitForNoJobs();
+  EXPECT_EQ(1, jobs_requested());
+  std::vector<DictionaryAdditions> additions;
+  GetDictionaryAdditions(&additions);
+  ASSERT_EQ(1u, additions.size());
+  EXPECT_EQ(
+      URLRequestSpecifiedResponseJob::ExpectedResponseForURL(dictionary_url),
+      additions[0].dictionary_text);
+  EXPECT_TRUE(last_load_flags_seen() & LOAD_ONLY_FROM_CACHE);
+}
+
+TEST_F(SdchDictionaryFetcherTest, ScheduleReloadFresh) {
+  std::string raw_headers = "\0";
+  response_info_to_return()->headers = new HttpResponseHeaders(
+      HttpUtil::AssembleRawHeaders(raw_headers.data(), raw_headers.size()));
+  response_info_to_return()->headers->AddHeader("Cache-Control: max-age=1000");
+
+  GURL dictionary_url(PathToGurl("dictionary"));
+  fetcher()->ScheduleReload(dictionary_url, GetDefaultCallback());
+
+  WaitForNoJobs();
+  EXPECT_EQ(1, jobs_requested());
+  std::vector<DictionaryAdditions> additions;
+  GetDictionaryAdditions(&additions);
+  ASSERT_EQ(1u, additions.size());
+  EXPECT_EQ(
+      URLRequestSpecifiedResponseJob::ExpectedResponseForURL(dictionary_url),
+      additions[0].dictionary_text);
+  EXPECT_TRUE(last_load_flags_seen() & LOAD_ONLY_FROM_CACHE);
+}
+
+TEST_F(SdchDictionaryFetcherTest, ScheduleReloadStale) {
+  response_info_to_return()->headers = new HttpResponseHeaders("");
+  response_info_to_return()->headers->AddHeader("Cache-Control: no-cache");
+
+  GURL dictionary_url(PathToGurl("dictionary"));
+  fetcher()->ScheduleReload(dictionary_url, GetDefaultCallback());
+
+  WaitForNoJobs();
+  EXPECT_EQ(1, jobs_requested());
+  std::vector<DictionaryAdditions> additions;
+  GetDictionaryAdditions(&additions);
+  EXPECT_EQ(0u, additions.size());
+  EXPECT_TRUE(last_load_flags_seen() & LOAD_ONLY_FROM_CACHE);
+}
+
+TEST_F(SdchDictionaryFetcherTest, ScheduleTwoReloads) {
+  GURL dictionary_url(PathToGurl("dictionary"));
+  EXPECT_TRUE(fetcher()->ScheduleReload(dictionary_url, GetDefaultCallback()));
+  EXPECT_TRUE(fetcher()->ScheduleReload(dictionary_url, GetDefaultCallback()));

[mmenke, 2015/02/20 21:58:12]

This shouldn't happen.

[Elly Fong-Jones, 2015/03/03 21:37:44]

Done.

+}
+
+TEST_F(SdchDictionaryFetcherTest, ScheduleReloadThenLoad) {
+  GURL dictionary_url(PathToGurl("dictionary"));
+  EXPECT_TRUE(fetcher()->ScheduleReload(dictionary_url, GetDefaultCallback()));
+  EXPECT_TRUE(fetcher()->Schedule(dictionary_url, GetDefaultCallback()));

[mmenke, 2015/02/20 21:58:12]

Should make sure we actually do a cached load, and if that fails, one that
bypasses the cache.

BUG:  We do in fact do that, but if we do a ScheduleReload with a different
dictionary first, we only do a load from the cache, I believe.

[Elly Fong-Jones, 2015/03/03 21:37:44]

Done.

+}
+
+TEST_F(SdchDictionaryFetcherTest, ScheduleThenReload) {
+  GURL dictionary_url(PathToGurl("dictionary"));
+  EXPECT_TRUE(fetcher()->Schedule(dictionary_url, GetDefaultCallback()));
+  EXPECT_FALSE(fetcher()->ScheduleReload(dictionary_url, GetDefaultCallback()));

[mmenke, 2015/02/20 21:58:12]

Should check that we actually only issue one request over the network, too, too,
just in case.

[Elly Fong-Jones, 2015/03/03 21:37:44]

Done.

+}
+
+TEST_F(SdchDictionaryFetcherTest, CancelAllowsFutureFetches) {
+  GURL dictionary_url(PathToGurl("dictionary"));
+  EXPECT_TRUE(fetcher()->Schedule(dictionary_url, GetDefaultCallback()));
+  EXPECT_FALSE(fetcher()->Schedule(dictionary_url, GetDefaultCallback()));
+  fetcher()->Cancel();

[mmenke, 2015/02/20 21:58:12]

Should make sure we cancel the pending network request.

+  EXPECT_TRUE(fetcher()->Schedule(dictionary_url, GetDefaultCallback()));

[mmenke, 2015/02/20 21:58:12]

Again, should make sure this really issues a network request.

+}
+
 }  // namespace
 
 }  // namespace net