Removed per database params in leveldb_proto experiment

This CL removes the option of setting up migrations for each database
individually, now if the migration experiment is on all databases will
be migrated (except by those in a blocklist).

This makes it simpler to launch the experiment to stable, as we will no
longer need to specify each database name on the experiment params.

Bug: 870813
Change-Id: I00259f5ceeae14790a40546d9fab064eac1aea29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630734Reviewed-by: ssid <ssid@chromium.org>
Commit-Queue: Salvador Guerrero <salg@google.com>
Cr-Commit-Position: refs/heads/master@{#843846}

components/leveldb_proto/internal/shared_proto_database_client_list_unittest.cc

@@ -12,21 +12,12 @@
 
 namespace leveldb_proto {
 
-namespace {
-const char kTestClientName[] = "TestDatabase1";
-}
 
 class SharedProtoDatabaseClientListTest : public testing::Test {
  public:
-  void SetUpExperimentParam(std::string key, std::string value) {
-    std::map<std::string, std::string> params = {
-        {"migrate_TestDatabase0", "true"},
-        {"migrate_" + key, value},
-        {"migrate_TestDatabase2", "false"},
-    };
-
-    scoped_feature_list_.InitAndEnableFeatureWithParameters(
-        kProtoDBSharedMigration, params);
+  void SetUpExperiment(bool isExperimentOn) {
+    scoped_feature_list_.InitWithFeatureState(kProtoDBSharedMigration,
+                                              isExperimentOn);
   }
 
  private:
@@ -34,8 +25,8 @@ class SharedProtoDatabaseClientListTest : public testing::Test {
 };
 
 TEST_F(SharedProtoDatabaseClientListTest, ShouldUseSharedDBTest) {
-  // Parameter value is case sensitive
-  SetUpExperimentParam(kTestClientName, "true");
+  // Set experiment to use Shared DB.
+  SetUpExperiment(true);
 
   bool use_shared = SharedProtoDatabaseClientList::ShouldUseSharedDB(
       ProtoDbType::TEST_DATABASE1);
@@ -44,8 +35,9 @@ TEST_F(SharedProtoDatabaseClientListTest, ShouldUseSharedDBTest) {
 }
 
 TEST_F(SharedProtoDatabaseClientListTest,
-       ShouldUseSharedDBTest_OnlyWhenParamMatchesName) {
-  SetUpExperimentParam("TestDatabase10", "true");
+       ShouldUseSharedDBTest_OnlyWhenExperimentIsOn) {
+  // Set experiment to use Unique DB.
+  SetUpExperiment(false);
 
   bool use_shared = SharedProtoDatabaseClientList::ShouldUseSharedDB(
       ProtoDbType::TEST_DATABASE1);
@@ -54,11 +46,13 @@ TEST_F(SharedProtoDatabaseClientListTest,
 }
 
 TEST_F(SharedProtoDatabaseClientListTest,
-       ShouldUseSharedDBTest_OnlyWhenParamValueIsTrue) {
-  SetUpExperimentParam(kTestClientName, "false");
+       ShouldUseSharedDBTest_ExceptIfDbIsBlocklisted) {
+  SetUpExperiment(true);
 
+  // GCM_KEY_STORE is blocklisted, it won't use a shared DB, regardless of
+  // experiment state.
   bool use_shared = SharedProtoDatabaseClientList::ShouldUseSharedDB(
-      ProtoDbType::TEST_DATABASE1);
+      ProtoDbType::GCM_KEY_STORE);
 
   ASSERT_FALSE(use_shared);
 }

components/leveldb_proto/public/shared_proto_database_client_list.cc

@@ -16,9 +16,6 @@
 
 namespace leveldb_proto {
 
-namespace {
-const char* const kDBNameParamPrefix = "migrate_";
-}  // namespace
 
 // static
 std::string SharedProtoDatabaseClientList::ProtoDbTypeToString(
@@ -100,18 +97,15 @@ std::string SharedProtoDatabaseClientList::ProtoDbTypeToString(
 
 // static
 bool SharedProtoDatabaseClientList::ShouldUseSharedDB(ProtoDbType db_type) {
-  for (size_t i = 0; kWhitelistedDbForSharedImpl[i] != ProtoDbType::LAST; ++i) {
-    if (kWhitelistedDbForSharedImpl[i] == db_type)
-      return true;
+  for (size_t i = 0; kBlocklistedDbForSharedImpl[i] != ProtoDbType::LAST; ++i) {
+    if (kBlocklistedDbForSharedImpl[i] == db_type)
+      return false;
   }
 
   if (!base::FeatureList::IsEnabled(kProtoDBSharedMigration))
     return false;
 
-  std::string name =
-      SharedProtoDatabaseClientList::ProtoDbTypeToString(db_type);
-  return base::GetFieldTrialParamByFeatureAsBool(
-      kProtoDBSharedMigration, kDBNameParamPrefix + name, false);
+  return true;
 }
 
 }  // namespace leveldb_proto

components/leveldb_proto/public/shared_proto_database_client_list.h

@@ -59,21 +59,15 @@ enum class ProtoDbType {
   LAST,
 };
 
-// List of databases that were introduced after shared db implementation was
-// created. These will be forced to use shared database implementation.
-constexpr ProtoDbType kWhitelistedDbForSharedImpl[]{
-    ProtoDbType::NOTIFICATION_SCHEDULER_ICON_STORE,
-    ProtoDbType::NOTIFICATION_SCHEDULER_IMPRESSION_STORE,
-    ProtoDbType::NOTIFICATION_SCHEDULER_NOTIFICATION_STORE,
-    ProtoDbType::PRINT_JOB_DATABASE,
-    ProtoDbType::FEED_STREAM_DATABASE,
-    ProtoDbType::PERSISTED_STATE_DATABASE,
-    ProtoDbType::UPBOARDING_QUERY_TILE_STORE,
-    ProtoDbType::NEARBY_SHARE_PUBLIC_CERTIFICATE_DATABASE,
-    ProtoDbType::VIDEO_TUTORIALS_DATABASE,
-    ProtoDbType::FEED_KEY_VALUE_DATABASE,
-    ProtoDbType::CART_DATABASE,
-    ProtoDbType::LAST,  // Marks the end of list.
+// List of databases that need to keep using unique db instances. New databases
+// shouldn't be here unless they have a good reason.
+constexpr ProtoDbType kBlocklistedDbForSharedImpl[]{
+    // DB is not tied to a profile, will always be unique.
+    ProtoDbType::GCM_KEY_STORE,
+    // DB Used by shared database, will always be unique.
+    ProtoDbType::SHARED_DB_METADATA,
+    // Marks the end of list.
+    ProtoDbType::LAST,
 };
 
 // Add any obsolete databases in this list so that, if the data is no longer