Make SDCH dictionaries persistent across browser restart.

net/sdch/sdch_owner_unittest.cc

 // Copyright 2014 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 "base/memory/memory_pressure_listener.h"
+#include "base/prefs/testing_pref_store.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/simple_test_clock.h"
+#include "base/values.h"
 #include "net/base/net_log.h"
 #include "net/base/sdch_manager.h"
 #include "net/sdch/sdch_owner.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_job_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ skipping 94 matching lines @@
 
   bool IsHandledURL(const GURL& url) const override {
     return url.SchemeIs("http");
   }
 
   bool IsSafeRedirectTarget(const GURL& location) const override {
     return false;
   }
 };
 
+// TestingPrefStore with some additional helper methods.
+class SdchTestingPrefStore : public TestingPrefStore {
+ public:
+  void CreateFirstLevelDictionary(const std::string& key) {
+    SetValue(key, new base::DictionaryValue());
+  }
+
+  // All of the following require that the parent dictionaries already exist.
+  void CreateSecondLevelDictionary(const std::string& key1,
+                                   const std::string& key2) {
+    base::Value* dict1_as_value;
+    base::DictionaryValue* dict1_as_dict;
+    bool success = GetMutableValue(key1, &dict1_as_value);
+    DCHECK(success);
+    success = dict1_as_value->GetAsDictionary(&dict1_as_dict);
+    DCHECK(success);

[mmenke, 2015/02/12 16:51:33]

Read my comments in the tests themselves before doing anything here...I'm not
sure we should be creating all these dictionaries manually.  Also, since we know
the structure of the dictionary, I think we can simplify the ones we do need. 
i.e.  "SetExpiractionTime(const GURL&, base::Time);"  If we ever modify the
format of the dictionary, that works better, too.

[mmenke, 2015/02/12 16:51:33]

Suggest a helper method for these 4 lines...Or maybe just a "GetFirstLevelDict"
method?  Actually, do we care about anything other than the SDCH dictionary? 
Rather than pass that everywhere, maybe just GetSDCHDictionary?

[Elly Fong-Jones, 2015/02/13 23:35:00]

Done.

[Elly Fong-Jones, 2015/02/13 23:35:00]

Done.

+    dict1_as_dict->Set(key2, new base::DictionaryValue());
+  }
+
+  void SetSecondLevelInteger(const std::string& key1,
+                             const std::string& key2,
+                             int value) {
+    base::Value* dict1_as_value;
+    base::DictionaryValue* dict1_as_dict;
+    bool success = GetMutableValue(key1, &dict1_as_value);
+    DCHECK(success);
+    success = dict1_as_value->GetAsDictionary(&dict1_as_dict);
+    DCHECK(success);
+    dict1_as_dict->SetInteger(key2, value);
+  }
+
+  void CreateThirdLevelDictionary(const std::string& key1,
+                                  const std::string& key2,
+                                  const std::string& key3) {
+    GetSecondLevelDict(key1, key2)->Set(key3, new base::DictionaryValue());
+  }
+
+  void SetFourthLevelInteger(const std::string& key1,
+                             const std::string& key2,
+                             const std::string& key3,
+                             const std::string& key4,
+                             int value) {
+    GetThirdLevelDict(key1, key2, key3)->SetInteger(key4, value);
+  }
+
+  void SetFourthLevelString(const std::string& key1,
+                            const std::string& key2,
+                            const std::string& key3,
+                            const std::string& key4,
+                            const std::string& value) {
+    GetThirdLevelDict(key1, key2, key3)->SetString(key4, value);
+  }
+
+  void SetFourthLevelDouble(const std::string& key1,
+                            const std::string& key2,
+                            const std::string& key3,
+                            const std::string& key4,
+                            double value) {
+    GetThirdLevelDict(key1, key2, key3)->SetDouble(key4, value);
+  }
+
+ private:
+  base::DictionaryValue* GetSecondLevelDict(const std::string& key1,
+                                            const std::string& key2) {
+    base::Value* dict1_as_value;
+    base::DictionaryValue* dict1_as_dict;
+    bool success = GetMutableValue(key1, &dict1_as_value);
+    DCHECK(success);
+    success = dict1_as_value->GetAsDictionary(&dict1_as_dict);
+    DCHECK(success);
+
+    base::Value* dict2_as_value;
+    base::DictionaryValue* dict2_as_dict;
+    success = dict1_as_dict->Get(key2, &dict2_as_value);
+    DCHECK(success);
+    success = dict2_as_value->GetAsDictionary(&dict2_as_dict);

[mmenke, 2015/02/12 16:51:32]

Can just use dict1_as_dict->GetDictionary(&dict2);  (Or
GetDictionaryWithoutPathExpansion)

[Elly Fong-Jones, 2015/02/13 23:35:00]

Done.

+    DCHECK(success);
+    return dict2_as_dict;
+  }
+
+  base::DictionaryValue* GetThirdLevelDict(const std::string& key1,
+                                           const std::string& key2,
+                                           const std::string& key3) {
+    base::Value* dict3_as_value;
+    base::DictionaryValue* dict3_as_dict;
+    bool success = GetSecondLevelDict(key1, key2)->Get(key3, &dict3_as_value);
+    DCHECK(success);
+    success = dict3_as_value->GetAsDictionary(&dict3_as_dict);

[mmenke, 2015/02/12 16:51:33]

Again, can just GetDictionary...Or GetDictionaryWithoutPathExpansion.  Or if
don't mind using path expansion, could actually just use the top level dict, and
use GetDictionary(key2 + "/" + key3);

[Elly Fong-Jones, 2015/02/13 23:35:00]

Done.

+    DCHECK(success);
+    return dict3_as_dict;
+  }
+};
+
+// File testing infrastructure summary:
+// * NewSdchDictionary(): Creates a dictionary of a specific size.
+// * URLRequestErrorCountingJob: A URLRequestJob that returns an error
+//   and counts the number of outstanding (started but not finished)
+//   jobs, and calls a global callback when that number transitions to zero.
+// * MockURLRequestJobFactory: Factory to create the above jobs.  Tracks
+//   the number of jobs created.
+// * SdchTestingPrefStore: A persistent pref store that can be configured
+//   for testing SdchOwner persistence functionality.
+// * SdchOwnerTest: Interfaces
+//      * Access manager, owner, and net log
+//      * Return the number of jobs created in a time interval
+//      * Return dictionary present in the manager
+//      * Notify SdchOwner of an incoming dictionary (& wait until jobs clear)
+//      * Attempt to add a dictionary and test for success.
+// Test patterns:
+//      * Let the owner know about a Get-Dictionary header and test for
+//        appropriate jobs being created.
+//      * Let the owner know that a dictionary was successfully fetched
+//        and test for appropriate outcome.
+//      * Either of the above, having previously added dictionaries to create
+//        a particular initial state.
 class SdchOwnerTest : public testing::Test {
  public:
   static const size_t kMaxSizeForTesting = 1000 * 50;
   static const size_t kMinFetchSpaceForTesting = 500;
 
   SdchOwnerTest()
       : last_jobs_created_(error_jobs_created),
         dictionary_creation_index_(0),
+        pref_store_(new SdchTestingPrefStore),
         sdch_owner_(&sdch_manager_, &url_request_context_) {
     // Any jobs created on this context will immediately error,
     // which leaves the test in control of signals to SdchOwner.
     url_request_context_.set_job_factory(&job_factory_);
 
     // Reduce sizes to reduce time for string operations.
     sdch_owner_.SetMaxTotalDictionarySize(kMaxSizeForTesting);
     sdch_owner_.SetMinSpaceForDictionaryFetch(kMinFetchSpaceForTesting);
   }
 
   SdchManager& sdch_manager() { return sdch_manager_; }
   SdchOwner& sdch_owner() { return sdch_owner_; }
   BoundNetLog& bound_net_log() { return net_log_; }
+  SdchTestingPrefStore& pref_store() { return *(pref_store_.get()); }
 
   int JobsRecentlyCreated() {
     int result = error_jobs_created - last_jobs_created_;
     last_jobs_created_ = error_jobs_created;
     return result;
   }
 
   bool DictionaryPresentInManager(const std::string& server_hash) {
     // Presumes all tests use generic url.
     SdchProblemCode tmp;
     scoped_ptr<SdchManager::DictionarySet> set(
         sdch_manager_.GetDictionarySetByHash(GURL(generic_url), server_hash,
                                              &tmp));
     return !!set.get();
   }
 
-  void SignalGetDictionaryAndClearJobs(GURL request_url, GURL dictionary_url) {
-    sdch_owner().OnGetDictionary(&sdch_manager_, request_url, dictionary_url);
+  void WaitForNoJobs() {
     if (outstanding_url_request_error_counting_jobs == 0)
       return;
+
     base::RunLoop run_loop;
     base::Closure quit_closure(run_loop.QuitClosure());
     empty_url_request_jobs_callback = &quit_closure;
     run_loop.Run();
     empty_url_request_jobs_callback = NULL;
   }
 
+  void SignalGetDictionaryAndClearJobs(GURL request_url, GURL dictionary_url) {
+    sdch_owner().OnGetDictionary(&sdch_manager_, request_url, dictionary_url);
+    WaitForNoJobs();
+  }
+
   // Create a unique (by hash) dictionary of the given size,
   // associate it with a unique URL, add it to the manager through
   // SdchOwner::OnDictionaryFetched(), and return whether that
   // addition was successful or not.
-  bool CreateAndAddDictionary(size_t size, std::string* server_hash_p) {
+  bool CreateAndAddDictionary(size_t size,
+                              std::string* server_hash_p,
+                              base::Time last_used_time) {
     GURL dictionary_url(
         base::StringPrintf("%s/d%d", generic_url, dictionary_creation_index_));
     std::string dictionary_text(NewSdchDictionary(size - 4));
     dictionary_text += base::StringPrintf("%04d", dictionary_creation_index_);
     ++dictionary_creation_index_;
     std::string client_hash;
     std::string server_hash;
     SdchManager::GenerateHash(dictionary_text, &client_hash, &server_hash);
 
     if (DictionaryPresentInManager(server_hash))
       return false;
-    sdch_owner().OnDictionaryFetched(dictionary_text, dictionary_url, net_log_);
+    sdch_owner().OnDictionaryFetched(last_used_time, 0, dictionary_text,
+                                     dictionary_url, net_log_);
     if (server_hash_p)
       *server_hash_p = server_hash;
     return DictionaryPresentInManager(server_hash);
   }
 
  private:
   int last_jobs_created_;
   BoundNetLog net_log_;
   int dictionary_creation_index_;
 
   // The dependencies of these objects (sdch_owner_ -> {sdch_manager_,
   // url_request_context_}, url_request_context_->job_factory_) require
   // this order for correct destruction semantics.
   MockURLRequestJobFactory job_factory_;
   URLRequestContext url_request_context_;
   SdchManager sdch_manager_;
+  scoped_refptr<SdchTestingPrefStore> pref_store_;
   SdchOwner sdch_owner_;
 
   DISALLOW_COPY_AND_ASSIGN(SdchOwnerTest);
 };
 
 // Does OnGetDictionary result in a fetch when there's enough space, and not
 // when there's not?
 TEST_F(SdchOwnerTest, OnGetDictionary_Fetching) {
   GURL request_url(std::string(generic_url) + "/r1");
 
   // Fetch generated when empty.
   GURL dict_url1(std::string(generic_url) + "/d1");
   EXPECT_EQ(0, JobsRecentlyCreated());
   SignalGetDictionaryAndClearJobs(request_url, dict_url1);
   EXPECT_EQ(1, JobsRecentlyCreated());
 
   // Fetch generated when half full.
   GURL dict_url2(std::string(generic_url) + "/d2");
   std::string dictionary1(NewSdchDictionary(kMaxSizeForTesting / 2));
-  sdch_owner().OnDictionaryFetched(dictionary1, dict_url1, bound_net_log());
+  sdch_owner().OnDictionaryFetched(base::Time::Now(), 1, dictionary1, dict_url1,
+                                   bound_net_log());
   EXPECT_EQ(0, JobsRecentlyCreated());
   SignalGetDictionaryAndClearJobs(request_url, dict_url2);
   EXPECT_EQ(1, JobsRecentlyCreated());
 
   // Fetch not generated when close to completely full.
   GURL dict_url3(std::string(generic_url) + "/d3");
   std::string dictionary2(NewSdchDictionary(
       (kMaxSizeForTesting / 2 - kMinFetchSpaceForTesting / 2)));
-  sdch_owner().OnDictionaryFetched(dictionary2, dict_url2, bound_net_log());
+  sdch_owner().OnDictionaryFetched(base::Time::Now(), 1, dictionary2, dict_url2,
+                                   bound_net_log());
   EXPECT_EQ(0, JobsRecentlyCreated());
   SignalGetDictionaryAndClearJobs(request_url, dict_url3);
   EXPECT_EQ(0, JobsRecentlyCreated());
 }
 
 // Make sure attempts to add dictionaries do what they should.
 TEST_F(SdchOwnerTest, OnDictionaryFetched_Fetching) {
   GURL request_url(std::string(generic_url) + "/r1");
   std::string client_hash;
   std::string server_hash;
 
+  // In the past, but still fresh for an unused dictionary.
+  base::Time dictionary_last_used_time(base::Time::Now() -
+                                       base::TimeDelta::FromMinutes(30));
+
   // Add successful when empty.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, nullptr));
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, nullptr,
+                                     dictionary_last_used_time));
   EXPECT_EQ(0, JobsRecentlyCreated());
 
   // Add successful when half full.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, nullptr));
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, nullptr,
+                                     dictionary_last_used_time));
   EXPECT_EQ(0, JobsRecentlyCreated());
 
   // Add unsuccessful when full.
