Remove the content settings dependencies from BrowsingDataFilterBuilder et al.

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

Index: chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
index 2818aa42be11ab6e5a312cb32881091b96a70918..e75f040540c9dd563178aea22899d2eba3acc802 100644
--- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
+++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
@@ -128,11 +128,21 @@ base::Callback<void(T)> IgnoreArgument(const base::Closure& callback) {
   return base::Bind(&IgnoreArgumentHelper<T>, callback);
 }
 
-bool ForwardPrimaryPatternCallback(
-    const base::Callback<bool(const ContentSettingsPattern&)> predicate,
+bool WebsiteSettingsFilterAdapter(
+    const base::Callback<bool(const GURL&)> predicate,
     const ContentSettingsPattern& primary_pattern,
     const ContentSettingsPattern& secondary_pattern) {
-  return predicate.Run(primary_pattern);
+  // Ignore the default setting.
+  if (primary_pattern == ContentSettingsPattern::Wildcard())
+    return false;
+
+  // Website settings only use origin-scoped patterns. The only content setting
+  // currently supported by ChromeBrowsingDataRemoverDelegate is
+  // DURABLE_STORAGE, which also only uses origin-scoped patterns. Such patterns
+  // can be directly translated to a GURL.
+  GURL url(primary_pattern.ToString());
+  DCHECK(url.is_valid());

[Mike West, 2017/01/09 08:53:48]

What happens if this DCHECK fails in Release? Will passing an invalid URL to
`predicate.Run()` explode somewhere in an ugly way?

[msramek, 2017/01/09 09:39:43]

An invalid URL will be passed to the filter, where we will try to match it with
listed origins (OriginFilterBuilder) or domains
(RegistrableDomainFilterBuilder). It will not be equal to any entry, which means
it will never be matched by whitelist and always be by blacklist. But it will
not explode.

I have already reached to raymes@ to discuss this, as the way we use
ContentSettingPattern in places where Origin would suffice seems cumbersome (and
I'm actually surprised this is the first time it comes up). This DCHECK should
eventually be handled somewhere in WebsiteSettingsRegistry.

+  return predicate.Run(url);
 }
 
 #if !defined(DISABLE_NACL)
@@ -252,8 +262,6 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
 
   base::Callback<bool(const GURL& url)> filter =
       filter_builder.BuildGeneralFilter();
-  base::Callback<bool(const ContentSettingsPattern& url)> same_pattern_filter =
-      filter_builder.BuildWebsiteSettingsPatternMatchesFilter();
 
   // Some backends support a filter that |is_null()| to make complete deletion
   // more efficient.
@@ -540,7 +548,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
     HostContentSettingsMapFactory::GetForProfile(profile_)
         ->ClearSettingsForOneTypeWithPredicate(
             CONTENT_SETTINGS_TYPE_DURABLE_STORAGE,
-            base::Bind(&ForwardPrimaryPatternCallback, same_pattern_filter));
+            base::Bind(&WebsiteSettingsFilterAdapter, filter));
   }
 
   //////////////////////////////////////////////////////////////////////////////
@@ -549,7 +557,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
     HostContentSettingsMapFactory::GetForProfile(profile_)
         ->ClearSettingsForOneTypeWithPredicate(
             CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
-            base::Bind(&ForwardPrimaryPatternCallback, same_pattern_filter));
+            base::Bind(&WebsiteSettingsFilterAdapter, filter));
   }
 
   if ((remove_mask & BrowsingDataRemover::REMOVE_SITE_USAGE_DATA) ||
@@ -557,7 +565,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
     HostContentSettingsMapFactory::GetForProfile(profile_)
         ->ClearSettingsForOneTypeWithPredicate(
             CONTENT_SETTINGS_TYPE_APP_BANNER,
-            base::Bind(&ForwardPrimaryPatternCallback, same_pattern_filter));
+            base::Bind(&WebsiteSettingsFilterAdapter, filter));
 
     PermissionDecisionAutoBlocker::RemoveCountsByUrl(profile_, filter);
   }