Refactoring how conditions without URL attributes are handled.

chrome/browser/extensions/api/declarative_webrequest/webrequest_condition_unittest.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_condition.h"
 
 #include <set>
 
 #include "base/message_loop.h"
 #include "base/test/values_test_util.h"
@@ skipping 67 matching lines @@
       WebRequestRule::RequestData(&match_request, ON_BEFORE_REQUEST)));
 
   net::TestURLRequest wrong_resource_type(
       GURL("https://www.example.com"), NULL, &context);
   content::ResourceRequestInfo::AllocateForTesting(&wrong_resource_type,
       ResourceType::SUB_FRAME, NULL, -1, -1);
   EXPECT_FALSE(result->IsFulfilled(
       WebRequestRule::RequestData(&wrong_resource_type, ON_BEFORE_REQUEST)));
 }
 
+// The IsFulfilledIndependentlyOfURL method tests that a condition has no
+// UrlFilter attributes, and all its (non-UrlFilter) attributes are satisfied.
+// We test here that:
+//   1. A non-empty condition without UrlFilter attributes is fulfilled iff its
+//      attributes are fulfilled.
+//   2. An empty condition (in particular, without UrlFilter attributes) is
+//      always fulfilled.
+//   3. A condition with a UrlFilter and a non-UrlFilter attribute is never
+//      fulfilled.
+//   4. A condition with only a UrlFilter attribute is never fulfilled.
+TEST(WebRequestConditionTest, IsFulfilledIndependentlyOfURL) {
+  // Necessary for TestURLRequest.
+  MessageLoop message_loop(MessageLoop::TYPE_IO);
+  URLMatcher matcher;
+  std::string error;
+
+  // The empty condition.
+  error.clear();
+  scoped_ptr<WebRequestCondition> condition_empty = WebRequestCondition::Create(
+      matcher.condition_factory(),
+      *base::test::ParseJson(
+          "{ \n"
+          "  \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n"
+          "}"),
+      &error);
+  EXPECT_EQ("", error);
+  ASSERT_TRUE(condition_empty.get());
+
+  // A condition without a UrlFilter attribute, which is always true.
+  error.clear();
+  scoped_ptr<WebRequestCondition> condition_no_url_true =
+      WebRequestCondition::Create(
+          matcher.condition_factory(),
+          *base::test::ParseJson(
+              "{ \n"
+              "  \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n"
+              // There is no "first party for cookies URL in the requests below,
+              // therefore all requests are considered first party for cookies.
+              "  \"thirdPartyForCookies\": false, \n"
+              "}"),
+          &error);
+  EXPECT_EQ("", error);
+  ASSERT_TRUE(condition_no_url_true.get());
+
+  // A condition without a UrlFilter attribute, which is always false.
+  error.clear();
+  scoped_ptr<WebRequestCondition> condition_no_url_false =
+      WebRequestCondition::Create(
+          matcher.condition_factory(),
+          *base::test::ParseJson(
+              "{ \n"
+              "  \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n"
+              "  \"thirdPartyForCookies\": true, \n"
+              "}"),
+          &error);
+  EXPECT_EQ("", error);
+  ASSERT_TRUE(condition_no_url_false.get());
+
+  // MATCHING_URL_FILTER must always match kUrl.
+  static const char kUrl[] = "https://www.example.com";
+#define MATCHING_URL_FILTER \

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

It'll work to define this as a std::string, and concatenate it with '+' to the
varying parts of the JSON string.

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

Agreed, only this part of the test is no longer needed after getting rid of
IsFulfilledIndependentlyOfURL, so I deleted it instead.

+      "  \"url\": { \n" \
+      "    \"hostSuffix\": \"example.com\", \n" \
+      "    \"hostPrefix\": \"www\", \n" \
+      "    \"schemes\": [\"https\"], \n" \
+      "  }, \n"
+
+  // A condition with only a UrlFilter attribute.
+  error.clear();
+  scoped_ptr<WebRequestCondition> condition_url = WebRequestCondition::Create(
+      matcher.condition_factory(),
+      *base::test::ParseJson(
+          "{ \n"
+          "  \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n"
+          MATCHING_URL_FILTER
+          "}"),
+      &error);
+  EXPECT_EQ("", error);
+  ASSERT_TRUE(condition_url.get());
+
+  // A condition with a UrlFilter attribute and an always true non-UrlFilter
+  // attribute.
+  error.clear();
+  scoped_ptr<WebRequestCondition> condition_both = WebRequestCondition::Create(
+      matcher.condition_factory(),
+      *base::test::ParseJson(
+          "{ \n"
+          "  \"instanceType\": \"declarativeWebRequest.RequestMatcher\", \n"
+          MATCHING_URL_FILTER
+          "  \"thirdPartyForCookies\": false, \n"
+          "}"),
+      &error);
+  EXPECT_EQ("", error);
+  ASSERT_TRUE(condition_both.get());
+#undef MATCHING_URL_FILTER
+
+  net::TestURLRequestContext context;
+  net::TestURLRequest https_request(GURL(kUrl), NULL, &context);
+
+  // 1. A non-empty condition without UrlFilter attributes is fulfilled iff its
+  //    attributes are fulfilled.
+  EXPECT_FALSE(condition_no_url_false->IsFulfilledIndependentlyOfURL(
+      WebRequestRule::RequestData(&https_request, ON_BEFORE_REQUEST)));
+
+  EXPECT_TRUE(condition_no_url_true->IsFulfilledIndependentlyOfURL(
+      WebRequestRule::RequestData(&https_request, ON_BEFORE_REQUEST)));
+
+  // 2. An empty condition (in particular, without UrlFilter attributes) is
+  //    always fulfilled.
+  EXPECT_TRUE(condition_empty->IsFulfilledIndependentlyOfURL(
+      WebRequestRule::RequestData(&https_request, ON_BEFORE_REQUEST)));
+
+  // 3. A condition with a UrlFilter and a non-UrlFilter attribute is never
+  //    fulfilled.
+  EXPECT_FALSE(condition_both->IsFulfilledIndependentlyOfURL(
+      WebRequestRule::RequestData(&https_request, ON_BEFORE_REQUEST)));
+
+  // 4. A condition with only a UrlFilter attribute is never fulfilled.
+  EXPECT_FALSE(condition_url->IsFulfilledIndependentlyOfURL(
+      WebRequestRule::RequestData(&https_request, ON_BEFORE_REQUEST)));
+}
+
 TEST(WebRequestConditionTest, CreateConditionSet) {
   // Necessary for TestURLRequest.
   MessageLoop message_loop(MessageLoop::TYPE_IO);
   URLMatcher matcher;
 
   WebRequestConditionSet::AnyVector conditions;
 
   linked_ptr<json_schema_compiler::any::Any> condition1 = make_linked_ptr(
       new json_schema_compiler::any::Any);
   condition1->Init(*base::test::ParseJson(
@@ skipping 27 matching lines @@
   EXPECT_EQ("", error);
   ASSERT_TRUE(result.get());
   EXPECT_EQ(2u, result->conditions().size());
 
   // Tell the URLMatcher about our shiny new patterns.
   URLMatcherConditionSet::Vector url_matcher_condition_set;
   result->GetURLMatcherConditionSets(&url_matcher_condition_set);
   matcher.AddConditionSets(url_matcher_condition_set);
 
   std::set<URLMatcherConditionSet::ID> url_match_ids;
-  int number_matches = 0;
 
   // Test that the result is correct and matches http://www.example.com and
   // https://www.example.com
   GURL http_url("http://www.example.com");
   net::TestURLRequestContext context;
   net::TestURLRequest http_request(http_url, NULL, &context);
   url_match_ids = matcher.MatchURL(http_url);
-  for (std::set<URLMatcherConditionSet::ID>::iterator i = url_match_ids.begin();
-       i != url_match_ids.end(); ++i) {
-    if (result->IsFulfilled(
-            *i, WebRequestRule::RequestData(&http_request, ON_BEFORE_REQUEST)))
-      ++number_matches;
-  }
-  EXPECT_EQ(1, number_matches);
+  EXPECT_TRUE(result->IsFulfilled(
+      url_match_ids,
+      WebRequestRule::RequestData(&http_request, ON_BEFORE_REQUEST)));
 
   GURL https_url("https://www.example.com");
   url_match_ids = matcher.MatchURL(https_url);
   net::TestURLRequest https_request(https_url, NULL, &context);
-  number_matches = 0;
-  for (std::set<URLMatcherConditionSet::ID>::iterator i = url_match_ids.begin();
-       i != url_match_ids.end(); ++i) {
-    if (result->IsFulfilled(
-            *i, WebRequestRule::RequestData(&https_request, ON_BEFORE_REQUEST)))
-      ++number_matches;
-  }
-  EXPECT_EQ(1, number_matches);
+  EXPECT_TRUE(result->IsFulfilled(
+      url_match_ids,
+      WebRequestRule::RequestData(&https_request, ON_BEFORE_REQUEST)));
 
   // Check that both, hostPrefix and hostSuffix are evaluated.
   GURL https_foo_url("https://foo.example.com");
   url_match_ids = matcher.MatchURL(https_foo_url);
   net::TestURLRequest https_foo_request(https_foo_url, NULL, &context);
-  number_matches = 0;
-  for (std::set<URLMatcherConditionSet::ID>::iterator i = url_match_ids.begin();
-       i != url_match_ids.end(); ++i) {
-    if (result->IsFulfilled(
-            *i, WebRequestRule::RequestData(
-                &https_foo_request, ON_BEFORE_REQUEST)))
-      ++number_matches;
-  }
-  EXPECT_EQ(0, number_matches);
+  EXPECT_FALSE(result->IsFulfilled(
+      url_match_ids,
+      WebRequestRule::RequestData(&https_foo_request, ON_BEFORE_REQUEST)));
 }
 
 TEST(WebRequestConditionTest, TestPortFilter) {
   // Necessary for TestURLRequest.
   MessageLoop message_loop(MessageLoop::TYPE_IO);
   URLMatcher matcher;
 
   linked_ptr<json_schema_compiler::any::Any> any_condition =
       make_linked_ptr(new json_schema_compiler::any::Any);
   any_condition->Init(*base::test::ParseJson(
@@ skipping 72 matching lines @@
           // filters.
           "  \"requestHeaders\": [{}], \n"
           "  \"responseHeaders\": [{}], \n"
           "}"),
       &error);
   EXPECT_FALSE(error.empty());
   EXPECT_FALSE(result.get());
 }
 
 }  // namespace extensions