-  EXPECT_FALSE(CreateAndAddDictionary(kMaxSizeForTesting / 2, nullptr));
+  EXPECT_FALSE(CreateAndAddDictionary(kMaxSizeForTesting / 2, nullptr,
+                                      dictionary_last_used_time));
   EXPECT_EQ(0, JobsRecentlyCreated());
 }
 
 // Confirm auto-eviction happens if space is needed.
 TEST_F(SdchOwnerTest, ConfirmAutoEviction) {
   std::string server_hash_d1;
   std::string server_hash_d2;
   std::string server_hash_d3;
 
   // Add two dictionaries, one recent, one more than a day in the past.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d1));
+  base::Time fresh(base::Time::Now() - base::TimeDelta::FromHours(23));
+  base::Time stale(base::Time::Now() - base::TimeDelta::FromHours(25));
 
-  scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(2));
-  sdch_owner().SetClockForTesting(clock.Pass());
-
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d2));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d1, fresh));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d2, stale));
 
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d2));
 
-  // The addition of a new dictionary should succeed, evicting the old one.
-  clock.reset(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now());
-  sdch_owner().SetClockForTesting(clock.Pass());
-
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d3));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d3, fresh));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3));
 }
 
 // Confirm auto-eviction happens if space is needed, with a more complicated
 // situation
 TEST_F(SdchOwnerTest, ConfirmAutoEviction_2) {
   std::string server_hash_d1;
   std::string server_hash_d2;
   std::string server_hash_d3;
 
   // Add dictionaries, one recent, two more than a day in the past that
   // between them add up to the space needed.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d1));
+  base::Time fresh(base::Time::Now() - base::TimeDelta::FromHours(23));
+  base::Time stale(base::Time::Now() - base::TimeDelta::FromHours(25));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d1, fresh));
 
