Make SdchOwner resilient to dictionary used notifications post deletion.

BUG=454198
R=ellyjones@chromium.org

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

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

2 files changed, 59 insertions, 4 deletions

diff --git a/net/sdch/sdch_owner.cc b/net/sdch/sdch_owner.cc
index 29f2d41..9d98581 100644
--- a/net/sdch/sdch_owner.cc
+++ b/net/sdch/sdch_owner.cc
@@ -498,19 +498,32 @@ void SdchOwner::OnDictionaryUsed(SdchManager* manager,
 
   base::Value* value = nullptr;
   bool success = pref_dictionary_map->Get(server_hash, &value);
-  DCHECK(success);
+  if (!success) {
+    // SdchManager::GetDictionarySet() pins the referenced dictionaries in
+    // memory past a possible deletion.  For this reason, OnDictionaryUsed()
+    // notifications may occur after SdchOwner thinks that dictionaries
+    // have been deleted.
+    SdchManager::SdchErrorRecovery(SDCH_DICTIONARY_USED_AFTER_DELETION);
+    return;
+  }
   base::DictionaryValue* specific_dictionary_map = nullptr;
   success = value->GetAsDictionary(&specific_dictionary_map);
-  DCHECK(success);
+  // TODO(rdsmith); Switch back to DCHECK() after http://crbug.com/454198 is
+  // resolved.
+  CHECK(success);
 
   double last_used_seconds_since_epoch = 0.0;
   success = specific_dictionary_map->GetDouble(kDictionaryLastUsedKey,
                                                &last_used_seconds_since_epoch);
-  DCHECK(success);
+  // TODO(rdsmith); Switch back to DCHECK() after http://crbug.com/454198 is
+  // resolved.
+  CHECK(success);
   int use_count = 0;
   success =
       specific_dictionary_map->GetInteger(kDictionaryUseCountKey, &use_count);
-  DCHECK(success);
+  // TODO(rdsmith); Switch back to DCHECK() after http://crbug.com/454198 is
+  // resolved.
+  CHECK(success);
 
   if (use_counts_at_load_.count(server_hash) == 0) {
     use_counts_at_load_[server_hash] = use_count;
diff --git a/net/sdch/sdch_owner_unittest.cc b/net/sdch/sdch_owner_unittest.cc
index bbaceed..ebcbd02 100644
--- a/net/sdch/sdch_owner_unittest.cc
+++ b/net/sdch/sdch_owner_unittest.cc
@@ -624,6 +624,48 @@ TEST_F(SdchOwnerTest, MemoryPressureReturnsSpace) {
       CreateAndAddDictionary(kMaxSizeForTesting, nullptr, base::Time::Now()));
 }
 
+// Confirm that use of a pinned dictionary after its removal works properly.
+TEST_F(SdchOwnerTest, PinRemoveUse) {
+  pref_store().SetInitializationCompleted();
+  sdch_owner().EnablePersistentStorage(&pref_store());
+
+  std::string server_hash_d1;
+  EXPECT_TRUE(CreateAndAddDictionary(kMaxSizeForTesting / 2, &server_hash_d1,
+                                     base::Time::Now()));
+
+  scoped_ptr<SdchManager::DictionarySet> return_set(
+      sdch_manager().GetDictionarySet(
+          GURL(std::string(generic_url) + "/x.html")));
+  ASSERT_TRUE(return_set.get());
+  EXPECT_TRUE(return_set->GetDictionary(server_hash_d1));
+
+  const base::Value* result = nullptr;
+  const base::DictionaryValue* dict_result = nullptr;
+  ASSERT_TRUE(pref_store().GetValue("SDCH", &result));
+  ASSERT_TRUE(result->GetAsDictionary(&dict_result));
+  EXPECT_TRUE(dict_result->Get("dictionaries", &result));
+  EXPECT_TRUE(dict_result->Get("dictionaries." + server_hash_d1, &result));
+
+  sdch_manager().ClearData();
+
+  ASSERT_TRUE(pref_store().GetValue("SDCH", &result));
+  ASSERT_TRUE(result->GetAsDictionary(&dict_result));
+  EXPECT_TRUE(dict_result->Get("dictionaries", &result));
+  EXPECT_FALSE(dict_result->Get("dictionaries." + server_hash_d1, &result));
+
+  scoped_ptr<SdchManager::DictionarySet> return_set2(
+      sdch_manager().GetDictionarySet(
+          GURL(std::string(generic_url) + "/x.html")));
+  EXPECT_FALSE(return_set2.get());
+
+  sdch_manager().OnDictionaryUsed(server_hash_d1);
+
+  ASSERT_TRUE(pref_store().GetValue("SDCH", &result));
+  ASSERT_TRUE(result->GetAsDictionary(&dict_result));
+  EXPECT_TRUE(dict_result->Get("dictionaries", &result));
+  EXPECT_FALSE(dict_result->Get("dictionaries." + server_hash_d1, &result));
+}
+
 class SdchOwnerPersistenceTest : public ::testing::Test {
  public:
   SdchOwnerPersistenceTest() : pref_store_(new TestingPrefStore()) {