Make SDCH dictionaries persistent across browser restart.

net/sdch/sdch_owner_unittest.cc

Index: net/sdch/sdch_owner_unittest.cc
diff --git a/net/sdch/sdch_owner_unittest.cc b/net/sdch/sdch_owner_unittest.cc
index e89533a361c4a45477c6affcf16a87ea2820031b..d54c74b94e300c00ad28dff8c9de8aceb2deb753 100644
--- a/net/sdch/sdch_owner_unittest.cc
+++ b/net/sdch/sdch_owner_unittest.cc
@@ -3,9 +3,11 @@
 // 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"
@@ -120,6 +122,121 @@ class MockURLRequestJobFactory : public URLRequestJobFactory {
   }
 };
 
+// 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;
@@ -128,6 +245,7 @@ class SdchOwnerTest : public testing::Test {
   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.
@@ -141,6 +259,7 @@ class SdchOwnerTest : public testing::Test {
   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_;
@@ -157,10 +276,10 @@ class SdchOwnerTest : public testing::Test {
     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;
@@ -168,11 +287,18 @@ class SdchOwnerTest : public testing::Test {
     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));
@@ -184,7 +310,8 @@ class SdchOwnerTest : public testing::Test {
 
     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);
@@ -201,6 +328,7 @@ class SdchOwnerTest : public testing::Test {
   MockURLRequestJobFactory job_factory_;
   URLRequestContext url_request_context_;
   SdchManager sdch_manager_;
+  scoped_refptr<SdchTestingPrefStore> pref_store_;
   SdchOwner sdch_owner_;
 
   DISALLOW_COPY_AND_ASSIGN(SdchOwnerTest);
@@ -220,7 +348,8 @@ TEST_F(SdchOwnerTest, OnGetDictionary_Fetching) {
   // 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());
@@ -229,7 +358,8 @@ TEST_F(SdchOwnerTest, OnGetDictionary_Fetching) {
   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());
@@ -241,16 +371,23 @@ TEST_F(SdchOwnerTest, OnDictionaryFetched_Fetching) {
   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());
 }
 
@@ -261,23 +398,19 @@ TEST_F(SdchOwnerTest, ConfirmAutoEviction) {
   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));
-
-  scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
-  clock->SetNow(base::Time::Now() - base::TimeDelta::FromDays(2));
-  sdch_owner().SetClockForTesting(clock.Pass());
+  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_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));
@@ -292,26 +425,23 @@ TEST_F(SdchOwnerTest, ConfirmAutoEviction_2) {
 
   // 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));
@@ -326,17 +456,18 @@ TEST_F(SdchOwnerTest, ConfirmAutoEviction_Oldest) {
 
   // 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));
@@ -344,12 +475,10 @@ TEST_F(SdchOwnerTest, ConfirmAutoEviction_Oldest) {
 
   // 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));
@@ -364,33 +493,31 @@ TEST_F(SdchOwnerTest, UseChangesEviction) {
 
   // 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));
+
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 4, &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_d2,
+                                     stale_newer));
 
-  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_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));
@@ -405,33 +532,31 @@ TEST_F(SdchOwnerTest, UsePreventsAddition) {
 
   // 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));
+
+  EXPECT_TRUE(
+      CreateAndAddDictionary(kMaxSizeForTesting / 4, &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_d2,
+                                     stale_newer));
 
-  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_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));
@@ -444,20 +569,20 @@ TEST_F(SdchOwnerTest, ClearReturnsSpace) {
   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.
@@ -466,10 +591,12 @@ TEST_F(SdchOwnerTest, MemoryPressureReturnsSpace) {
   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));
@@ -485,7 +612,126 @@ TEST_F(SdchOwnerTest, MemoryPressureReturnsSpace) {
   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