Decouple RulesCacheDelegate from RulesRegistry

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

Index: chrome/browser/extensions/api/declarative/rules_registry.cc
diff --git a/chrome/browser/extensions/api/declarative/rules_registry.cc b/chrome/browser/extensions/api/declarative/rules_registry.cc
index 1e6e5c118203889e6639a4af04f1811806fbd3f4..985195d995dc03d3cf9ba412278161350a3d69ba 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry.cc
+++ b/chrome/browser/extensions/api/declarative/rules_registry.cc
@@ -38,28 +38,6 @@ scoped_ptr<base::Value> RulesToValue(
   return list.PassAs<base::Value>();
 }
 
-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;
-}
-
 std::string ToId(int identifier) {
   return base::StringPrintf("_%d_", identifier);
 }
@@ -74,30 +52,14 @@ namespace extensions {
 RulesRegistry::RulesRegistry(
     Profile* profile,
     const std::string& event_name,
-    content::BrowserThread::ID owner_thread,
-    bool log_storage_init_delay,
-    scoped_ptr<RulesCacheDelegate>* ui_part)
-    : owner_thread_(owner_thread),
+    content::BrowserThread::ID owner_thread)
+    : profile_(profile),
+      owner_thread_(owner_thread),
       event_name_(event_name),

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

event_name_ is never used in RulesRegistry, just the CacheDelegate, so it'd be
nice to store it only in the right place. 
... I think I see a way to do that by calling Init() from the subclasses, but
that's definitely more than you need to do in this CL.

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

More cleanup coming in subsequent CLs. I'd like to keep this simple so I can
make continuous forward progress. :-)

       weak_ptr_factory_(profile ? this : NULL),
-      cache_delegate_(
-          (profile ? (new RulesCacheDelegate(profile,
-                                          event_name,
-                                          owner_thread,
-                                          weak_ptr_factory_.GetWeakPtr(),
-                                          log_storage_init_delay))->GetWeakPtr()
-                   : base::WeakPtr<RulesCacheDelegate>())),
       process_changed_rules_requested_(profile ? NOT_SCHEDULED_FOR_PROCESSING
                                                : NEVER_PROCESS),
       last_generated_rule_identifier_id_(0) {
-  if (!profile) {
-    CHECK(!ui_part);
-    return;
-  }
-
-  ui_part->reset(cache_delegate_.get());
-
-  cache_delegate_->Init();
 }
 
 std::string RulesRegistry::AddRules(
@@ -218,6 +180,15 @@ void RulesRegistry::OnExtensionUnloaded(const std::string& extension_id) {
   used_rule_identifiers_.erase(extension_id);
 }
 
+bool RulesRegistry::SetCacheDelegate(
+    base::WeakPtr<RulesCacheDelegate> cache_delegate) {
+  if (cache_delegate_)
+    return false;
+  cache_delegate_ = cache_delegate;
+  cache_delegate_->Init();
+  return true;
+}
+
 size_t RulesRegistry::GetNumberOfUsedRuleIdentifiersForTesting() const {
   size_t entry_count = 0u;
   for (RuleIdentifiersMap::const_iterator extension =
@@ -245,14 +216,6 @@ void RulesRegistry::MarkReady(base::Time storage_init_time) {
   ready_.Signal();
 }
 
-void RulesRegistry::DeserializeAndAddRules(
-    const std::string& extension_id,
-    scoped_ptr<base::Value> rules) {
-  DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
-
-  AddRules(extension_id, RulesFromValue(rules.get()));
-}
-
 void RulesRegistry::ProcessChangedRules(const std::string& extension_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));