Refactor URLBlacklistManager to use URLMatcher.

chrome/common/extensions/matcher/url_matcher.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/common/extensions/matcher/url_matcher.h"
 
 #include <algorithm>
 #include <iterator>
 
 #include "base/logging.h"
@@ skipping 241 matching lines @@
 }
 
 URLMatcherCondition URLMatcherConditionFactory::CreatePathPrefixCondition(
     const std::string& prefix) {
   return CreateCondition(URLMatcherCondition::PATH_PREFIX,
       kEndOfDomain + prefix);
 }
 
 URLMatcherCondition URLMatcherConditionFactory::CreatePathSuffixCondition(
     const std::string& suffix) {
   return CreateCondition(URLMatcherCondition::HOST_SUFFIX, suffix + kEndOfPath);

[Joao da Silva, 2012/07/13 12:43:13]

Hmm PATH_SUFFIX?

If you fix this, do you mind adding a test that would make this fail before the
fix?

[Bernhard Bauer, 2012/07/13 14:21:07]

It looks like using PATH_SUFFIX vs. HOST_SUFFIX doesn't make a difference here:
the condition is only used when determining which parts of the URL to match
against, and PATH_SUFFIX and HOST_SUFFIX are equivalent in that regard. In fact,
neither one is referenced again in the file.

Dominic, do you think it would make sense to reduce the number of conditions? We
probably only need to distinguish the following cases:

* Match host (HOST_CONTAINS)
* Match path (PATH_CONTAINS)
* Match query (QUERY_CONTAINS)
* Match full URL (URL_{PREFIX,SUFFIX,CONTAINS,EQUALS})
* Match component-separated URL (all others)
I'd like to hold off on that until we have determined whether we want to change
the enum.

[battre, 2012/07/13 15:28:54]

I have a slight tendency towards sticking to the current naming scheme because I
think that otherwise, we would need to come up with a better name for
URLMatcherCondition::Criterion which I cannot at the moment.

[Joao da Silva, 2012/07/13 15:37:12]

The Criterion enum has the PATH_SUFFIX constant, but it isn't used anywhere.
Should it be removed then?

[Bernhard Bauer, 2012/07/13 16:40:39]

How about:

enum MatchAgainst {
  MATCH_AGAINST_HOST,
  MATCH_AGAINST_PATH,
  MATCH_AGAINST_QUERY,
  MATCH_AGAINST_URL_FULL,
  MATCH_AGAINST_URL_COMPONENTS,
}

?
It is used in CreatePathSuffixCondition().

[battre, 2012/07/16 09:17:00]

I think that line 262 should contain PATH_SUFFIX

[battre, 2012/07/16 09:17:00]

Frankly, I am not such a big fan of this, because I think it makes the code more
difficult to understand and write, while the gain is pretty limited.

 }
 
 URLMatcherCondition URLMatcherConditionFactory::CreatePathContainsCondition(
     const std::string& str) {
   return CreateCondition(URLMatcherCondition::PATH_CONTAINS, str);
 }
 
 URLMatcherCondition URLMatcherConditionFactory::CreatePathEqualsCondition(
     const std::string& str) {
   return CreateCondition(URLMatcherCondition::PATH_EQUALS,
@@ skipping 23 matching lines @@
 }
 
 URLMatcherCondition
     URLMatcherConditionFactory::CreateHostSuffixPathPrefixCondition(
     const std::string& host_suffix,
     const std::string& path_prefix) {
   return CreateCondition(URLMatcherCondition::HOST_SUFFIX_PATH_PREFIX,
       host_suffix + kEndOfDomain + path_prefix);
 }
 
+URLMatcherCondition
+URLMatcherConditionFactory::CreateHostEqualsPathPrefixCondition(
+    const std::string& host,
+    const std::string& path_prefix) {
+  return CreateCondition(URLMatcherCondition::HOST_EQUALS_PATH_PREFIX,
+      kBeginningOfURL + CanonicalizeHostname(host) + kEndOfDomain +
+      path_prefix);
+}
+
 std::string URLMatcherConditionFactory::CanonicalizeURLForFullSearches(
     const GURL& url) {
   return kBeginningOfURL + CanonicalizeHostname(url.host()) + url.path() +
       (url.has_query() ? "?" + url.query() : "") + kEndOfURL;
 }
 
 URLMatcherCondition URLMatcherConditionFactory::CreateURLPrefixCondition(
     const std::string& prefix) {
   return CreateCondition(URLMatcherCondition::URL_PREFIX,
       kBeginningOfURL + CanonicalizeHostname(prefix));
@@ skipping 354 matching lines @@
 }
 
 void URLMatcher::UpdateInternalDatastructures() {
   UpdateSubstringSetMatcher(false);
   UpdateSubstringSetMatcher(true);
   UpdateTriggers();
   UpdateConditionFactory();
 }
 
 }  // namespace extensions