Refactor RulesRegistryWithCache to 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_with_cache.cc b/chrome/browser/extensions/api/declarative/rules_registry.cc
similarity index 42%
rename from chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc
rename to chrome/browser/extensions/api/declarative/rules_registry.cc
index a452130fb03c26eacd4b2a2c1590c28fdd0c1538..1e6e5c118203889e6639a4af04f1811806fbd3f4 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc
+++ b/chrome/browser/extensions/api/declarative/rules_registry.cc
@@ -2,7 +2,7 @@
 // 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_with_cache.h"
+#include "chrome/browser/extensions/api/declarative/rules_registry.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ -12,6 +12,7 @@
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/extensions/api/declarative/rules_cache_delegate.h"
 #include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ -59,13 +60,8 @@ std::vector<linked_ptr<extensions::RulesRegistry::Rule> > RulesFromValue(
   return rules;
 }
 
-// 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::string ToId(int identifier) {
+  return base::StringPrintf("_%d_", identifier);
 }
 
 }  // namespace
@@ -73,40 +69,47 @@ std::string GetDeclarativeRuleStorageKey(const std::string& event_name,
 
 namespace extensions {
 
-// RulesRegistryWithCache
+// RulesRegistry
 
-RulesRegistryWithCache::RulesRegistryWithCache(
+RulesRegistry::RulesRegistry(
     Profile* profile,
     const std::string& event_name,
     content::BrowserThread::ID owner_thread,
     bool log_storage_init_delay,
-    scoped_ptr<RuleStorageOnUI>* ui_part)
-    : RulesRegistry(owner_thread, event_name),
+    scoped_ptr<RulesCacheDelegate>* ui_part)
+    : owner_thread_(owner_thread),
+      event_name_(event_name),
       weak_ptr_factory_(profile ? this : NULL),
-      storage_on_ui_(
-          (profile ? (new RuleStorageOnUI(profile,
+      cache_delegate_(
+          (profile ? (new RulesCacheDelegate(profile,
                                           event_name,
                                           owner_thread,
                                           weak_ptr_factory_.GetWeakPtr(),
                                           log_storage_init_delay))->GetWeakPtr()
-                   : base::WeakPtr<RuleStorageOnUI>())),
+                   : base::WeakPtr<RulesCacheDelegate>())),
       process_changed_rules_requested_(profile ? NOT_SCHEDULED_FOR_PROCESSING
-                                               : NEVER_PROCESS) {
+                                               : NEVER_PROCESS),
+      last_generated_rule_identifier_id_(0) {
   if (!profile) {
     CHECK(!ui_part);
     return;
   }
 
-  ui_part->reset(storage_on_ui_.get());
+  ui_part->reset(cache_delegate_.get());
 
-  storage_on_ui_->Init();
+  cache_delegate_->Init();
 }
 
-std::string RulesRegistryWithCache::AddRules(
+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;
+  FillInOptionalPriorities(rules);
+
   // Verify that all rule IDs are new.
   for (std::vector<linked_ptr<Rule> >::const_iterator i =
       rules.begin(); i != rules.end(); ++i) {
@@ -116,7 +119,7 @@ std::string RulesRegistryWithCache::AddRules(
       return base::StringPrintf(kDuplicateRuleId, rule_id.c_str());
   }
 
-  std::string error = AddRulesImpl(extension_id, rules);
+  error = AddRulesImpl(extension_id, rules);
 
   if (!error.empty())
     return error;
@@ -133,7 +136,7 @@ std::string RulesRegistryWithCache::AddRules(
   return kSuccess;
 }
 
-std::string RulesRegistryWithCache::RemoveRules(
+std::string RulesRegistry::RemoveRules(
     const std::string& extension_id,
     const std::vector<std::string>& rule_identifiers) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
@@ -151,11 +154,11 @@ std::string RulesRegistryWithCache::RemoveRules(
   }
 
   MaybeProcessChangedRules(extension_id);
+  RemoveUsedRuleIdentifiers(extension_id, rule_identifiers);
   return kSuccess;
 }
 
-std::string RulesRegistryWithCache::RemoveAllRules(
-    const std::string& extension_id) {
+std::string RulesRegistry::RemoveAllRules(const std::string& extension_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
 
   std::string error = RemoveAllRulesImpl(extension_id);
@@ -173,10 +176,11 @@ std::string RulesRegistryWithCache::RemoveAllRules(
   }
 
   MaybeProcessChangedRules(extension_id);
+  RemoveAllUsedRuleIdentifiers(extension_id);
   return kSuccess;
 }
 
-std::string RulesRegistryWithCache::GetRules(
+std::string RulesRegistry::GetRules(
     const std::string& extension_id,
     const std::vector<std::string>& rule_identifiers,
     std::vector<linked_ptr<RulesRegistry::Rule> >* out) {
@@ -192,7 +196,7 @@ std::string RulesRegistryWithCache::GetRules(
   return kSuccess;
 }
 
-std::string RulesRegistryWithCache::GetAllRules(
+std::string RulesRegistry::GetAllRules(
     const std::string& extension_id,
     std::vector<linked_ptr<RulesRegistry::Rule> >* out) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
@@ -206,18 +210,31 @@ std::string RulesRegistryWithCache::GetAllRules(
   return kSuccess;
 }
 
-void RulesRegistryWithCache::OnExtensionUnloaded(
-    const std::string& extension_id) {
+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);
+}
+
+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;
 }
 
-RulesRegistryWithCache::~RulesRegistryWithCache() {
+RulesRegistry::~RulesRegistry() {
 }
 
-void RulesRegistryWithCache::MarkReady(base::Time storage_init_time) {
+void RulesRegistry::MarkReady(base::Time storage_init_time) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
 
   if (!storage_init_time.is_null()) {
@@ -228,7 +245,7 @@ void RulesRegistryWithCache::MarkReady(base::Time storage_init_time) {
   ready_.Signal();
 }
 
-void RulesRegistryWithCache::DeserializeAndAddRules(
+void RulesRegistry::DeserializeAndAddRules(
     const std::string& extension_id,
     scoped_ptr<base::Value> rules) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
@@ -236,8 +253,7 @@ void RulesRegistryWithCache::DeserializeAndAddRules(
   AddRules(extension_id, RulesFromValue(rules.get()));
 }
 
-void RulesRegistryWithCache::ProcessChangedRules(
-    const std::string& extension_id) {
+void RulesRegistry::ProcessChangedRules(const std::string& extension_id) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
 
   process_changed_rules_requested_ = NOT_SCHEDULED_FOR_PROCESSING;
@@ -248,237 +264,91 @@ void RulesRegistryWithCache::ProcessChangedRules(
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
-      base::Bind(&RuleStorageOnUI::WriteToStorage,
-                 storage_on_ui_,
+      base::Bind(&RulesCacheDelegate::WriteToStorage,
+                 cache_delegate_,
                  extension_id,
                  base::Passed(RulesToValue(new_rules))));
 }
 
-void RulesRegistryWithCache::MaybeProcessChangedRules(
-    const std::string& extension_id) {
+void RulesRegistry::MaybeProcessChangedRules(const std::string& extension_id) {
   if (process_changed_rules_requested_ != NOT_SCHEDULED_FOR_PROCESSING)
     return;
 
   process_changed_rules_requested_ = SCHEDULED_FOR_PROCESSING;
   ready_.Post(FROM_HERE,
-              base::Bind(&RulesRegistryWithCache::ProcessChangedRules,
+              base::Bind(&RulesRegistry::ProcessChangedRules,
                          weak_ptr_factory_.GetWeakPtr(),
                          extension_id));
 }
 
-// RulesRegistryWithCache::RuleStorageOnUI
-
-const char RulesRegistryWithCache::RuleStorageOnUI::kRulesStoredKey[] =
-    "has_declarative_rules";
-
-RulesRegistryWithCache::RuleStorageOnUI::RuleStorageOnUI(
-    Profile* profile,
-    const std::string& event_name,
-    content::BrowserThread::ID rules_registry_thread,
-    base::WeakPtr<RulesRegistryWithCache> registry,
-    bool log_storage_init_delay)
-    : profile_(profile),
-      storage_key_(GetDeclarativeRuleStorageKey(event_name,
-                                                profile->IsOffTheRecord())),
-      rules_stored_key_(GetRulesStoredKey(event_name,
-                                          profile->IsOffTheRecord())),
-      log_storage_init_delay_(log_storage_init_delay),
-      registry_(registry),
-      rules_registry_thread_(rules_registry_thread),
-      notified_registry_(false),
-      weak_ptr_factory_(this) {}
-
-RulesRegistryWithCache::RuleStorageOnUI::~RuleStorageOnUI() {}
-
-// Returns the key to use for storing whether the rules have been stored.
-// static
-std::string RulesRegistryWithCache::RuleStorageOnUI::GetRulesStoredKey(
-    const std::string& event_name,
-    bool incognito) {
-  std::string result(kRulesStoredKey);
-  result += incognito ? ".incognito." : ".";
-  return result + event_name;
+bool RulesRegistry::IsUniqueId(const std::string& extension_id,
+                               const std::string& rule_id) const {
+  RuleIdentifiersMap::const_iterator identifiers =
+      used_rule_identifiers_.find(extension_id);
+  if (identifiers == used_rule_identifiers_.end())
+    return true;
+  return identifiers->second.find(rule_id) == identifiers->second.end();
 }
 
-// This is called from the constructor of RulesRegistryWithCache, so it is
-// important that it both
-// 1. calls no (in particular virtual) methods of the rules registry, and
-// 2. does not create scoped_refptr holding the registry. (A short-lived
-// scoped_refptr might delete the rules registry before it is constructed.)
-void RulesRegistryWithCache::RuleStorageOnUI::Init() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
-  ExtensionSystem& system = *ExtensionSystem::Get(profile_);
-  extensions::StateStore* store = system.rules_store();
-  if (store)
-    store->RegisterKey(storage_key_);
-
-  registrar_.Add(this,
-                 chrome::NOTIFICATION_EXTENSION_LOADED,
-                 content::Source<Profile>(profile_->GetOriginalProfile()));
-
-  if (profile_->IsOffTheRecord())
-    log_storage_init_delay_ = false;
-
-  system.ready().Post(
-      FROM_HERE,
-      base::Bind(&RuleStorageOnUI::ReadRulesForInstalledExtensions,
-                 GetWeakPtr()));
-  system.ready().Post(FROM_HERE,
-                      base::Bind(&RuleStorageOnUI::CheckIfReady, GetWeakPtr()));
+std::string RulesRegistry::GenerateUniqueId(const std::string& extension_id) {
+  while (!IsUniqueId(extension_id, ToId(last_generated_rule_identifier_id_)))
+    ++last_generated_rule_identifier_id_;
+  return ToId(last_generated_rule_identifier_id_);
 }
 
-void RulesRegistryWithCache::RuleStorageOnUI::WriteToStorage(
+std::string RulesRegistry::CheckAndFillInOptionalRules(
     const std::string& extension_id,
-    scoped_ptr<base::Value> value) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  if (!profile_)
-    return;
+    const std::vector<linked_ptr<RulesRegistry::Rule> >& rules) {
+  // IDs we have inserted, in case we need to rollback this operation.
+  std::vector<std::string> rollback_log;
 
-  const base::ListValue* rules = NULL;
-  CHECK(value->GetAsList(&rules));
-  bool rules_stored_previously = GetDeclarativeRulesStored(extension_id);
-  bool store_rules = !rules->empty();
-  SetDeclarativeRulesStored(extension_id, store_rules);
-  if (!rules_stored_previously && !store_rules)
-    return;
-
-  StateStore* store = ExtensionSystem::Get(profile_)->rules_store();
-  if (store)
-    store->SetExtensionValue(extension_id, storage_key_, value.Pass());
-}
-
-void RulesRegistryWithCache::RuleStorageOnUI::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  DCHECK(type == chrome::NOTIFICATION_EXTENSION_LOADED);
-
-  const extensions::Extension* extension =
-      content::Details<const extensions::Extension>(details).ptr();
-  // TODO(mpcomplete): This API check should generalize to any use of
-  // declarative rules, not just webRequest.
-  if (extension->HasAPIPermission(APIPermission::kDeclarativeContent) ||
-      extension->HasAPIPermission(APIPermission::kDeclarativeWebRequest)) {
-    ExtensionInfoMap* extension_info_map =
-        ExtensionSystem::Get(profile_)->info_map();
-    if (profile_->IsOffTheRecord() &&
-        !extension_info_map->IsIncognitoEnabled(extension->id())) {
-      // Ignore this extension.
-    } else {
-      ReadFromStorage(extension->id());
+  // First we insert all rules with existing identifier, so that generated
+  // identifiers cannot collide with identifiers passed by the caller.
+  for (std::vector<linked_ptr<RulesRegistry::Rule> >::const_iterator i =
+      rules.begin(); i != rules.end(); ++i) {
+    RulesRegistry::Rule* rule = i->get();
+    if (rule->id.get()) {
+      std::string id = *(rule->id);
+      if (!IsUniqueId(extension_id, id)) {
+        RemoveUsedRuleIdentifiers(extension_id, rollback_log);
+        return "Id " + id + " was used multiple times.";
+      }
+      used_rule_identifiers_[extension_id].insert(id);
     }
   }
-}
-
-void RulesRegistryWithCache::RuleStorageOnUI::CheckIfReady() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  if (notified_registry_ || !waiting_for_extensions_.empty())
-    return;
-
-  content::BrowserThread::PostTask(
-      rules_registry_thread_,
-      FROM_HERE,
-      base::Bind(
-          &RulesRegistryWithCache::MarkReady, registry_, storage_init_time_));
-  notified_registry_ = true;
-}
-
-void
-RulesRegistryWithCache::RuleStorageOnUI::ReadRulesForInstalledExtensions() {
-  ExtensionSystem& system = *ExtensionSystem::Get(profile_);
-  ExtensionService* extension_service = system.extension_service();
-  DCHECK(extension_service);
-  // In an OTR profile, we start on top of a normal profile already, so the
-  // extension service should be ready.
-  DCHECK(!profile_->IsOffTheRecord() || extension_service->is_ready());
-  if (extension_service->is_ready()) {
-    const ExtensionSet* extensions = extension_service->extensions();
-    for (ExtensionSet::const_iterator i = extensions->begin();
-         i != extensions->end();
-         ++i) {
-      bool needs_apis_storing_rules =
-          (*i)->HasAPIPermission(APIPermission::kDeclarativeContent) ||
-          (*i)->HasAPIPermission(APIPermission::kDeclarativeWebRequest);
-      bool respects_off_the_record =
-          !(profile_->IsOffTheRecord()) ||
-          extension_util::IsIncognitoEnabled((*i)->id(), extension_service);
-      if (needs_apis_storing_rules && respects_off_the_record)
-        ReadFromStorage((*i)->id());
+  // Now we generate IDs in case they were not specified in the rules. This
+  // cannot fail so we do not need to keep track of a rollback log.
+  for (std::vector<linked_ptr<RulesRegistry::Rule> >::const_iterator i =
+      rules.begin(); i != rules.end(); ++i) {
+    RulesRegistry::Rule* rule = i->get();
+    if (!rule->id.get()) {
+      rule->id.reset(new std::string(GenerateUniqueId(extension_id)));
+      used_rule_identifiers_[extension_id].insert(*(rule->id));
     }
   }
+  return std::string();
 }
 
-void RulesRegistryWithCache::RuleStorageOnUI::ReadFromStorage(
-    const std::string& extension_id) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  if (!profile_)
-    return;
-
-  if (log_storage_init_delay_ && storage_init_time_.is_null())
-    storage_init_time_ = base::Time::Now();
-
-  if (!GetDeclarativeRulesStored(extension_id)) {
-    ExtensionSystem::Get(profile_)->ready().Post(
-        FROM_HERE, base::Bind(&RuleStorageOnUI::CheckIfReady, GetWeakPtr()));
-    return;
+void RulesRegistry::FillInOptionalPriorities(
+    const std::vector<linked_ptr<RulesRegistry::Rule> >& rules) {
+  std::vector<linked_ptr<RulesRegistry::Rule> >::const_iterator i;
+  for (i = rules.begin(); i != rules.end(); ++i) {
+    if (!(*i)->priority.get())
+      (*i)->priority.reset(new int(DEFAULT_PRIORITY));
   }
-
-  extensions::StateStore* store = ExtensionSystem::Get(profile_)->rules_store();
-  if (!store)
-    return;
-  waiting_for_extensions_.insert(extension_id);
-  store->GetExtensionValue(extension_id,
-                           storage_key_,
-                           base::Bind(&RuleStorageOnUI::ReadFromStorageCallback,
-                                      weak_ptr_factory_.GetWeakPtr(),
-                                      extension_id));
 }
 
-void RulesRegistryWithCache::RuleStorageOnUI::ReadFromStorageCallback(
+void RulesRegistry::RemoveUsedRuleIdentifiers(
     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(&RulesRegistryWithCache::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(&RuleStorageOnUI::CheckIfReady, GetWeakPtr()));
+    const std::vector<std::string>& identifiers) {
+  std::vector<std::string>::const_iterator i;
+  for (i = identifiers.begin(); i != identifiers.end(); ++i)
+    used_rule_identifiers_[extension_id].erase(*i);
 }
 
-bool RulesRegistryWithCache::RuleStorageOnUI::GetDeclarativeRulesStored(
-    const std::string& extension_id) const {
-  CHECK(profile_);
-  const ExtensionScopedPrefs* extension_prefs = ExtensionPrefs::Get(profile_);
-
-  bool rules_stored = true;
-  if (extension_prefs->ReadPrefAsBoolean(
-          extension_id, rules_stored_key_, &rules_stored))
-    return rules_stored;
-
-  // Safe default -- if we don't know that the rules are not stored, we force
-  // a read by returning true.
-  return true;
-}
-
-void RulesRegistryWithCache::RuleStorageOnUI::SetDeclarativeRulesStored(
-    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));
+void RulesRegistry::RemoveAllUsedRuleIdentifiers(
+    const std::string& extension_id) {
+  used_rule_identifiers_.erase(extension_id);
 }
 
 }  // namespace extensions