Add an OriginFilterBuilder class for [white|black]listing origins.

url/origin_filter.cc

Index: url/origin_filter.cc
diff --git a/url/origin_filter.cc b/url/origin_filter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..33ed7ffd3ca651290ad153958a180cb13ea42158
--- /dev/null
+++ b/url/origin_filter.cc
@@ -0,0 +1,130 @@
+// Copyright 2016 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 "url/origin_filter.h"
+
+#include <string>
+#include <vector>
+
+namespace url {
+
+// static
+scoped_ptr<OriginFilter> OriginFilter::AsWhitelist(
+    const std::vector<Origin>& origins) {
+  return make_scoped_ptr(new OriginFilter(origins, true /* whitelist_mode */));
+}
+
+// static
+scoped_ptr<OriginFilter> OriginFilter::AsBlacklist(
+    const std::vector<Origin>& origins) {
+  return make_scoped_ptr(new OriginFilter(origins, false /* whitelist_mode */));
+}
+
+// static
+scoped_ptr<OriginFilter> OriginFilter::Empty() {
+  // Empty filter matches everything, which means it's an empty blacklist.
+  std::vector<Origin> empty;
+  return make_scoped_ptr(new OriginFilter(empty, false /* whitelist_mode */));
+}
+
+OriginFilter::~OriginFilter() {
+}
+
+bool OriginFilter::MatchesURL(const GURL& url) const {
+  return ((origin_list_.find(Origin(url)) != origin_list_.end()) ==
+          whitelist_mode_);
+}
+
+bool OriginFilter::MatchesURLWithSubdomains(const GURL& url) const {

[brettw, 2016/02/01 22:48:25]

I don't understand this function. Why would the caller be responsible for
determining URL vs. subdomain rather than the filter creator?

Also, I think if you want to expose a "delete this domain" you want to use the
effective TLD service. Probably that should replace the code you have here.

[msramek, 2016/02/05 17:16:35]

If the decision between exact origin / subdomains is determined on a per-backend
basis, then it's the same thing. If it's not, we can have a wrapper that decides
between the two methods e.g. according to user preference (I do plan to extend
this class, as I currently don't have insight to special cases awaiting in
various backends, nor product decisions of possible clients). But since we now
only pass around callback and this class is only used within
BrowsingDataRemover, it now is the creator who determines that.

I thought about using eTLD+1 for this case, but I don't think it's the right
choice. I think it's reasonable to want to delete 'plus.google.com' data without
deleting 'google.com'.

+  // If there is no concept of subdomains, simply delegate to MatchesURL().
+  if (url.HostIsIPAddress())
+    return MatchesURL(url);
+
+  // TODO(msramek): We do not expect filters to be particularly large.
+  // If they are, replace std::set with a trie for faster searching.
+  std::string host = url.host();
+  for (unsigned int i = 0; i < host.length(); ++i) {
+    if (i != 0 && host[i - 1] != '.')
+      continue;
+
+    Origin origin_with_removed_subdomain =
+        Origin::UnsafelyCreateOriginWithoutNormalization(
+            url.scheme(),
+            host.substr(i),
+            url.EffectiveIntPort());
+
+    // If we recognize the origin, return true for whitelist and false
+    // for blacklist.
+    if (origin_list_.find(Origin(origin_with_removed_subdomain))
+        != origin_list_.end()) {
+      return whitelist_mode_;
+    }
+  }
+
+  // We do not recognize the URL. Return false for whitelist mode and true
+  // for blacklist mode.
+  return !whitelist_mode_;
+}
+
+bool OriginFilter::operator==(const OriginFilter& other) const {
+  // Two filters can only be equal if they are both whitelists or both
+  // blacklists, otherwise one of them is finite and the other infinite.
+  if (whitelist_mode_ != other.whitelist_mode_)
+    return false;
+
+  // Since unique origins are excluded, the filters are equivalent if they
+  // contain the same set of origins.
+  if (origin_list_.size() != other.origin_list_.size())
+    return false;
+
+  std::set<Origin>::const_iterator their_origin =
+      other.origin_list_.begin();
+  for (std::set<Origin>::const_iterator our_origin = origin_list_.begin();

[brettw, 2016/02/01 22:48:25]

for (const Origin& : origin_list) ...

[msramek, 2016/02/05 17:16:35]

I removed operator==, as it's not needed anymore.

+       our_origin != origin_list_.end(); ++our_origin, ++their_origin) {
+    if (!our_origin->IsSameOriginWith(*their_origin))
+      return false;
+  }
+  DCHECK(their_origin == other.origin_list_.end());
+
+  return true;
+}
+
+bool OriginFilter::operator!=(const OriginFilter& other) const {
+  return !(this->operator==(other));
+}
+
+OriginFilter::OriginFilter(const std::vector<Origin>& origins,
+                           bool whitelist_mode)
+    : whitelist_mode_(whitelist_mode) {
+  for (const Origin& origin : origins) {
+    // By limiting the filter to non-unique origins, we can guarantee that
+    // origin1 < origin2 && origin1 > origin2 <=> origin1.isSameOrigin(origin2).
+    // This means that std::set::find() will use the same semantics for
+    // origin comparison as Origin::IsSameOriginWith(). Furthermore, this
+    // means that two filters are equal iff they are equal element-wise.
+    if (origin.unique()) {
+      LOG(WARNING) << "Invalid origin passed into OriginFilter.";
+      continue;
+    }
+
+    // TODO(msramek): All urls with file scheme currently map to the same
+    // origin. This is currently not a problem, but if it becomes one,
+    // consider recognizing the URL path.
+
+    origin_list_.insert(origin);
+  }
+}
+
+std::ostream& operator<<(std::ostream& out, const OriginFilter& filter) {
+  out << "OriginFilter("
+      << (filter.whitelist_mode_ ? "whitelist" : "blacklist")
+      << " = { ";
+  for (const Origin& origin : filter.origin_list_) {
+    out << origin << ", ";
+  }
+  out << "})";
+  return out;
+}
+
+}  // namespace url