-  scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(2));
-  sdch_owner().SetClockForTesting(clock.Pass());
-
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d2));
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d3));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d2, stale));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d3, stale));
 
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3));
 
-  // The addition of a new dictionary should succeed, evicting the old one.
-  clock.reset(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now());
-  sdch_owner().SetClockForTesting(clock.Pass());
-
   std::string server_hash_d4;
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d4));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d4, fresh));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d3));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d4));
 }
 
 // Confirm if only one dictionary needs to be evicted it's the oldest.
 TEST_F(SdchOwnerTest, ConfirmAutoEviction_Oldest) {
   std::string server_hash_d1;
   std::string server_hash_d2;
   std::string server_hash_d3;
 
   // Add dictionaries, one recent, one two days in the past, and one
   // four days in the past.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d1));
+  base::Time fresh(base::Time::Now() - base::TimeDelta::FromHours(23));
+  base::Time stale_newer(base::Time::Now() - base::TimeDelta::FromHours(47));
+  base::Time stale_older(base::Time::Now() - base::TimeDelta::FromHours(71));
 
-  scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(2));
-  sdch_owner().SetClockForTesting(clock.Pass());
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d2));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d1, fresh));
 
-  clock.reset(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(4));
-  sdch_owner().SetClockForTesting(clock.Pass());
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d3));
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d2,
+                                     stale_newer));
+
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d3,
+                                     stale_older));
 
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3));
 
   // The addition of a new dictionary should succeed, evicting only the
   // oldest one.
