Convert RemoveDataMask from enum to pointers and split it between content and embedder

chrome/browser/browsing_data/browsing_data_remover_impl.cc

Index: chrome/browser/browsing_data/browsing_data_remover_impl.cc
diff --git a/chrome/browser/browsing_data/browsing_data_remover_impl.cc b/chrome/browser/browsing_data/browsing_data_remover_impl.cc
index dd3a924c22ca7cd1b62fe8b9a5a4e2d848e91ae6..d822f6d12cf5914507673f467f8d7a0df5e0f0e1 100644
--- a/chrome/browser/browsing_data/browsing_data_remover_impl.cc
+++ b/chrome/browser/browsing_data/browsing_data_remover_impl.cc
@@ -46,6 +46,7 @@
 using base::UserMetricsAction;
 using content::BrowserContext;
 using content::BrowserThread;
+using content::BrowsingDataType;
 using content::BrowsingDataFilterBuilder;
 using content::DOMStorageContext;
 
@@ -117,6 +118,20 @@ void ClearChannelIDsOnIOThread(
                  base::RetainedRef(std::move(rq_context)), callback));
 }
 
+bool IsFilterable(const std::set<const BrowsingDataType*>& remove_mask) {
+  for (const BrowsingDataType* data_type : remove_mask) {
+    if (data_type && !data_type->filterable)

[Bernhard Bauer, 2017/02/17 11:10:43]

Why the null check?

[msramek, 2017/02/17 18:00:20]

In browsing_data_api.cc, there is one place where we need a "datatype or
nothing", which I replaced with "BrowsingDataType non-null or null".

I rewrote that part a bit so that we can DCHECK here.

+      return false;
+  }
+
+  return true;
+}
+
+inline bool MaskContains(const std::set<const BrowsingDataType*>& remove_mask,

[Bernhard Bauer, 2017/02/17 11:10:43]

You could use base::ContainsValue (from base/stl_util.h) instead of this.

[msramek, 2017/02/17 18:00:20]

Done. Thanks!

+                         const BrowsingDataType* data_type) {
+  return remove_mask.find(data_type) != remove_mask.end();
+}
+
 }  // namespace
 
 BrowsingDataRemoverImpl::CompletionInhibitor*
