Decouple RulesCacheDelegate from RulesRegistry

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

 // Copyright 2013 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_cache_delegate.h"
 
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/declarative/rules_registry.h"
 #include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/extensions/state_store.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 
 namespace {
 
 // Returns the key to use for storing declarative rules in the state store.
 std::string GetDeclarativeRuleStorageKey(const std::string& event_name,
                                          bool incognito) {
   if (incognito)
     return "declarative_rules.incognito." + event_name;
   else
     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 {
 
 // RulesCacheDelegate
 
 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) {}
 
 RulesCacheDelegate::~RulesCacheDelegate() {}
 
 // Returns the key to use for storing whether the rules have been stored.
 // static
 std::string RulesCacheDelegate::GetRulesStoredKey(const std::string& event_name,
                                                   bool incognito) {
   std::string result(kRulesStoredKey);
@@ skipping 136 matching lines @@
                  extension_id));
 }
 
 void RulesCacheDelegate::ReadFromStorageCallback(
     const std::string& extension_id,
     scoped_ptr<base::Value> value) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   content::BrowserThread::PostTask(
       rules_registry_thread_,
       FROM_HERE,
-      base::Bind(&RulesRegistry::DeserializeAndAddRules,
+      base::Bind(&RulesCacheDelegate::DeserializeAndAddRules,
                  registry_,
                  extension_id,
                  base::Passed(&value)));
 
   waiting_for_extensions_.erase(extension_id);
 
   if (waiting_for_extensions_.empty())
     ExtensionSystem::Get(profile_)->ready().Post(
         FROM_HERE, base::Bind(&RulesCacheDelegate::CheckIfReady, GetWeakPtr()));
 }
@@ skipping 17 matching lines @@
     const std::string& extension_id,
     bool rules_stored) {
   CHECK(profile_);
   ExtensionScopedPrefs* extension_prefs = ExtensionPrefs::Get(profile_);
   extension_prefs->UpdateExtensionPref(
       extension_id,
       rules_stored_key_,
       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