Rules Registry Optimization: Don't fill in rules pulled from storage

As a follow-up to a previous refactoring, http://crrev.com/49693003, this
CL avoids initializing rules that are pulled from storage as they are already
initialized. This should address concerns about a potential performance regression.

BUG=312461

Review URL: https://codereview.chromium.org/61763021

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@235203 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 30 insertions, 10 deletions

diff --git a/chrome/browser/extensions/api/declarative/rules_registry.cc b/chrome/browser/extensions/api/declarative/rules_registry.cc
index 0fb789e..e8b2174 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry.cc
+++ b/chrome/browser/extensions/api/declarative/rules_registry.cc
@@ -95,26 +95,23 @@ RulesRegistry::RulesRegistry(
   }
 }
 
-std::string RulesRegistry::AddRules(
+std::string RulesRegistry::AddRulesNoFill(
     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) {
     const RuleId& rule_id = *((*i)->id);
+    // Every rule should have a priority assigned.
+    DCHECK((*i)->priority);
     RulesDictionaryKey key(extension_id, rule_id);
     if (rules_.find(key) != rules_.end())
       return base::StringPrintf(kDuplicateRuleId, rule_id.c_str());
   }
 
-  error = AddRulesImpl(extension_id, rules);
+  std::string error = AddRulesImpl(extension_id, rules);
 
   if (!error.empty())
     return error;
@@ -131,6 +128,19 @@ std::string RulesRegistry::AddRules(
   return kSuccess;
 }
 
+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);
+
+  return AddRulesNoFill(extension_id, rules);
+}
+
 std::string RulesRegistry::RemoveRules(
     const std::string& extension_id,
     const std::vector<std::string>& rule_identifiers) {
@@ -231,7 +241,7 @@ void RulesRegistry::DeserializeAndAddRules(
     scoped_ptr<base::Value> rules) {
   DCHECK(content::BrowserThread::CurrentlyOn(owner_thread()));
 
-  AddRules(extension_id, RulesFromValue(rules.get()));
+  AddRulesNoFill(extension_id, RulesFromValue(rules.get()));
 }
 
 RulesRegistry::~RulesRegistry() {
diff --git a/chrome/browser/extensions/api/declarative/rules_registry.h b/chrome/browser/extensions/api/declarative/rules_registry.h
index ae16269..dc03c73 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry.h
+++ b/chrome/browser/extensions/api/declarative/rules_registry.h
@@ -157,6 +157,17 @@ class RulesRegistry : public base::RefCountedThreadSafe<RulesRegistry> {
  protected:
   virtual ~RulesRegistry();
 
+  // The precondition for calling this method is that all rules have unique IDs.
+  // AddRules establishes this precondition and calls into this method.
+  // Stored rules already meet this precondition and so they avoid calling
+  // CheckAndFillInOptionalRules for improved performance.
+  //
+  // Returns an empty string if the function is successful or an error
+  // message otherwise.
+  std::string AddRulesNoFill(
+      const std::string& extension_id,
+      const std::vector<linked_ptr<RulesRegistry::Rule> >& rules);
+
   // These functions need to apply the rules to the browser, while the base
   // class will handle defaulting empty fields before calling *Impl, and will
   // automatically cache the rules and re-call *Impl on browser startup.
diff --git a/chrome/browser/extensions/api/declarative/rules_registry_service.h b/chrome/browser/extensions/api/declarative/rules_registry_service.h
index 461377d..af4ce2b 100644
--- a/chrome/browser/extensions/api/declarative/rules_registry_service.h
+++ b/chrome/browser/extensions/api/declarative/rules_registry_service.h
@@ -67,8 +67,7 @@ class RulesRegistryService : public ProfileKeyedAPI,
   static RulesRegistryService* Get(Profile* profile);
 
   // Registers the default RulesRegistries used in Chromium.
-  void EnsureDefaultRulesRegistriesRegistered(
-      const WebViewKey& webview_key);
+  void EnsureDefaultRulesRegistriesRegistered(const WebViewKey& webview_key);
 
   // Registers a RulesRegistry and wraps it in an InitializingRulesRegistry.
   void RegisterRulesRegistry(scoped_refptr<RulesRegistry> rule_registry);