@@ -152,7 +167,6 @@ void BrowsingDataRemoverImpl::SubTask::CompletionCallback() {
 BrowsingDataRemoverImpl::BrowsingDataRemoverImpl(
     content::BrowserContext* browser_context)
     : browser_context_(browser_context),
-      remove_mask_(-1),
       origin_type_mask_(-1),
       is_removing_(false),
       sub_task_forward_callback_(
@@ -204,10 +218,11 @@ BrowsingDataRemoverImpl::GetEmbedderDelegate() const {
   return embedder_delegate_.get();
 }
 
-void BrowsingDataRemoverImpl::Remove(const base::Time& delete_begin,
-                                 const base::Time& delete_end,
-                                 int remove_mask,
-                                 int origin_type_mask) {
+void BrowsingDataRemoverImpl::Remove(
+    const base::Time& delete_begin,
+    const base::Time& delete_end,
+    const std::set<const BrowsingDataType*>& remove_mask,
+    int origin_type_mask) {
   RemoveInternal(delete_begin, delete_end, remove_mask, origin_type_mask,
                  std::unique_ptr<BrowsingDataFilterBuilder>(), nullptr);
 }
@@ -215,7 +230,7 @@ void BrowsingDataRemoverImpl::Remove(const base::Time& delete_begin,
 void BrowsingDataRemoverImpl::RemoveAndReply(
     const base::Time& delete_begin,
     const base::Time& delete_end,
-    int remove_mask,
+    const std::set<const BrowsingDataType*>& remove_mask,
     int origin_type_mask,
     Observer* observer) {
   DCHECK(observer);
@@ -226,10 +241,10 @@ void BrowsingDataRemoverImpl::RemoveAndReply(
 void BrowsingDataRemoverImpl::RemoveWithFilter(
     const base::Time& delete_begin,
     const base::Time& delete_end,
-    int remove_mask,
+    const std::set<const BrowsingDataType*>& remove_mask,
     int origin_type_mask,
     std::unique_ptr<BrowsingDataFilterBuilder> filter_builder) {
-  DCHECK_EQ(0, remove_mask & ~FILTERABLE_DATATYPES);
+  DCHECK(IsFilterable(remove_mask));
   DCHECK(filter_builder);
   RemoveInternal(delete_begin, delete_end, remove_mask, origin_type_mask,
                  std::move(filter_builder), nullptr);
@@ -238,11 +253,11 @@ void BrowsingDataRemoverImpl::RemoveWithFilter(
 void BrowsingDataRemoverImpl::RemoveWithFilterAndReply(
     const base::Time& delete_begin,
     const base::Time& delete_end,
-    int remove_mask,
+    const std::set<const BrowsingDataType*>& remove_mask,
     int origin_type_mask,
     std::unique_ptr<BrowsingDataFilterBuilder> filter_builder,
     Observer* observer) {
-  DCHECK_EQ(0, remove_mask & ~FILTERABLE_DATATYPES);
+  DCHECK(IsFilterable(remove_mask));
   DCHECK(filter_builder);
   DCHECK(observer);
   RemoveInternal(delete_begin, delete_end, remove_mask, origin_type_mask,
@@ -252,7 +267,7 @@ void BrowsingDataRemoverImpl::RemoveWithFilterAndReply(
 void BrowsingDataRemoverImpl::RemoveInternal(
     const base::Time& delete_begin,
     const base::Time& delete_end,
-    int remove_mask,
+    const std::set<const BrowsingDataType*>& remove_mask,
     int origin_type_mask,
     std::unique_ptr<BrowsingDataFilterBuilder> filter_builder,
     Observer* observer) {
@@ -299,7 +314,7 @@ void BrowsingDataRemoverImpl::RunNextTask() {
 void BrowsingDataRemoverImpl::RemoveImpl(
     const base::Time& delete_begin,
     const base::Time& delete_end,
-    int remove_mask,
+    const std::set<const BrowsingDataType*>& remove_mask,
     const BrowsingDataFilterBuilder& filter_builder,
     int origin_type_mask) {
   // =============== README before adding more storage backends ===============
@@ -345,11 +360,11 @@ void BrowsingDataRemoverImpl::RemoveImpl(
 
   // Record the combined deletion of cookies and cache.
   CookieOrCacheDeletionChoice choice = NEITHER_COOKIES_NOR_CACHE;
-  if (remove_mask & REMOVE_COOKIES &&
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeCookies) &&
       origin_type_mask_ & BrowsingDataHelper::UNPROTECTED_WEB) {
-    choice = remove_mask & REMOVE_CACHE ? BOTH_COOKIES_AND_CACHE
-                                        : ONLY_COOKIES;
-  } else if (remove_mask & REMOVE_CACHE) {
+    choice = MaskContains(remove_mask, &content::kBrowsingDataTypeCache)
+        ? BOTH_COOKIES_AND_CACHE : ONLY_COOKIES;
+  } else if (MaskContains(remove_mask, &content::kBrowsingDataTypeCache)) {
     choice = ONLY_CACHE;
   }
 
@@ -372,8 +387,9 @@ void BrowsingDataRemoverImpl::RemoveImpl(
       filter_builder.BuildGeneralFilter();
 
   //////////////////////////////////////////////////////////////////////////////
-  // REMOVE_DOWNLOADS
-  if ((remove_mask & REMOVE_DOWNLOADS) && may_delete_history) {
+  // kBrowsingDataTypeDownloads
+  if ((MaskContains(remove_mask, &content::kBrowsingDataTypeDownloads))
+          && may_delete_history) {
     content::RecordAction(UserMetricsAction("ClearBrowsingData_Downloads"));
     content::DownloadManager* download_manager =
         BrowserContext::GetDownloadManager(browser_context_);
@@ -382,10 +398,10 @@ void BrowsingDataRemoverImpl::RemoveImpl(
   }
 
   //////////////////////////////////////////////////////////////////////////////
-  // REMOVE_CHANNEL_IDS
+  // kBrowsingDataTypeChannelIDs
   // Channel IDs are not separated for protected and unprotected web
   // origins. We check the origin_type_mask_ to prevent unintended deletion.
-  if (remove_mask & REMOVE_CHANNEL_IDS &&
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeChannelIDs) &&
       origin_type_mask_ & BrowsingDataHelper::UNPROTECTED_WEB) {
     content::RecordAction(
         UserMetricsAction("ClearBrowsingData_ChannelIDs"));
@@ -411,36 +427,36 @@ void BrowsingDataRemoverImpl::RemoveImpl(
   // don't accidentally remove the cookies that are associated with the
   // UNPROTECTED_WEB origin. This is necessary because cookies are not separated
   // between UNPROTECTED_WEB and PROTECTED_WEB.
-  if (remove_mask & REMOVE_COOKIES &&
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeCookies) &&
       origin_type_mask_ & BrowsingDataHelper::UNPROTECTED_WEB) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_COOKIES;
   }
-  if (remove_mask & REMOVE_LOCAL_STORAGE) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeLocalStorage)) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_LOCAL_STORAGE;
   }
-  if (remove_mask & REMOVE_INDEXEDDB) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeIndexedDB)) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_INDEXEDDB;
   }
-  if (remove_mask & REMOVE_WEBSQL) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeWebSQL)) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_WEBSQL;
   }
-  if (remove_mask & REMOVE_APPCACHE) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeAppCache)) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_APPCACHE;
   }
-  if (remove_mask & REMOVE_SERVICE_WORKERS) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeServiceWorkers)) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_SERVICE_WORKERS;
   }
-  if (remove_mask & REMOVE_CACHE_STORAGE) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeCacheStorage)) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_CACHE_STORAGE;
   }
