Refactoring how conditions without URL attributes are handled.

chrome/browser/extensions/api/declarative_webrequest/webrequest_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_webrequest/webrequest_rules_registry.h"
 
 #include <limits>
 
 #include "chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api_helpers.h"
 #include "chrome/browser/extensions/api/web_request/web_request_permissions.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "net/url_request/url_request.h"
 
 namespace extensions {
 
 WebRequestRulesRegistry::WebRequestRulesRegistry(Profile* profile,
                                                  Delegate* delegate)
     : RulesRegistryWithCache(delegate) {
   if (profile)
     extension_info_map_ = ExtensionSystem::Get(profile)->info_map();
 }
 
-std::set<WebRequestRule::GlobalRuleId>
+std::set<const WebRequestRule*>
 WebRequestRulesRegistry::GetMatches(
     const WebRequestRule::RequestData& request_data) {
-  std::set<WebRequestRule::GlobalRuleId> result;
+  // 1st phase -- add all rules with some conditions without UrlFilter
+  // attributes.
+  std::set<const WebRequestRule*> result(rules_with_untriggered_conditions_);
 
-  // Figure out for which rules the URL match conditions were fulfilled.
+  // 2nd phase -- add all rules with some conditions triggered by URL matches.
   typedef std::set<URLMatcherConditionSet::ID> URLMatches;
   URLMatches url_matches = url_matcher_.MatchURL(request_data.request->url());
+  for (URLMatches::const_iterator url_match = url_matches.begin();
+       url_match != url_matches.end(); ++url_match) {
+    RuleTriggers::const_iterator rule_trigger = rule_triggers_.find(*url_match);
+    CHECK(rule_trigger != rule_triggers_.end());
+    result.insert(rule_trigger->second);
+  }
 
-  // Then we need to check for each of these, whether the other
-  // WebRequestConditionAttributes are also fulfilled.
-  for (URLMatches::iterator url_match = url_matches.begin();
-       url_match != url_matches.end(); ++url_match) {
-    RuleTriggers::iterator rule_trigger = rule_triggers_.find(*url_match);
-    CHECK(rule_trigger != rule_triggers_.end());
+  // 3rd phase -- eliminate all rules with no satisfied condition (that may
+  // happen due to non-UrlFilter attributes).
+  std::set<const WebRequestRule*>::iterator it = result.begin();
+  while (it != result.end()) {
+    if ((*it)->conditions().IsFulfilled(url_matches, request_data))
+      ++it;
+    else
+      result.erase(it++);
+  }

[battre, 2013/01/07 17:36:27]

I think this can be simplified by just iterating over
rules_with_untriggered_conditions_ and url_matches and only inserting what
matches. I think it may be fewer lines of code and require less ressources.

[Jeffrey Yasskin, 2013/01/07 20:47:41]

Yes, that's cheaper. I had guessed that it would be more lines of code since
you'd want to avoid checking IsFulfilled on rules you'd already checked, but I
could be wrong.

[vabr (Chromium), 2013/01/10 15:12:22]

Done.

 
-    WebRequestRule* rule = rule_trigger->second;
-    if (rule->conditions().IsFulfilled(*url_match, request_data))
-      result.insert(rule->id());
-  }
   return result;
 }
 
 std::list<LinkedPtrEventResponseDelta> WebRequestRulesRegistry::CreateDeltas(
     const ExtensionInfoMap* extension_info_map,
     const WebRequestRule::RequestData& request_data,
     bool crosses_incognito) {
   if (webrequest_rules_.empty())
     return std::list<LinkedPtrEventResponseDelta>();
 
-  std::set<WebRequestRule::GlobalRuleId> matches =
-      GetMatches(request_data);
+  std::set<const WebRequestRule*> matches = GetMatches(request_data);
 
   // Sort all matching rules by their priority so that they can be processed
   // in decreasing order.
   typedef std::pair<WebRequestRule::Priority, WebRequestRule::GlobalRuleId>
       PriorityRuleIdPair;
   std::vector<PriorityRuleIdPair> ordered_matches;
   ordered_matches.reserve(matches.size());
-  for (std::set<WebRequestRule::GlobalRuleId>::iterator i = matches.begin();
+  for (std::set<const WebRequestRule*>::iterator i = matches.begin();
        i != matches.end(); ++i) {
-    RulesMap::const_iterator rule = webrequest_rules_.find(*i);
-    CHECK(rule != webrequest_rules_.end());
-    ordered_matches.push_back(make_pair(rule->second->priority(), *i));
+    ordered_matches.push_back(make_pair((*i)->priority(), (*i)->id()));
   }
   // Sort from rbegin to rend in order to get descending priority order.
   std::sort(ordered_matches.rbegin(), ordered_matches.rend());
 
   // Build a map that maps each extension id to the minimum required priority
   // for rules of that extension. Initially, this priority is -infinite and
   // will be increased when the rules are processed and raise the bar via
   // WebRequestIgnoreRulesActions.
   typedef std::string ExtensionId;
   typedef std::map<ExtensionId, WebRequestRule::Priority> MinPriorities;
@@ skipping 72 matching lines @@
        i != new_webrequest_rules.end(); ++i) {
     URLMatcherConditionSet::Vector url_condition_sets;
     const WebRequestConditionSet& conditions = i->second->conditions();
     conditions.GetURLMatcherConditionSets(&url_condition_sets);
     for (URLMatcherConditionSet::Vector::iterator j =
          url_condition_sets.begin(); j != url_condition_sets.end(); ++j) {
       rule_triggers_[(*j)->id()] = i->second.get();
     }
   }
 
-  // Register url patterns in url_matcher_.
+  // Register url patterns in |url_matcher_| and
+  // |rules_with_untriggered_conditions_|.
   URLMatcherConditionSet::Vector all_new_condition_sets;
   for (RulesMap::iterator i = new_webrequest_rules.begin();
        i != new_webrequest_rules.end(); ++i) {
     i->second->conditions().GetURLMatcherConditionSets(&all_new_condition_sets);
+    if (i->second->conditions().HasConditionsWithoutUrls())
+      rules_with_untriggered_conditions_.insert(i->second.get());
   }
   url_matcher_.AddConditionSets(all_new_condition_sets);
 
   ClearCacheOnNavigation();
 
   return "";
 }
 
 std::string WebRequestRulesRegistry::RemoveRulesImpl(
     const std::string& extension_id,
@@ skipping 13 matching lines @@
     // Remove all triggers but collect their IDs.
     URLMatcherConditionSet::Vector condition_sets;
     WebRequestRule* rule = webrequest_rules_entry->second.get();
     rule->conditions().GetURLMatcherConditionSets(&condition_sets);
     for (URLMatcherConditionSet::Vector::iterator j = condition_sets.begin();
          j != condition_sets.end(); ++j) {
       remove_from_url_matcher.push_back((*j)->id());
       rule_triggers_.erase((*j)->id());
     }
 
-    // Remove reference to actual rule.
+    rules_with_untriggered_conditions_.erase(rule);
+
+    // Removes the owning references to (and thus deletes) the rule.
     webrequest_rules_.erase(webrequest_rules_entry);
   }
 
   // Clear URLMatcher based on condition_set_ids that are not needed any more.
   url_matcher_.RemoveConditionSets(remove_from_url_matcher);
 
   ClearCacheOnNavigation();
 
   return "";
 }
@@ skipping 31 matching lines @@
     return base::Time();
 
   return extension_info_map_->GetInstallTime(extension_id);
 }
 
 void WebRequestRulesRegistry::ClearCacheOnNavigation() {
   extension_web_request_api_helpers::ClearCacheOnNavigation();
 }
 
 }  // namespace extensions