Declarative Web Request: firstPartyForCookiesUrl

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 <algorithm>
 #include <limits>
 #include <utility>
 
@@ skipping 12 matching lines @@
 
 namespace extensions {
 
 WebRequestRulesRegistry::WebRequestRulesRegistry(Profile* profile,
                                                  Delegate* delegate)
     : RulesRegistryWithCache(delegate) {
   if (profile)
     extension_info_map_ = ExtensionSystem::Get(profile)->info_map();
 }
 
-std::set<const WebRequestRule*>
-WebRequestRulesRegistry::GetMatches(
-    const DeclarativeWebRequestData& request_data) {
-  typedef std::set<const WebRequestRule*> RuleSet;
-  typedef std::set<URLMatcherConditionSet::ID> URLMatches;
+std::set<const WebRequestRule*> WebRequestRulesRegistry::GetMatches(
+    DeclarativeWebRequestData* request_data) {

[battre, 2013/01/23 18:56:34]

What do you think this:

struct WebRequestData {
  // The network request that is currently being processed.
  net::URLRequest* request;
  // The stage (progress) of the network request.
  RequestStage stage;
  // Additional information about requests that is not
  // available in all request stages.
  const net::HttpResponseHeaders* original_response_headers;
};

struct DeclarativeWebRequestData {
  const WebRequestData* request_data;
  std::set<URLMatcherConditionSet::ID> url_match_ids;
  std::set<URLMatcherConditionSet::ID> first_party_url_match_ids;
};

std::set<const WebRequestRule*> WebRequestRulesRegistry::GetMatches(
    const WebRequestData& request_data) {
  DeclarativeWebRequestData declarative_request_data;
  declarative_request_data.request_data = &request_data;
  declarative_request_data.url_match_ids = ...
  ...

  for (...)
    if ((*it)->conditions().IsFulfilled(-1, declarative_request_data)) {
      result.insert(*it);
    }
}

This way the we can keep GetMatched and CreateDeltas purely without side-effects
and take const references as parameters.

[Jeffrey Yasskin, 2013/01/24 00:39:17]

I don't have a strong opinion here, but I don't think WebRequestData vs
DeclarativeWebRequestData really captures the difference between these two
structs. I don't have better names though, so don't let me hold you up if you
want to split the struct.

[vabr (Chromium), 2013/01/24 18:24:08]

I thought about this, and did not dare to suggest the split, as it seemed too
complicated. But once you suggest it :), I'm for it.
I like the name of WebRequestData, but I would change DeclarativeWebRequestData
to WebRequestDataWithMatchIds. WDYT?

+  RuleSet result;
 
-  RuleSet result;
-  URLMatches url_matches = url_matcher_.MatchURL(request_data.request->url());
+  request_data->url_match_ids =
+      url_matcher_.MatchURL(request_data->request->url());
+  request_data->first_party_url_match_ids =
+      url_matcher_.MatchURL(request_data->request->first_party_for_cookies());
 
   // 1st phase -- add all rules with some conditions without UrlFilter
   // attributes.
   for (RuleSet::const_iterator it = rules_with_untriggered_conditions_.begin();
        it != rules_with_untriggered_conditions_.end(); ++it) {
-    if ((*it)->conditions().IsFulfilled(-1, url_matches, request_data))
+    if ((*it)->conditions().IsFulfilled(-1, *request_data))
       result.insert(*it);
   }
 
   // 2nd phase -- add all rules with some conditions triggered by URL matches.
-  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());
-    if (!ContainsKey(result, rule_trigger->second) &&
-        rule_trigger->second->conditions().IsFulfilled(*url_match, url_matches,
-                                                       request_data))
-      result.insert(rule_trigger->second);
-  }
+  AddTriggeredRules(request_data->url_match_ids, *request_data, &result);
+  AddTriggeredRules(request_data->first_party_url_match_ids,
+                    *request_data, &result);
 
   return result;
 }
 
 std::list<LinkedPtrEventResponseDelta> WebRequestRulesRegistry::CreateDeltas(
     const ExtensionInfoMap* extension_info_map,
-    const DeclarativeWebRequestData& request_data,
+    DeclarativeWebRequestData* request_data,
     bool crosses_incognito) {
   if (webrequest_rules_.empty())
     return std::list<LinkedPtrEventResponseDelta>();
 
   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;
@@ skipping 32 matching lines @@
 
     // Skip rule if a previous rule of this extension instructed to ignore
     // all rules with a lower priority than min_priorities[extension_id].
     int current_min_priority = min_priorities[extension_id];
     if (priority_of_rule < current_min_priority)
       continue;
 
 
     std::list<LinkedPtrEventResponseDelta> rule_result;
     WebRequestAction::ApplyInfo apply_info = {
-      extension_info_map, request_data, crosses_incognito, &rule_result
+      extension_info_map, *request_data, crosses_incognito, &rule_result
     };
     rule->Apply(&apply_info);
     result.splice(result.begin(), rule_result);
 
     min_priorities[extension_id] = std::max(current_min_priority,
                                             rule->GetMinimumPriority());
   }
   return result;
 }
 
@@ skipping 163 matching lines @@
         found_matching_condition = true;
     }
     if (!found_matching_condition) {
       *error = kActionCannotBeExecuted;
       return false;
     }
   }
   return true;
 }
 
+void WebRequestRulesRegistry::AddTriggeredRules(
+    const URLMatches& url_matches,
+    const DeclarativeWebRequestData& request_data,
+    RuleSet* result) {
+  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());
+    if (!ContainsKey(*result, rule_trigger->second) &&
+        rule_trigger->second->conditions().IsFulfilled(*url_match,
+                                                       request_data))
+      result->insert(rule_trigger->second);
+  }
+}
+
 }  // namespace extensions