Add removal filter support for Cookies, Storage, and Content Settings.

chrome/browser/browsing_data/origin_filter_builder.cc

 // 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"
+#include "components/content_settings/core/common/content_settings_pattern.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 
 namespace {
 
 bool NoopFilter(const GURL& url) {
   return true;
 }
 
 }  // namespace
 
 OriginFilterBuilder::OriginFilterBuilder(Mode mode)
@@ skipping 19 matching lines @@
   // origin. This is currently not a problem, but if it becomes one,
   // consider recognizing the URL path.
 
   origin_list_.insert(origin);
 }
 
 void OriginFilterBuilder::SetMode(Mode mode) {
   mode_ = mode;
 }
 
+bool OriginFilterBuilder::IsEmptyBlacklist() const {
+  return mode_ == Mode::BLACKLIST && origin_list_.empty();
+}
+
 base::Callback<bool(const GURL&)>
     OriginFilterBuilder::BuildSameOriginFilter() const {
   std::set<url::Origin>* origins = new std::set<url::Origin>(origin_list_);
   return base::Bind(&OriginFilterBuilder::MatchesURL,
                     base::Owned(origins), mode_);
 }
 
+base::Callback<bool(const ContentSettingsPattern& pattern)>
+    OriginFilterBuilder::BuildSameOriginContentSettingsFilter() const {

[msramek, 2016/03/10 20:10:08]

Consider renaming to BuildWebsiteSettingsFilter().

Two reasons:
1. The matching semantics here are not really about same origin, but about
ContentSettingsPattern::Matches(). The pattern can be much broader than just
origin (such as [*.]google.com or *:80).

2. s/ContentSettings/WebsiteSettings/
The state of the code here is a bit unfortunate. The component is called
content_settings. The class is called ContentSettingsPattern. However, the
nomenclature has recently changed. We call the settings "website settings", and
"content settings" are a special subclass that only support ContentSetting as a
value. This is well illustrated in the classes WebsiteSettingsRegistry and
ContentSettingsRegistry.

Most importantly - we use this method on APP_BANNER and SITE_ENGAGEMENT, which
are website settings, but NOT content settings.   In fact, we really don't want
to use this on actual content settings. If I, as a user, set up a setting for
pattern "https://*" and then delete the origin "https://google.com", I would be
really surprised if that broad setting was deleted. But no such surprises should
happen for APP_BANNER and SITE_ENGAGEMENT, since they are not manually settable
by the user, and thus (to my knowledge) only use origins as their patterns. But
I really appreciate that you did test various possible patterns in this method's
unittest.

[dmurph, 2016/03/10 23:00:11]

Hmm.... sounds good, should I add more documentation around this filter to make
sure this isn't misused? What do you recommend?

[msramek, 2016/03/11 20:54:50]

We're currently only dealing with website settings (and not content settings) in
BrowsingDataRemover, so it isn't that likely to be misused. If the method is
called BuildWebsiteSettingsFilter and its parameter is ContentSettingsPattern,
it will hopefully be clear that it matches according to pattern.

But you might want to add a comment to the tests, noting that we're testing
broader-scoped patterns just for the sake of completeness, and we don't expect
website settings to actually use them.

+  std::set<url::Origin>* origins = new std::set<url::Origin>(origin_list_);
+  return base::Bind(&OriginFilterBuilder::MatchesContentSettingsPattern,
+                    base::Owned(origins), mode_);
+}
+
+base::Callback<bool(const net::CanonicalCookie& pattern)>
+    OriginFilterBuilder::BuildDomainCookieFilter() const {
+  std::set<url::Origin>* origins = new std::set<url::Origin>(origin_list_);
+  return base::Bind(&OriginFilterBuilder::MatchesCookieForTLDPlusOne,
+                    base::Owned(origins), mode_);
+}
+
 base::Callback<bool(const GURL&)>
     OriginFilterBuilder::BuildDomainFilter() const {
   std::set<url::Origin>* origins = new std::set<url::Origin>(origin_list_);
   return base::Bind(&OriginFilterBuilder::MatchesURLWithSubdomains,
                     base::Owned(origins), mode_);
 }
 
 // static
 base::Callback<bool(const GURL&)> OriginFilterBuilder::BuildNoopFilter() {
   return base::Bind(&NoopFilter);
 }
 
 // static
 bool OriginFilterBuilder::MatchesURL(
     std::set<url::Origin>* origins, Mode mode, const GURL& url) {
   return ((origins->find(url::Origin(url)) != origins->end()) ==
           (mode == WHITELIST));
 }
 
 // static
+bool OriginFilterBuilder::MatchesContentSettingsPattern(
+    std::set<url::Origin>* origins,
+    Mode mode,
+    const ContentSettingsPattern& pattern) {
+  for (const url::Origin& origin : *origins) {
+    if (pattern.Matches(GURL(origin.Serialize())))
+      return mode == WHITELIST;
+  }
+  return mode != WHITELIST;
+}
+
+// static
+bool OriginFilterBuilder::MatchesCookieForTLDPlusOne(
+    std::set<url::Origin>* origins,
+    Mode mode,
+    const net::CanonicalCookie& cookie) {
+  if (origins->empty())
+    return mode == BLACKLIST;
+
+  bool found = false;
+  std::string cookie_domain = cookie.Domain();
+  if (cookie.IsDomainCookie()) {
+    cookie_domain = cookie_domain.substr(1);
+  }

[Mike West, 2016/03/11 08:36:10]

Nit: No {} for single-line clauses.

[dmurph, 2016/04/05 00:16:28]

Done.

+  std::string cookie_tld_plus_one =
+      net::registry_controlled_domains::GetDomainAndRegistry(

[msramek, 2016/03/10 20:10:08]

I'm not really familiar with cookies, so I'll trust bsittler@'s judgement.

However, as a sanity check. My understanding is that some cookies, those where
Set-Cookie header was empty, are only scoped to the particular domain, and not
shared with superdomains. Do we still want to do eTLD+1 comparison for those?

[dmurph, 2016/03/10 23:00:11]

We basically break websites unless we treat this as eTLD+1 (we can't clear a
subdomain and leave the domain still there, and vice versa). I would also like
Mike West's feedback here.

+          cookie_domain,
+          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
+  for (const url::Origin& origin : *origins) {
+    GURL origin_url(origin.Serialize());
+    if (origin_url.HostIsIPAddress()) {
+      if (origin_url.host() == cookie_domain) {
+        found = true;
+        break;
+      }
+    } else if (cookie_tld_plus_one ==
+               net::registry_controlled_domains::GetDomainAndRegistry(
+                   origin.host(), net::registry_controlled_domains::
+                                      INCLUDE_PRIVATE_REGISTRIES)) {
+      found = true;
+      break;
+    }
+  }
+  return (mode == WHITELIST) == found;
+}
+
+// static
 bool OriginFilterBuilder::MatchesURLWithSubdomains(
     std::set<url::Origin>* origins, Mode mode, const GURL& url) {
   if (origins->empty())
     return mode == BLACKLIST;
 
   // If there is no concept of subdomains, simply delegate to MatchesURL().
   if (url.HostIsIPAddress())
     return MatchesURL(origins, mode, url);
 
   // TODO(msramek): We do not expect filters to be particularly large.
@@ skipping 15 matching lines @@
     if (origins->find(url::Origin(origin_with_removed_subdomain))
         != origins->end()) {
       return mode == WHITELIST;
     }
   }
 
   // We do not recognize the URL. Return false for whitelist mode and true
   // for blacklist mode.
   return mode == BLACKLIST;
 }