-  clock.reset(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now());
-  sdch_owner().SetClockForTesting(clock.Pass());
 
   std::string server_hash_d4;
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d4));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d4, fresh));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d3));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d4));
 }
 
 // Confirm using a dictionary changes eviction behavior properly.
 TEST_F(SdchOwnerTest, UseChangesEviction) {
   std::string server_hash_d1;
   std::string server_hash_d2;
   std::string server_hash_d3;
 
   // Add dictionaries, one recent, one two days in the past, and one
   // four days in the past.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d1));
+  base::Time fresh(base::Time::Now() - base::TimeDelta::FromHours(23));
+  base::Time stale_newer(base::Time::Now() - base::TimeDelta::FromHours(47));
+  base::Time stale_older(base::Time::Now() - base::TimeDelta::FromHours(71));
 
-  scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(2));
-  sdch_owner().SetClockForTesting(clock.Pass());
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d2));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d1, fresh));
 
-  clock.reset(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(4));
-  sdch_owner().SetClockForTesting(clock.Pass());
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d3));
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d2,
+                                     stale_newer));
+
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d3,
+                                     stale_older));
 
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3));
 
-  clock.reset(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now());
-  sdch_owner().SetClockForTesting(clock.Pass());
-
   // Use the oldest dictionary.
   sdch_owner().OnDictionaryUsed(&sdch_manager(), server_hash_d3);
 
   // The addition of a new dictionary should succeed, evicting only the
   // newer stale one.
   std::string server_hash_d4;
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d4));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d4, fresh));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d4));
 }
 
 // Confirm using a dictionary can prevent the addition of a new dictionary.
 TEST_F(SdchOwnerTest, UsePreventsAddition) {
   std::string server_hash_d1;
   std::string server_hash_d2;
   std::string server_hash_d3;
 
   // Add dictionaries, one recent, one two days in the past, and one
   // four days in the past.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d1));
+  base::Time fresh(base::Time::Now() - base::TimeDelta::FromMinutes(30));
+  base::Time stale_newer(base::Time::Now() - base::TimeDelta::FromHours(47));
+  base::Time stale_older(base::Time::Now() - base::TimeDelta::FromHours(71));
 
-  scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(2));
-  sdch_owner().SetClockForTesting(clock.Pass());
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d2));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d1, fresh));
 
