BrowsingDataRemover: Use nullable_filter where possible

Lots of deletion code supports a nullable filter but we pass a proper
filter even when there is an empty blacklist. To avoid checking the
empty filter for every type of data we should just pass a nullable
filter.

Change-Id: I2a580e17f3b8e18a4a1181705907abea2617b6d3
Reviewed-on: https://chromium-review.googlesource.com/c/1322769
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606029}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc

@@ -190,9 +190,8 @@ bool WebsiteSettingsFilterAdapter(
     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.
+  // this filter is used for 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());
   return predicate.Run(url);
@@ -358,6 +357,11 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
           ? base::RepeatingCallback<bool(const GURL&)>()
           : filter;
 
+  HostContentSettingsMap::PatternSourcePredicate website_settings_filter =
+      filter_builder.IsEmptyBlacklist()
+          ? HostContentSettingsMap::PatternSourcePredicate()
+          : base::BindRepeating(&WebsiteSettingsFilterAdapter, filter);
+
   // Managed devices and supervised users can have restrictions on history
   // deletion.
   PrefService* prefs = profile_->GetPrefs();
@@ -462,12 +466,12 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
       template_url_sub_ = keywords_model->RegisterOnLoadedCallback(
           base::AdaptCallbackForRepeating(base::BindOnce(
               &ChromeBrowsingDataRemoverDelegate::OnKeywordsLoaded,
-              weak_ptr_factory_.GetWeakPtr(), filter,
+              weak_ptr_factory_.GetWeakPtr(), nullable_filter,
               CreatePendingTaskCompletionClosure())));
       keywords_model->Load();
     } else if (keywords_model) {
-      keywords_model->RemoveAutoGeneratedForUrlsBetween(filter, delete_begin_,
-                                                        delete_end_);
+      keywords_model->RemoveAutoGeneratedForUrlsBetween(
+          nullable_filter, delete_begin_, delete_end_);
     }
 
     // The PrerenderManager keeps history of prerendered pages, so clear that.
@@ -643,7 +647,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
 
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
         CONTENT_SETTINGS_TYPE_CLIENT_HINTS, base::Time(), base::Time::Max(),
-        base::BindRepeating(&WebsiteSettingsFilterAdapter, filter));
+        website_settings_filter);
 
     // Clear the safebrowsing cookies only if time period is for "all time".  It
     // doesn't make sense to apply the time period of deleting in the last X
@@ -735,7 +739,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
   if (remove_mask & DATA_TYPE_DURABLE_PERMISSION) {
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
         CONTENT_SETTINGS_TYPE_DURABLE_STORAGE, base::Time(), base::Time::Max(),
-        base::BindRepeating(&WebsiteSettingsFilterAdapter, filter));
+        website_settings_filter);
   }
 
   //////////////////////////////////////////////////////////////////////////////
@@ -745,7 +749,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
 
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
         CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, base::Time(), base::Time::Max(),
-        base::BindRepeating(&WebsiteSettingsFilterAdapter, filter));
+        website_settings_filter);
 
     if (MediaEngagementService::IsEnabled()) {
       MediaEngagementService::Get(profile_)->ClearDataBetweenTime(delete_begin_,
@@ -757,7 +761,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
       (remove_mask & DATA_TYPE_HISTORY)) {
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
         CONTENT_SETTINGS_TYPE_APP_BANNER, base::Time(), base::Time::Max(),
-        base::BindRepeating(&WebsiteSettingsFilterAdapter, filter));
+        website_settings_filter);
 
     PermissionDecisionAutoBlocker::GetForProfile(profile_)->RemoveCountsByUrl(
         filter);
@@ -765,7 +769,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
 #if BUILDFLAG(ENABLE_PLUGINS)
     host_content_settings_map_->ClearSettingsForOneTypeWithPredicate(
         CONTENT_SETTINGS_TYPE_PLUGINS_DATA, base::Time(), base::Time::Max(),
-        base::Bind(&WebsiteSettingsFilterAdapter, filter));
+        website_settings_filter);
 #endif
   }
 
@@ -1040,7 +1044,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
 #endif  // defined(OS_CHROMEOS)
 
 #if defined(OS_ANDROID)
-    ClearMediaDrmLicenses(prefs, delete_begin_, delete_end, filter,
+    ClearMediaDrmLicenses(prefs, delete_begin_, delete_end, nullable_filter,
                           CreatePendingTaskCompletionClosure());
 #endif  // defined(OS_ANDROID);
   }
@@ -1069,7 +1073,7 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData(
       else
         mode = domain_reliability::CLEAR_BEACONS;
 
-      service->ClearBrowsingData(mode, filter,
+      service->ClearBrowsingData(mode, nullable_filter,
                                  base::AdaptCallbackForRepeating(
                                      CreatePendingTaskCompletionClosure()));
     }

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -1799,8 +1799,7 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, DomainReliability_Beacons) {
       ChromeBrowsingDataRemoverDelegate::DATA_TYPE_HISTORY, false);
   EXPECT_EQ(1u, tester.clear_count());
   EXPECT_EQ(CLEAR_BEACONS, tester.last_clear_mode());
-  EXPECT_TRUE(ProbablySameFilters(
-      BrowsingDataFilterBuilder::BuildNoopFilter(), tester.last_filter()));
+  EXPECT_TRUE(tester.last_filter().is_null());
 }
 
 // TODO(crbug.com/589586): Disabled, since history is not yet marked as
@@ -1832,8 +1831,7 @@ TEST_F(ChromeBrowsingDataRemoverDelegateTest, DomainReliability_Contexts) {
                                 false);
   EXPECT_EQ(1u, tester.clear_count());
   EXPECT_EQ(CLEAR_CONTEXTS, tester.last_clear_mode());
-  EXPECT_TRUE(ProbablySameFilters(
-      BrowsingDataFilterBuilder::BuildNoopFilter(), tester.last_filter()));
+  EXPECT_TRUE(tester.last_filter().is_null());
 }
 
 TEST_F(ChromeBrowsingDataRemoverDelegateTest,

content/browser/browsing_data/browsing_data_remover_impl.cc

@@ -284,6 +284,13 @@ void BrowsingDataRemoverImpl::RemoveImpl(
   base::RepeatingCallback<bool(const GURL& url)> filter =
       filter_builder.BuildGeneralFilter();
 
+  // Some backends support a filter that |is_null()| to make complete deletion
+  // more efficient.
+  base::RepeatingCallback<bool(const GURL&)> nullable_filter =
+      filter_builder.IsEmptyBlacklist()
+          ? base::RepeatingCallback<bool(const GURL&)>()
+          : filter;
+
   //////////////////////////////////////////////////////////////////////////////
   // DATA_TYPE_DOWNLOADS
   if ((remove_mask & DATA_TYPE_DOWNLOADS) &&
@@ -434,10 +441,7 @@ void BrowsingDataRemoverImpl::RemoveImpl(
           CreatePendingTaskCompletionClosureForMojo());
     } else {
       storage_partition->ClearHttpAndMediaCaches(
-          delete_begin, delete_end,
-          filter_builder.IsEmptyBlacklist()
-              ? base::Callback<bool(const GURL&)>()
-              : filter,
+          delete_begin, delete_end, nullable_filter,
           CreatePendingTaskCompletionClosureForMojo());
     }
     storage_partition->ClearCodeCaches(