-  if (remove_mask & REMOVE_FILE_SYSTEMS) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeFileSystems)) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_FILE_SYSTEMS;
   }
@@ -448,7 +464,7 @@ void BrowsingDataRemoverImpl::RemoveImpl(
   // Content Decryption Modules used by Encrypted Media store licenses in a
   // private filesystem. These are different than content licenses used by
   // Flash (which are deleted father down in this method).
-  if (remove_mask & REMOVE_MEDIA_LICENSES) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeMediaLicenses)) {
     storage_partition_remove_mask |=
         content::StoragePartition::REMOVE_DATA_MASK_PLUGIN_PRIVATE_DATA;
   }
@@ -493,8 +509,8 @@ void BrowsingDataRemoverImpl::RemoveImpl(
   }
 
   //////////////////////////////////////////////////////////////////////////////
-  // CACHE
-  if (remove_mask & REMOVE_CACHE) {
+  // kBrowsingDataTypeCache
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeCache)) {
     // Tell the renderers to clear their cache.
     web_cache::WebCacheManager::GetInstance()->ClearCache();
 
@@ -523,7 +539,7 @@ void BrowsingDataRemoverImpl::RemoveImpl(
 
   //////////////////////////////////////////////////////////////////////////////
   // Auth cache.
-  if (remove_mask & REMOVE_COOKIES) {
+  if (MaskContains(remove_mask, &content::kBrowsingDataTypeCookies)) {
     scoped_refptr<net::URLRequestContextGetter> request_context =
         BrowserContext::GetDefaultStoragePartition(browser_context_)
             ->GetURLRequestContext();
@@ -573,7 +589,8 @@ const base::Time& BrowsingDataRemoverImpl::GetLastUsedEndTime() {
   return delete_end_;
 }
 
-int BrowsingDataRemoverImpl::GetLastUsedRemovalMask() {
+const std::set<const BrowsingDataType*>&
+BrowsingDataRemoverImpl::GetLastUsedRemovalMask() {
   return remove_mask_;
 }
 
@@ -584,7 +601,7 @@ int BrowsingDataRemoverImpl::GetLastUsedOriginTypeMask() {
 BrowsingDataRemoverImpl::RemovalTask::RemovalTask(
     const base::Time& delete_begin,
     const base::Time& delete_end,
-    int remove_mask,
+    const std::set<const BrowsingDataType*>& remove_mask,
     int origin_type_mask,
     std::unique_ptr<BrowsingDataFilterBuilder> filter_builder,
     Observer* observer)