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

chrome/browser/browsing_data/origin_filter_builder.cc

Index: chrome/browser/browsing_data/origin_filter_builder.cc
diff --git a/chrome/browser/browsing_data/origin_filter_builder.cc b/chrome/browser/browsing_data/origin_filter_builder.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8923e911bd131e4e37c1c5eeb37d5a8d7c6f41ee
--- /dev/null
+++ b/chrome/browser/browsing_data/origin_filter_builder.cc
@@ -0,0 +1,111 @@
+// 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 "chrome/browser/browsing_data/origin_filter_builder.h"
+
+#include <string>
+#include <vector>
+
+#include "base/bind.h"
+
+// static
+scoped_ptr<OriginFilterBuilder> OriginFilterBuilder::AsWhitelist(
+    const std::vector<url::Origin>& origins) {
+  return make_scoped_ptr(
+      new OriginFilterBuilder(origins, true /* whitelist_mode */));
+}
+
+// static
+scoped_ptr<OriginFilterBuilder> OriginFilterBuilder::AsBlacklist(
+    const std::vector<url::Origin>& origins) {
+  return make_scoped_ptr(
+      new OriginFilterBuilder(origins, false /* whitelist_mode */));
+}
+
+// static
+scoped_ptr<OriginFilterBuilder> OriginFilterBuilder::Empty() {
+  // Empty filter matches everything, which means it's an empty blacklist.
+  std::vector<url::Origin> empty;
+  return make_scoped_ptr(
+      new OriginFilterBuilder(empty, false /* whitelist_mode */));
+}
+
+OriginFilterBuilder::~OriginFilterBuilder() {
+}
+
+const base::Callback<bool(const GURL&)>&
+    OriginFilterBuilder::GetSameOriginFilter() {
+  return matches_url_callback_;
+}
+
+const base::Callback<bool(const GURL&)>&
+    OriginFilterBuilder::GetSubdomainFilter() {
+  return matches_url_with_subdomains_callback_;
+}
+
+bool OriginFilterBuilder::MatchesURL(const GURL& url) const {
+  return ((origin_list_.find(url::Origin(url)) != origin_list_.end()) ==
+          whitelist_mode_);
+}
+
+bool OriginFilterBuilder::MatchesURLWithSubdomains(const GURL& url) const {

[brettw, 2016/02/05 21:26:04]

Since you have an "Empty" pattern that matches everything, you should probably
optimize this and return false if there are no origins so you don't do a lot of
work taking apart every URL for doing just time-based deletions. Same with
MatchesURL above (although the problem is less there).

[msramek, 2016/02/10 14:30:37]

Done. Good point, especially since we expect most calls to be just plain old
unfiltered data deletions.

+  // 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();

[brettw, 2016/02/05 21:26:04]

Instead here:
  base::StringPiece host_piece = url.host_piece();
  for (size_t i = 0; i...

[msramek, 2016/02/10 14:30:36]

Done.

+  for (unsigned int i = 0; i < host.length(); ++i) {
+    if (i != 0 && host[i - 1] != '.')
+      continue;
+
+    url::Origin origin_with_removed_subdomain =
+        url::Origin::UnsafelyCreateOriginWithoutNormalization(
+            url.scheme(),

[brettw, 2016/02/05 21:26:04]

url.scheme_piece()

[msramek, 2016/02/10 14:30:37]

Done.

+            host.substr(i),
+            url.EffectiveIntPort());

[brettw, 2016/02/05 21:26:04]

Can you remove extracting the port from the loop so we don't call it every time?

[msramek, 2016/02/10 14:30:36]

Done.

+
+    // If we recognize the origin, return true for whitelist and false
+    // for blacklist.
+    if (origin_list_.find(url::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_;
+}
+
+OriginFilterBuilder::OriginFilterBuilder(
+    const std::vector<url::Origin>& origins,
+    bool whitelist_mode)
+    : whitelist_mode_(whitelist_mode) {
+  for (const url::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.";

[brettw, 2016/02/05 21:26:04]

Can this be a NOTREACHED instead? THat will be debug only. I don't think having
this in release build gives much value, and it adds >100 bytes.

[msramek, 2016/02/10 14:30:36]

Done. One reason why this wasn't a NOTREACHED originally was that I used some
unique origins in the operator== test, but that one doesn't exist anymore.

+      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);
+  }
+
+  matches_url_callback_ = base::Bind(

[brettw, 2016/02/05 21:26:04]

I think it's sufficient (and less code) to just make these callbacks on demand
from the getter and return them directly rather than keeping them on the class.

[msramek, 2016/02/10 14:30:37]

Done.

I think my intention was to make it possible to compare filters in tests (see my
comments in BrowsingDataRemoverTest - if the test and the class itself use the
same instance of OriginFilterBuilder, and they reference the very same callback,
then we're able to just compare the callbacks with operator==. Otherwise, it's
difficult to test whether they're equivalent. But the former assumption is
currently not true, so it doesn't matter for now.

+      &OriginFilterBuilder::MatchesURL,
+      base::Unretained(this));
+  matches_url_with_subdomains_callback_ = base::Bind(
+      &OriginFilterBuilder::MatchesURLWithSubdomains,
+      base::Unretained(this));
+}