Refactoring how conditions without URL attributes are handled.

chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc

Index: chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc
diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc
index 1ece019e6a397c6c4e7e31d0ecc7cc457b17551c..7428e49a54ac1300398d0a7a37d0fc249d337c50 100644
--- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc
+++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_condition.cc
@@ -49,7 +49,6 @@ WebRequestCondition::WebRequestCondition(
     : url_matcher_conditions_(url_matcher_conditions),
       condition_attributes_(condition_attributes),
       applicable_request_stages_(~0) {
-  CHECK(url_matcher_conditions.get());
   for (WebRequestConditionAttributes::const_iterator i =
        condition_attributes_.begin(); i != condition_attributes_.end(); ++i) {
     applicable_request_stages_ &= (*i)->GetStages();
@@ -74,6 +73,13 @@ bool WebRequestCondition::IsFulfilled(
   return true;
 }
 
+bool WebRequestCondition::IsFulfilledIndependentlyOfURL(
+    const WebRequestRule::RequestData& request_data) const {
+  if (url_matcher_conditions_.get())
+    return false;
+  return IsFulfilled(request_data);
+}
+
 // static
 scoped_ptr<WebRequestCondition> WebRequestCondition::Create(
     URLMatcherConditionFactory* url_matcher_condition_factory,
@@ -132,13 +138,6 @@ scoped_ptr<WebRequestCondition> WebRequestCondition::Create(
       return scoped_ptr<WebRequestCondition>(NULL);
   }
 
-  if (!url_matcher_condition_set) {
-    URLMatcherConditionSet::Conditions url_matcher_conditions;
-    url_matcher_conditions.insert(
-        url_matcher_condition_factory->CreateHostPrefixCondition(""));
-    url_matcher_condition_set =
-        new URLMatcherConditionSet(++g_next_id, url_matcher_conditions);
-  }
   scoped_ptr<WebRequestCondition> result(
       new WebRequestCondition(url_matcher_condition_set, attributes));
 
@@ -150,38 +149,63 @@ scoped_ptr<WebRequestCondition> WebRequestCondition::Create(
   return result.Pass();
 }
 
-
 //
 // WebRequestConditionSet
 //
 
+namespace {
+
+// Returns whether there is a condition among |conditions| which has no
+// URLMatcherConditionSet assigned.
+bool HasConditionWithoutUrlMatcher(
+    const WebRequestConditionSet::Conditions& conditions) {
+  for (WebRequestConditionSet::Conditions::const_iterator i =
+           conditions.begin();
+       i != conditions.end(); ++i) {
+    if (!(*i)->url_matcher_condition_set().get())
+      return true;
+  }
+  return false;
+}
+
+}  // namespace
+
 WebRequestConditionSet::WebRequestConditionSet(
     const WebRequestConditionSet::Conditions& conditions)
-    : conditions_(conditions) {
-  for (Conditions::iterator i = conditions_.begin(); i != conditions_.end();
-       ++i) {
-    URLMatcherConditionSet::ID trigger_id =
-        (*i)->url_matcher_condition_set_id();
-    match_triggers_[trigger_id] = i->get();
-  }
+    : conditions_(conditions),
+      has_conditions_without_urls_(HasConditionWithoutUrlMatcher(conditions)) {
 }
 
 WebRequestConditionSet::~WebRequestConditionSet() {}
 
 bool WebRequestConditionSet::IsFulfilled(
-    URLMatcherConditionSet::ID url_match,
+    const std::set<URLMatcherConditionSet::ID>& url_matches,
     const WebRequestRule::RequestData& request_data) const {
-  MatchTriggers::const_iterator trigger = match_triggers_.find(url_match);
-  DCHECK(trigger != match_triggers_.end());
-  DCHECK_EQ(url_match, trigger->second->url_matcher_condition_set_id());
-  return trigger->second->IsFulfilled(request_data);
+  for (Conditions::const_iterator i = conditions_.begin();
+       i != conditions_.end(); ++i) {
+    const URLMatcherConditionSet* set = (*i)->url_matcher_condition_set().get();

[Jeffrey Yasskin, 2012/12/18 21:56:02]

Can we put this logic inside Condition::IsFulfilled()? That is, you'd always
pass a url_matches set into Condition::IsFulfilled(), it would check whatever it
needs to on that set<ID> and RequestData, and IsFulfilledIndependentlyOfURL
would go away.

This will make my templatizing change easier since ConditionSet::IsFulfilled
would go back to being a simple loop.

[vabr (Chromium), 2012/12/19 08:38:19]

Great idea, thanks!
Done.

+    if (set != NULL) {
+      // This condition has a UrlFilter attribute.
+      if (url_matches.find(set->id()) != url_matches.end() &&
+          (*i)->IsFulfilled(request_data))
+        return true;
+    } else {
+      // This condition has no UrlFilter attribute.
+      if ((*i)->IsFulfilledIndependentlyOfURL(request_data))
+        return true;
+    }
+  }
+  return false;
 }
 
 void WebRequestConditionSet::GetURLMatcherConditionSets(
     URLMatcherConditionSet::Vector* condition_sets) const {
   for (Conditions::const_iterator i = conditions_.begin();
        i != conditions_.end(); ++i) {
-    condition_sets->push_back((*i)->url_matcher_condition_set());
+    scoped_refptr<URLMatcherConditionSet> set =
+        (*i)->url_matcher_condition_set();
+    if (set.get())
+      condition_sets->push_back(set);
   }
 }