-  clock.reset(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(4));
-  sdch_owner().SetClockForTesting(clock.Pass());
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d3));
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d2,
+                                     stale_newer));
+
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 4, &server_hash_d3,
+                                     stale_older));
 
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3));
 
-  clock.reset(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now());
-  sdch_owner().SetClockForTesting(clock.Pass());
-
   // Use the older dictionaries.
   sdch_owner().OnDictionaryUsed(&sdch_manager(), server_hash_d2);
   sdch_owner().OnDictionaryUsed(&sdch_manager(), server_hash_d3);
 
   // The addition of a new dictionary should fail, not evicting anything.
   std::string server_hash_d4;
-  EXPECT_FALSE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d4));
+  EXPECT_FALSE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d4, fresh));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d2));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d3));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d4));
 }
 
 // Confirm clear gets all the space back.
 TEST_F(SdchOwnerTest, ClearReturnsSpace) {
   std::string server_hash_d1;
   std::string server_hash_d2;
 
   // Take up all the space.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting, &server_hash_d1));
-
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting, &server_hash_d1,
+                                     base::Time::Now()));
   // Addition should fail.
-  EXPECT_FALSE(CreateAndAddDictionary(kMaxSizeForTesting, &server_hash_d2));
-
+  EXPECT_FALSE(CreateAndAddDictionary(kMaxSizeForTesting, &server_hash_d2,
+                                      base::Time::Now()));
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d2));
-
   sdch_manager().ClearData();
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d2));
 
   // Addition should now succeed.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting, nullptr));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting, nullptr, base::Time::Now()));
 }
 
 // Confirm memory pressure gets all the space back.
 TEST_F(SdchOwnerTest, MemoryPressureReturnsSpace) {
   std::string server_hash_d1;
   std::string server_hash_d2;
 
   // Take up all the space.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting, &server_hash_d1));
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting, &server_hash_d1,
+                                     base::Time::Now()));
 
   // Addition should fail.
