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

chrome/browser/browsing_data/chrome_browsing_data_types.cc

Index: chrome/browser/browsing_data/chrome_browsing_data_types.cc
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_types.cc b/chrome/browser/browsing_data/chrome_browsing_data_types.cc
new file mode 100644
index 0000000000000000000000000000000000000000..43c8e6f4867f5b46082470d3823637c0c5638de7
--- /dev/null
+++ b/chrome/browser/browsing_data/chrome_browsing_data_types.cc
@@ -0,0 +1,108 @@
+// Copyright 2017 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/chrome_browsing_data_types.h"
+
+#include <set>
+
+#include "base/lazy_instance.h"
+
+////////////////////////////////////////////////////////////////////////////////
+// Chrome-specific datatypes.
+
+const content::BrowsingDataType kBrowsingDataTypeHistory {"history", false};
+
+const content::BrowsingDataType kBrowsingDataTypeFormData {"form data", false};
+
+const content::BrowsingDataType kBrowsingDataTypePasswords {"passwords", true};
+
+const content::BrowsingDataType kBrowsingDataTypePluginData {
+  "plugin data", true
+};
+
+#if defined(OS_ANDROID)
+const content::BrowsingDataType kBrowsingDataTypeWebappData {
+  "webapp data", true
+};
+#endif
+
+const content::BrowsingDataType kBrowsingDataTypeSiteUsageData {
+  "site usage data", true
+};
+
+const content::BrowsingDataType kBrowsingDataTypeDurablePermission {
+  "durable permission", true
+};
+
+const content::BrowsingDataType kBrowsingDataTypeHostedAppDataTestOnly {
+  "hosted app data (test only)", true
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// Group datatypes.
+
+namespace {
+
+class GroupDataTypes {
+ public:
+  GroupDataTypes() {
+    // kBrowsingDataTypeSetSiteData
+    site_data = {
+        &kBrowsingDataTypeAppCache,
+        &kBrowsingDataTypeCookies,
+        &kBrowsingDataTypeFileSystems,
+        &kBrowsingDataTypeIndexedDB,
+        &kBrowsingDataTypeLocalStorage,
+        &kBrowsingDataTypePluginData,
+        &kBrowsingDataTypeServiceWorkers,
+        &kBrowsingDataTypeCacheStorage,
+        &kBrowsingDataTypeWebSQL,
+        &kBrowsingDataTypeChannelIDs,
+#if defined(OS_ANDROID)
+        &kBrowsingDataTypeWebappData,
+#endif
+        &kBrowsingDataTypeSiteUsageData,
+        &kBrowsingDataTypeDurablePermission
+    };
+
+    // kBrowsingDataTypeSetImportantSites
+    important_sites = site_data;
+    important_sites.insert(&kBrowsingDataTypeCache);
+
+    // kBrowsingDataTypeSetAll
+    all = important_sites;
+    all.insert(&kBrowsingDataTypeDownloads);
+    all.insert(&kBrowsingDataTypeFormData);
+    all.insert(&kBrowsingDataTypeHistory);
+    all.insert(&kBrowsingDataTypePasswords);
+
+    // kBrowsingDataTypeSetWipeProfile
+    wipe_profile = all;
+    wipe_profile.insert(&kBrowsingDataTypeNoChecks);
+  }
+
+  ~GroupDataTypes() {}
+
+  std::set<const content::BrowsingDataType*> site_data;
+  std::set<const content::BrowsingDataType*> important_sites;
+  std::set<const content::BrowsingDataType*> all;
+  std::set<const content::BrowsingDataType*> wipe_profile;
+};
+
+static base::LazyInstance<GroupDataTypes>::Leaky g_group_data_types =
+    LAZY_INSTANCE_INITIALIZER;
+
+}  // namespace
+
+const std::set<const content::BrowsingDataType*>& kBrowsingDataTypeSetSiteData =

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

So, what you are saying is that this does not create static initializer code due
to the magic of C++11?

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

No, I totally misspoke here, sorry.

I wanted to praise C++11 for its list initializers, not static initializers,
i.e. the ability to use braces to construct a set. I updated the CL description.

These things here are definitely static. Note also that I'm using LazyInstance
pretty much only for code style purposes. I considered these approaches:

a) CR_DEFINE_STATIC_LOCAL should not be used because this is not local, and it
also doesn't allow complicated (non-constexpr) definitions.

b)

namespace {
std::set<>* all = new ...;
}
kBrowsingDataTypeAll = *all;

But random "new" operators also don't seem to be in line with the code style and
sanitizers are probably not going to like them.

c) base::LazyInstance seems to be the preferred way to go about static
initializations, but in our case it's not lazy; Get() is called right below.

I'm happy to hear your thoughts if there's a better way :) I don't have much
experience with this.

[Bernhard Bauer, 2017/02/20 14:19:16]

So, primarily my problem with this right now is that it does create static
initializer code, which is disallowed, and AFAIK checked by at least one bot
(not all of them though, as it requires instrumentation or fiddling with clang
or something, so this might not show up immediately). I think there is going to
be no way to get a static instance of a std::set that you can just access as a
constant without the compiler creating these initializers. Your options are:
1) Writing methods that return these -- essentially, wrap these lines into a
method each, which will take care of the initialization the first time they're
called.
2) Only expose the LazyInstance to clients and have them get the sets from
there. You could also use a separate LazyInstance for each of them.

Out of those options, I would probably prefer the first one. 

[msramek, 2017/02/20 17:11:16]

Done. I changed them to methods.

My original goal is to have them as constants so that they're similar to
individual datatypes, but that's actually probably not even a good thing. No
need to copy the old world.

+    g_group_data_types.Get().site_data;
+
+const std::set<const content::BrowsingDataType*>&
+kBrowsingDataTypeSetImportantSites = g_group_data_types.Get().important_sites;
+
+const std::set<const content::BrowsingDataType*>& kBrowsingDataTypeSetAll =
+    g_group_data_types.Get().all;
+
+const std::set<const content::BrowsingDataType*>&
+kBrowsingDataTypeSetWipeProfile = g_group_data_types.Get().wipe_profile;