Declarative Web Request: firstPartyForCookiesUrl

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

Index: chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc
diff --git a/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc b/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc
index 0e90c2d22ddd6a2d92a6302d2a31009b4f48be5d..ca3aafd0288b09a04807e2ae4e4a3e69955cc157 100644
--- a/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc
+++ b/chrome/browser/extensions/api/declarative_webrequest/webrequest_rules_registry.cc
@@ -37,7 +37,11 @@ WebRequestRulesRegistry::GetMatches(
   typedef std::set<URLMatcherConditionSet::ID> URLMatches;
 
   RuleSet result;
+
   URLMatches url_matches = url_matcher_.MatchURL(request_data.request->url());
+  URLMatches first_party = first_party_url_matcher_.MatchURL(
+      request_data.request->first_party_for_cookies());
+  url_matches.insert(first_party.begin(), first_party.end());

[vabr (Chromium), 2013/01/21 10:57:48]

This means that we first check all conditions triggered through a matching 'url'
attribute, then all conditions triggered through a matching
'firstPartyForCookiesUrl' attribute. For conditions which have both, but also
some non-URL attribute which is not satisfied, this means those are being
checked twice.

Is this a bearable performance burden, or should I try to implement some result
caching Condition::IsFulfilled? Note that the cache would need to check that
request_data did not change, which might be comparably expensive to checking one
or a couple of non-URL attributes.

[battre, 2013/01/21 17:32:08]

I think we are good here with the current approach.

 
   // 1st phase -- add all rules with some conditions without UrlFilter
   // attributes.
@@ -136,18 +140,22 @@ std::string WebRequestRulesRegistry::AddRulesImpl(
   std::string error;
   RulesMap new_webrequest_rules;
 
+  std::vector<URLMatcherConditionFactory*> factories;
+  factories.push_back(url_matcher_.condition_factory());
+  factories.push_back(first_party_url_matcher_.condition_factory());
+
   for (std::vector<linked_ptr<RulesRegistry::Rule> >::const_iterator rule =
        rules.begin(); rule != rules.end(); ++rule) {
     WebRequestRule::GlobalRuleId rule_id(extension_id, *(*rule)->id);
     DCHECK(webrequest_rules_.find(rule_id) == webrequest_rules_.end());
 
     scoped_ptr<WebRequestRule> webrequest_rule(
-        WebRequestRule::Create(url_matcher_.condition_factory(), extension_id,
-                               extension_installation_time, *rule,
-                               &CheckConsistency, &error));
+        WebRequestRule::Create(factories,
+                               extension_id, extension_installation_time,
+                               *rule, &CheckConsistency, &error));
     if (!error.empty()) {
       // We don't return here, because we want to clear temporary
-      // condition sets in the url_matcher_.
+      // condition sets in |url_matcher_| and |first_party_url_matcher_|.
       break;
     }
 
@@ -157,6 +165,7 @@ std::string WebRequestRulesRegistry::AddRulesImpl(
   if (!error.empty()) {
     // Clean up temporary condition sets created during rule creation.
     url_matcher_.ClearUnusedConditionSets();
+    first_party_url_matcher_.ClearUnusedConditionSets();
     return error;
   }
 
@@ -169,23 +178,29 @@ std::string WebRequestRulesRegistry::AddRulesImpl(
        i != new_webrequest_rules.end(); ++i) {
     URLMatcherConditionSet::Vector url_condition_sets;
     const WebRequestConditionSet& conditions = i->second->conditions();
-    conditions.GetURLMatcherConditionSets(&url_condition_sets);
+    conditions.GetURLMatcherConditionSets(&url_condition_sets, 0);
+    conditions.GetURLMatcherConditionSets(&url_condition_sets, 1);
     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_| and
+  // Register url patterns with both URL matchers and
   // |rules_with_untriggered_conditions_|.
   URLMatcherConditionSet::Vector all_new_condition_sets;
+  URLMatcherConditionSet::Vector all_new_condition_sets_first_party;
   for (RulesMap::iterator i = new_webrequest_rules.begin();
        i != new_webrequest_rules.end(); ++i) {
-    i->second->conditions().GetURLMatcherConditionSets(&all_new_condition_sets);
+    i->second->conditions().GetURLMatcherConditionSets(
+        &all_new_condition_sets, 0);
+    i->second->conditions().GetURLMatcherConditionSets(
+        &all_new_condition_sets_first_party, 1);
     if (i->second->conditions().HasConditionsWithoutUrls())
       rules_with_untriggered_conditions_.insert(i->second.get());
   }
   url_matcher_.AddConditionSets(all_new_condition_sets);
+  first_party_url_matcher_.AddConditionSets(all_new_condition_sets_first_party);
 
   ClearCacheOnNavigation();
 
@@ -195,8 +210,9 @@ std::string WebRequestRulesRegistry::AddRulesImpl(
 std::string WebRequestRulesRegistry::RemoveRulesImpl(
     const std::string& extension_id,
     const std::vector<std::string>& rule_identifiers) {
-  // URLMatcherConditionSet IDs that can be removed from URLMatcher.
+  // URLMatcherConditionSet IDs that can be removed from URL matchers.
   std::vector<URLMatcherConditionSet::ID> remove_from_url_matcher;
+  std::vector<URLMatcherConditionSet::ID> remove_from_first_party_url_matcher;
 
   for (std::vector<std::string>::const_iterator i = rule_identifiers.begin();
        i != rule_identifiers.end(); ++i) {
@@ -209,13 +225,22 @@ std::string WebRequestRulesRegistry::RemoveRulesImpl(
 
     // Remove all triggers but collect their IDs.
     URLMatcherConditionSet::Vector condition_sets;
+    URLMatcherConditionSet::Vector condition_sets_first_party;
     WebRequestRule* rule = webrequest_rules_entry->second.get();
-    rule->conditions().GetURLMatcherConditionSets(&condition_sets);
+    rule->conditions().GetURLMatcherConditionSets(&condition_sets, 0);
+    rule->conditions().GetURLMatcherConditionSets(
+        &condition_sets_first_party, 1);
     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());
     }
+    for (URLMatcherConditionSet::Vector::iterator j =
+             condition_sets_first_party.begin();
+         j != condition_sets_first_party.end(); ++j) {
+      remove_from_first_party_url_matcher.push_back((*j)->id());
+      rule_triggers_.erase((*j)->id());
+    }
 
     rules_with_untriggered_conditions_.erase(rule);
 
@@ -223,8 +248,10 @@ std::string WebRequestRulesRegistry::RemoveRulesImpl(
     webrequest_rules_.erase(webrequest_rules_entry);
   }
 
-  // Clear URLMatcher based on condition_set_ids that are not needed any more.
+  // Clear URL matchers based on condition_set_ids that are not needed any more.
   url_matcher_.RemoveConditionSets(remove_from_url_matcher);
+  first_party_url_matcher_.RemoveConditionSets(
+      remove_from_first_party_url_matcher);
 
   ClearCacheOnNavigation();
 
@@ -253,7 +280,7 @@ content::BrowserThread::ID WebRequestRulesRegistry::GetOwnerThread() const {
 
 bool WebRequestRulesRegistry::IsEmpty() const {
   return rule_triggers_.empty() && webrequest_rules_.empty() &&
-      url_matcher_.IsEmpty();
+      url_matcher_.IsEmpty() && first_party_url_matcher_.IsEmpty();
 }
 
 WebRequestRulesRegistry::~WebRequestRulesRegistry() {}