-  EXPECT_FALSE(CreateAndAddDictionary(kMaxSizeForTesting, &server_hash_d2));
+  EXPECT_FALSE(CreateAndAddDictionary(kMaxSizeForTesting, &server_hash_d2,
+                                      base::Time::Now()));
 
   EXPECT_TRUE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d2));
 
   base::MemoryPressureListener::NotifyMemoryPressure(
       base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE);
   // The notification may have (implementation note: does :-}) use a PostTask,
   // so we drain the local message queue.  This should be safe (i.e. not have
   // an inifinite number of messages) in a unit test.
   base::RunLoop().RunUntilIdle();
 
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d1));
   EXPECT_FALSE(DictionaryPresentInManager(server_hash_d2));
 
   // Addition should now succeed.
-  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting, nullptr));
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting, nullptr, base::Time::Now()));
+}
+
+// Test an empty persistence store.
+TEST_F(SdchOwnerTest, Persistent_Empty) {
+  pref_store().SetInitializationCompleted();
+  EXPECT_EQ(0, JobsRecentlyCreated());
+  sdch_owner().EnablePersistentStorage(&pref_store());
+  WaitForNoJobs();
+  EXPECT_EQ(0, JobsRecentlyCreated());
+}
+
+// Test a persistence store with an empty dictionary.
+TEST_F(SdchOwnerTest, Persistent_EmptyDict) {
+  pref_store().CreateFirstLevelDictionary("SDCH");
+  pref_store().SetInitializationCompleted();
+
+  EXPECT_EQ(0, JobsRecentlyCreated());
+  sdch_owner().EnablePersistentStorage(&pref_store());
+  WaitForNoJobs();
+  EXPECT_EQ(0, JobsRecentlyCreated());
+}
+
+// Test a persistence store with a bad version number.
+TEST_F(SdchOwnerTest, Persistent_BadVersion) {
+  pref_store().CreateFirstLevelDictionary("SDCH");
+  pref_store().SetSecondLevelInteger("SDCH", "version", 2);
+  pref_store().SetInitializationCompleted();
+
+  EXPECT_EQ(0, JobsRecentlyCreated());
+  sdch_owner().EnablePersistentStorage(&pref_store());
+  WaitForNoJobs();
+  EXPECT_EQ(0, JobsRecentlyCreated());
+}
+
+// Test a persistence store with a bad version number.
+TEST_F(SdchOwnerTest, Persistent_EmptyDictList) {
+  pref_store().CreateFirstLevelDictionary("SDCH");
+  pref_store().SetSecondLevelInteger("SDCH", "version", 1);
+  pref_store().CreateSecondLevelDictionary("SDCH", "dictionaries");
+  pref_store().SetInitializationCompleted();

[mmenke, 2015/02/12 16:51:33]

We should have a test where this happens after initialization.  And one where it
happens after destruction of the dictionary.

[Elly Fong-Jones, 2015/02/13 23:35:00]

Where what happens after initialization? Modification of the pref store? I think
we currently deliberately don't listen for that (?).

+
+  EXPECT_EQ(0, JobsRecentlyCreated());
+  sdch_owner().EnablePersistentStorage(&pref_store());
+  WaitForNoJobs();
+  EXPECT_EQ(0, JobsRecentlyCreated());
+}
+
+// Test a persistence store with a single good dictionary.
+TEST_F(SdchOwnerTest, Persistent_OneDict) {
+  pref_store().CreateFirstLevelDictionary("SDCH");
+  pref_store().SetSecondLevelInteger("SDCH", "version", 1);
+  pref_store().CreateSecondLevelDictionary("SDCH", "dictionaries");
+  pref_store().CreateThirdLevelDictionary("SDCH", "dictionaries", "hashhash");
+  pref_store().SetFourthLevelString("SDCH", "dictionaries", "hashhash", "url",
+                                    "http://sample.toplevel.com/dict");
+  pref_store().SetFourthLevelInteger("SDCH", "dictionaries", "hashhash",
+                                     "use_count", 1);
+  pref_store().SetFourthLevelDouble("SDCH", "dictionaries", "hashhash",
+                                    "last_used", base::Time::Now().ToDoubleT());

[mmenke, 2015/02/12 16:51:32]

I wonder about hard coding all this.  Can we just create an SDCH manager with a
pref store, have it load the dictionaries, flush it to disk and clean it up, and
then load from that pref store?

That means we don't have to worry about changes to the format, and also means we
test the saving as well as the loading of the dictionary, which we aren't
currently testing.

[Elly Fong-Jones, 2015/02/13 23:35:00]

Done.

+  pref_store().SetInitializationCompleted();
+
+  EXPECT_EQ(0, JobsRecentlyCreated());
+  sdch_owner().EnablePersistentStorage(&pref_store());
+  WaitForNoJobs();
+  EXPECT_EQ(1, JobsRecentlyCreated());
+}
+
+// Test a persistence store with two good dictionaries.
+TEST_F(SdchOwnerTest, Persistent_TwoDict) {
+  pref_store().CreateFirstLevelDictionary("SDCH");
+  pref_store().SetSecondLevelInteger("SDCH", "version", 1);
+  pref_store().CreateSecondLevelDictionary("SDCH", "dictionaries");
+  pref_store().CreateThirdLevelDictionary("SDCH", "dictionaries", "hashhash");
+  pref_store().SetFourthLevelString("SDCH", "dictionaries", "hashhash", "url",
+                                    "http://sample.toplevel.com/dict");
+  pref_store().SetFourthLevelInteger("SDCH", "dictionaries", "hashhash",
+                                     "use_count", 1);
+  pref_store().SetFourthLevelDouble("SDCH", "dictionaries", "hashhash",
+                                    "last_used", base::Time::Now().ToDoubleT());
+  pref_store().CreateThirdLevelDictionary("SDCH", "dictionaries", "hashhss2");
+  pref_store().SetFourthLevelString("SDCH", "dictionaries", "hashhss2", "url",
+                                    "http://sample.toplevel.com/dict2");
+  pref_store().SetFourthLevelInteger("SDCH", "dictionaries", "hashhss2",
+                                     "use_count", 1);
+  pref_store().SetFourthLevelDouble("SDCH", "dictionaries", "hashhss2",
+                                    "last_used", base::Time::Now().ToDoubleT());
+  pref_store().SetInitializationCompleted();
+
+  EXPECT_EQ(0, JobsRecentlyCreated());
+  sdch_owner().EnablePersistentStorage(&pref_store());
+  WaitForNoJobs();
+  EXPECT_EQ(2, JobsRecentlyCreated());
+}
+
+// Test a persistence store with one good and one bad dictionary.
+TEST_F(SdchOwnerTest, Persistent_OneBad) {
+  pref_store().CreateFirstLevelDictionary("SDCH");
+  pref_store().SetSecondLevelInteger("SDCH", "version", 1);
+  pref_store().CreateSecondLevelDictionary("SDCH", "dictionaries");
+  pref_store().CreateThirdLevelDictionary("SDCH", "dictionaries", "hashhash");
+  pref_store().SetFourthLevelString("SDCH", "dictionaries", "hashhash", "url",
+                                    "http://sample.toplevel.com/dict");
+  pref_store().SetFourthLevelInteger("SDCH", "dictionaries", "hashhash",
+                                     "use_count", 1);
+  pref_store().SetFourthLevelDouble("SDCH", "dictionaries", "hashhash",
+                                    "last_used", base::Time::Now().ToDoubleT());
+
+  pref_store().CreateThirdLevelDictionary("SDCH", "dictionaries", "hashhss2");
+  pref_store().SetFourthLevelString("SDCH", "dictionaries", "hashhss2", "url",
+                                    "http://sample.toplevel.com/dict2");
+  pref_store().SetFourthLevelDouble("SDCH", "dictionaries", "hashhss2",
+                                    "last_used", base::Time::Now().ToDoubleT());
+  pref_store().SetInitializationCompleted();
+
+  EXPECT_EQ(0, JobsRecentlyCreated());
+  sdch_owner().EnablePersistentStorage(&pref_store());
+  WaitForNoJobs();
+  EXPECT_EQ(1, JobsRecentlyCreated());
 }

[mmenke, 2015/02/12 16:51:33]

Other test suggestions:

* If we load a dict and the init completes, we correctly merge the list of
dictionaries.
* Eviction has the right priority for loaded dictionaries (A loaded used
dictionary that's, say, 25 hours old can be evicted, as can a loaded unused one
that's 2 hours old, but a loaded used one that's 12 hours old is not - could
have one test with all those dictionaries).

[Elly Fong-Jones, 2015/02/13 23:35:00]

Done.
I'll need to dig a little more to understand how to do this.

 
 }  // namespace net