Decouple RulesCacheDelegate from RulesRegistry

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

 // Copyright (c) 2012 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 "chrome/browser/extensions/api/declarative/rules_registry.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
@@ skipping 20 matching lines @@
 const char kDuplicateRuleId[] = "Duplicate rule ID: %s";
 
 scoped_ptr<base::Value> RulesToValue(
     const std::vector<linked_ptr<extensions::RulesRegistry::Rule> >& rules) {
   scoped_ptr<base::ListValue> list(new base::ListValue());
   for (size_t i = 0; i < rules.size(); ++i)
     list->Append(rules[i]->ToValue().release());
   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);
 }
 
 }  // namespace
 
 
 namespace extensions {
 
 // RulesRegistry
 
 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(
     const std::string& extension_id,
     const std::vector<linked_ptr<Rule> >& rules) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
 
   std::string error = CheckAndFillInOptionalRules(extension_id, rules);
   if (!error.empty())
     return error;
@@ skipping 100 matching lines @@
 }
 
 void RulesRegistry::OnExtensionUnloaded(const std::string& extension_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
   std::string error = RemoveAllRules(extension_id);
   if (!error.empty())
     LOG(ERROR) << error;
   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 =
            used_rule_identifiers_.begin();
        extension != used_rule_identifiers_.end();
        ++extension) {
     // Each extension is counted as 1 just for being there. Otherwise we miss
     // keys with empty values.
     entry_count += 1u + extension->second.size();
   }
   return entry_count;
 }
 
 RulesRegistry::~RulesRegistry() {
 }
 
 void RulesRegistry::MarkReady(base::Time storage_init_time) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
 
   if (!storage_init_time.is_null()) {
     UMA_HISTOGRAM_TIMES("Extensions.DeclarativeRulesStorageInitialization",
                         base::Time::Now() - 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()));
 
   process_changed_rules_requested_ = NOT_SCHEDULED_FOR_PROCESSING;
 
   std::vector<linked_ptr<RulesRegistry::Rule> > new_rules;
   std::string error = GetAllRules(extension_id, &new_rules);
   DCHECK_EQ(std::string(), error);
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
@@ skipping 79 matching lines @@
   for (i = identifiers.begin(); i != identifiers.end(); ++i)
     used_rule_identifiers_[extension_id].erase(*i);
 }
 
 void RulesRegistry::RemoveAllUsedRuleIdentifiers(
     const std::string& extension_id) {
   used_rule_identifiers_.erase(extension_id);
 }
 
 }  // namespace extensions