Decouple RulesCacheDelegate from RulesRegistry

chrome/browser/extensions/api/declarative/rules_cache_delegate.cc

Index: chrome/browser/extensions/api/declarative/rules_cache_delegate.cc
diff --git a/chrome/browser/extensions/api/declarative/rules_cache_delegate.cc b/chrome/browser/extensions/api/declarative/rules_cache_delegate.cc
index 5af6b58bd20d99ea73043db5ad88eb1f1eb72c23..3cf0772ae08d3feab637a8c05475101895bff97d 100644
--- a/chrome/browser/extensions/api/declarative/rules_cache_delegate.cc
+++ b/chrome/browser/extensions/api/declarative/rules_cache_delegate.cc
@@ -26,6 +26,28 @@ std::string GetDeclarativeRuleStorageKey(const std::string& event_name,
     return "declarative_rules." + event_name;
 }
 
+std::vector<linked_ptr<extensions::RulesRegistry::Rule> > RulesFromValue(
+    const base::Value* value) {
+  std::vector<linked_ptr<extensions::RulesRegistry::Rule> > rules;
+
+  const base::ListValue* list = NULL;
+  if (!value || !value->GetAsList(&list))
+    return rules;
+
+  rules.reserve(list->GetSize());
+  for (size_t i = 0; i < list->GetSize(); ++i) {
+    const base::DictionaryValue* dict = NULL;
+    if (!list->GetDictionary(i, &dict))
+      continue;
+    linked_ptr<extensions::RulesRegistry::Rule> rule(
+        new extensions::RulesRegistry::Rule());
+    if (extensions::RulesRegistry::Rule::Populate(*dict, rule.get()))
+      rules.push_back(rule);
+  }
+
+  return rules;
+}
+
 }  // namespace
 
 namespace extensions {
@@ -36,19 +58,16 @@ const char RulesCacheDelegate::kRulesStoredKey[] =
     "has_declarative_rules";
 
 RulesCacheDelegate::RulesCacheDelegate(
-    Profile* profile,
-    const std::string& event_name,
-    content::BrowserThread::ID rules_registry_thread,
     base::WeakPtr<RulesRegistry> registry,
     bool log_storage_init_delay)
-    : profile_(profile),
-      storage_key_(GetDeclarativeRuleStorageKey(event_name,
-                                                profile->IsOffTheRecord())),
-      rules_stored_key_(GetRulesStoredKey(event_name,
-                                          profile->IsOffTheRecord())),
+    : profile_(registry->profile()),
+      storage_key_(GetDeclarativeRuleStorageKey(registry->event_name(),
+                                                profile_->IsOffTheRecord())),
+      rules_stored_key_(GetRulesStoredKey(registry->event_name(),
+                                          profile_->IsOffTheRecord())),
       log_storage_init_delay_(log_storage_init_delay),
       registry_(registry),
-      rules_registry_thread_(rules_registry_thread),
+      rules_registry_thread_(registry->owner_thread()),
       notified_registry_(false),
       weak_ptr_factory_(this) {}
 
@@ -205,7 +224,7 @@ void RulesCacheDelegate::ReadFromStorageCallback(
   content::BrowserThread::PostTask(
       rules_registry_thread_,
       FROM_HERE,
-      base::Bind(&RulesRegistry::DeserializeAndAddRules,
+      base::Bind(&RulesCacheDelegate::DeserializeAndAddRules,
                  registry_,
                  extension_id,
                  base::Passed(&value)));
@@ -243,4 +262,17 @@ void RulesCacheDelegate::SetDeclarativeRulesStored(
       new base::FundamentalValue(rules_stored));
 }
 
+// static
+void RulesCacheDelegate::DeserializeAndAddRules(

[Jeffrey Yasskin, 2013/11/01 00:15:09]

I'm curious why this moved. It doesn't seem to need a RulesCacheDelegate, and it
runs on the registry's thread rather than the UI thread, which is the general
split between methods of RulesRegistry and RulesCacheDelegate.

[Fady Samuel, 2013/11/01 15:13:39]

I thought it would be cleaner but it didn't really turn out that way so I'll
move it back.

+    base::WeakPtr<RulesRegistry> registry,
+    const std::string& extension_id,
+    scoped_ptr<base::Value> rules) {
+  DCHECK(content::BrowserThread::CurrentlyOn(registry->owner_thread()));
+
+  if (!registry)

[Jeffrey Yasskin, 2013/11/01 00:15:09]

This needs to be before the CurrentlyOn call, which assumes |registry| is not
null.

[Fady Samuel, 2013/11/01 15:13:39]

This is no longer relevant. DeserialAndAddRules has been moved back.

+    return;
+
+  registry->AddRules(extension_id, RulesFromValue(rules.get()));
+}
+
 }  // namespace extensions