Apply templates to PersistedStateDB database

By using templates the database is reusable for different protocol
buffers. One application is for chrome cart which in the experimental
code looked very similar to persisted_state_db. By using templates we
can avoid duplicating a lot of code.

Bug: 1154499
Change-Id: I65bdaa3142eede152db26cd5629001b815f3b493
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570451Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Reviewed-by: Yue Zhang <yuezhanggg@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: David Maunder <davidjm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836924}

chrome/browser/BUILD.gn

@@ -1977,6 +1977,7 @@ static_library("browser") {
     "//chrome/browser/notifications/scheduler:factory",
     "//chrome/browser/notifications/scheduler/public",
     "//chrome/browser/persisted_state_db:persisted_state_db",
+    "//chrome/browser/persisted_state_db:persisted_state_db_content_proto",
     "//chrome/browser/policy:path_parser",
     "//chrome/browser/privacy_budget",
     "//chrome/browser/profiling_host",

chrome/browser/persisted_state_db/BUILD.gn

@@ -7,12 +7,18 @@ import("//third_party/protobuf/proto_library.gni")
 
 source_set("persisted_state_db") {
   sources = [
-    "persisted_state_db.cc",
-    "persisted_state_db.h",
-    "persisted_state_db_factory.cc",
-    "persisted_state_db_factory.h",
+    "profile_proto_db.h",
+    "profile_proto_db_factory.cc",
+    "profile_proto_db_factory.h",
   ]
 
+  if (is_android) {
+    sources += [
+      "persisted_state_db.cc",
+      "persisted_state_db.h",
+    ]
+  }
+
   deps = [
     ":persisted_state_db_content_proto",
     "//base:base",

chrome/browser/persisted_state_db/persisted_state_db.h

@@ -9,31 +9,21 @@
 #include <string>
 #include <vector>
 
+#include "base/android/scoped_java_ref.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/sequenced_task_runner.h"
+#include "build/build_config.h"
 #include "chrome/browser/persisted_state_db/persisted_state_db_content.pb.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/leveldb_proto/public/proto_database.h"
 
-#if defined(OS_ANDROID)
-
-#include "base/android/scoped_java_ref.h"
-#include "build/build_config.h"
-
-#endif  // OS_ANDROID
-
-namespace leveldb_proto {
-class ProtoDatabaseProvider;
-}  // namespace leveldb_proto
-
 namespace content {
 class BrowserContext;
 }  // namespace content
 
-class PersistedStateDBFactory;
-class PersistedStateDBFactoryTest;
-class PersistedStateDBTest;
+template <typename T>
+class ProfileProtoDB;
 
 // PersistedStateDB is leveldb backend store for NonCriticalPersistedTabData.
 // NonCriticalPersistedTabData is an extension of TabState where data for
@@ -42,42 +32,12 @@ class PersistedStateDBTest;
 // <NonCriticalPersistedTabData id>_<Tab id>
 
 // NonCriticalPersistedTabData is stored in key/value pairs.
-class PersistedStateDB : public KeyedService {
+class PersistedStateDB {
  public:
-  using KeyAndValue = std::pair<std::string, std::vector<uint8_t>>;
-
-  // Callback when content is acquired
-  using LoadCallback = base::OnceCallback<void(bool, std::vector<KeyAndValue>)>;
-
-  // Used for confirming an operation was completed successfully (e.g.
-  // insert, delete). This will be invoked on a different SequenceRunner
-  // to TabStateDB.
-  using OperationCallback = base::OnceCallback<void(bool)>;
-
-  // Entry in the database
-  using ContentEntry = leveldb_proto::ProtoDatabase<
-      persisted_state_db::PersistedStateContentProto>::KeyEntryVector;
-
-  ~PersistedStateDB() override;
-
-  // Loads the content data for the key and passes them to the callback
-  void LoadContent(const std::string& key, LoadCallback callback);
-
-  // Inserts a value for a given key and passes the result (success/failure) to
-  // OperationCallback
-  void InsertContent(const std::string& key,
-                     const std::vector<uint8_t>& value,
-                     OperationCallback callback);
-
-  // Deletes content in the database, matching all keys which have a prefix
-  // that matches the key
-  void DeleteContent(const std::string& key, OperationCallback callback);
-
-  // Delete all content in the database
-  void DeleteAllContent(OperationCallback callback);
-
-#if defined(OS_ANDROID)
-  // The following are callable from LevelDBPersistedTabDataStorage over JNI.
+  explicit PersistedStateDB(content::BrowserContext* browser_context);
+  PersistedStateDB(const PersistedStateDB&) = delete;
+  PersistedStateDB& operator=(const PersistedStateDB&) = delete;
+  ~PersistedStateDB();
 
   // Save byte array for key.
   void Save(JNIEnv* env,
@@ -98,62 +58,10 @@ class PersistedStateDB : public KeyedService {
   // Destroy PersistedStateDB object.
   void Destroy(JNIEnv* env);
 
-#endif  // OS_ANDROID
-
  private:
-  friend class ::PersistedStateDBTest;
-  friend class ::PersistedStateDBFactory;
-  friend class ::PersistedStateDBFactoryTest;
-
-  // Initializes the database
-  PersistedStateDB(
-      content::BrowserContext* browser_context,
-      leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
-      const base::FilePath& profile_directory);
-
-  // Used for tests
-  explicit PersistedStateDB(
-      std::unique_ptr<leveldb_proto::ProtoDatabase<
-          persisted_state_db::PersistedStateContentProto>> storage_database,
-      scoped_refptr<base::SequencedTaskRunner> task_runner);
-
-  // Passes back database status following database initialization
-  void OnDatabaseInitialized(leveldb_proto::Enums::InitStatus status);
-
-  // Callback when content is loaded
-  void OnLoadContent(
-      LoadCallback callback,
-      bool success,
-      std::unique_ptr<
-          std::vector<persisted_state_db::PersistedStateContentProto>> content);
-
-  // Callback when an operation (e.g. insert or delete) is called
-  void OnOperationCommitted(OperationCallback callback, bool success);
-
-  // Returns true if initialization status of database is not yet known
-  bool InitStatusUnknown() const;
-
-  // Returns true if the database failed to initialize
-  bool FailedToInit() const;
-
-  // Browser context associated with the PersistedStateDB
-  content::BrowserContext* browser_context_;
-
-  // Status of the database initialization.
-  base::Optional<leveldb_proto::Enums::InitStatus> database_status_;
-
-  // The database for storing content storage information.
-  std::unique_ptr<leveldb_proto::ProtoDatabase<
-      persisted_state_db::PersistedStateContentProto>>
-      storage_database_;
-
-  // Store operations until the database is initialized at which point
-  // deferred_operations_ is flushed and all operations are executed.
-  std::vector<base::OnceClosure> deferred_operations_;
+  ProfileProtoDB<persisted_state_db::PersistedStateContentProto>* proto_db_;
 
   base::WeakPtrFactory<PersistedStateDB> weak_ptr_factory_{this};
-
-  DISALLOW_COPY_AND_ASSIGN(PersistedStateDB);
 };
 
 #endif  // CHROME_BROWSER_PERSISTED_STATE_DB_PERSISTED_STATE_DB_H_

chrome/browser/persisted_state_db/persisted_state_db_factory.cc

-// Copyright 2020 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/persisted_state_db/persisted_state_db_factory.h"
-
-#include "chrome/browser/persisted_state_db/persisted_state_db.h"
-#include "components/keyed_service/content/browser_context_dependency_manager.h"
-#include "content/public/browser/browser_context.h"
-#include "content/public/browser/storage_partition.h"
-
-namespace {
-const char kPersistedStateDBFolder[] = "persisted_state_db";
-}  // namespace
-
-// static
-PersistedStateDBFactory* PersistedStateDBFactory::GetInstance() {
-  return base::Singleton<PersistedStateDBFactory>::get();
-}
-
-// static
-PersistedStateDB* PersistedStateDBFactory::GetForProfile(
-    content::BrowserContext* context) {
-  // Incognito is currently not supported
-  if (context->IsOffTheRecord())
-    return nullptr;
-
-  return static_cast<PersistedStateDB*>(
-      GetInstance()->GetServiceForBrowserContext(context, true));
-}
-
-PersistedStateDBFactory::PersistedStateDBFactory()
-    : BrowserContextKeyedServiceFactory(
-          "PersistedStateDBKeyedService",
-          BrowserContextDependencyManager::GetInstance()) {}
-
-PersistedStateDBFactory::~PersistedStateDBFactory() = default;
-
-KeyedService* PersistedStateDBFactory::BuildServiceInstanceFor(
-    content::BrowserContext* context) const {
-  DCHECK(!context->IsOffTheRecord());
-
-  leveldb_proto::ProtoDatabaseProvider* proto_database_provider =
-      content::BrowserContext::GetDefaultStoragePartition(context)
-          ->GetProtoDatabaseProvider();
-  base::FilePath tab_state_db_dir(
-      context->GetPath().AppendASCII(kPersistedStateDBFolder));
-  return new PersistedStateDB(context, proto_database_provider,
-                              tab_state_db_dir);
-}

chrome/browser/persisted_state_db/persisted_state_db_factory.h

-// Copyright 2020 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.
-
-#ifndef CHROME_BROWSER_PERSISTED_STATE_DB_PERSISTED_STATE_DB_FACTORY_H_
-#define CHROME_BROWSER_PERSISTED_STATE_DB_PERSISTED_STATE_DB_FACTORY_H_
-
-#include "base/memory/singleton.h"
-#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
-
-class KeyedService;
-class PersistedStateDB;
-
-// Factory to create on PersistedStateDB per profile. Incognito is currently
-// not supported and the factory will return nullptr for an incognito profile.
-class PersistedStateDBFactory : public BrowserContextKeyedServiceFactory {
- public:
-  // Acquire instance of PersistedStateDBFactory
-  static PersistedStateDBFactory* GetInstance();
-
-  // Acquire PersistedStateDB - there is one per profile.
-  static PersistedStateDB* GetForProfile(content::BrowserContext* context);
-
-  // Call the parent Disassocaite which is a protected method
-  void Disassociate(content::BrowserContext* context) {
-    BrowserContextKeyedServiceFactory::Disassociate(context);
-  }
-
- private:
-  friend struct base::DefaultSingletonTraits<PersistedStateDBFactory>;
-
-  PersistedStateDBFactory();
-  ~PersistedStateDBFactory() override;
-
-  KeyedService* BuildServiceInstanceFor(
-      content::BrowserContext* context) const override;
-};
-
-#endif  // CHROME_BROWSER_PERSISTED_STATE_DB_PERSISTED_STATE_DB_FACTORY_H_

chrome/browser/persisted_state_db/profile_proto_db_factory.cc

+// Copyright 2020 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/persisted_state_db/profile_proto_db_factory.h"
+
+#include "base/no_destructor.h"
+#include "chrome/browser/persisted_state_db/persisted_state_db_content.pb.h"
+
+ProfileProtoDBFactory<persisted_state_db::PersistedStateContentProto>*
+GetPersistedStateProfileProtoDBFactory() {
+  static base::NoDestructor<
+      ProfileProtoDBFactory<persisted_state_db::PersistedStateContentProto>>
+      instance;
+  return instance.get();
+}

chrome/browser/persisted_state_db/profile_proto_db_factory.h

+// Copyright 2020 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.
+
+#ifndef CHROME_BROWSER_PERSISTED_STATE_DB_PROFILE_PROTO_DB_FACTORY_H_
+#define CHROME_BROWSER_PERSISTED_STATE_DB_PROFILE_PROTO_DB_FACTORY_H_
+
+#include "chrome/browser/persisted_state_db/profile_proto_db.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/storage_partition.h"
+
+namespace {
+const char kPersistedStateDBFolder[] = "persisted_state_db";
+}  // namespace
+
+ProfileProtoDBFactory<persisted_state_db::PersistedStateContentProto>*
+GetPersistedStateProfileProtoDBFactory();
+
+// Factory to create a ProtoDB per profile and per proto. Incognito is
+// currently not supported and the factory will return nullptr for an incognito
+// profile.
+template <typename T>
+class ProfileProtoDBFactory : public BrowserContextKeyedServiceFactory {
+ public:
+  // Acquire instance of ProfileProtoDBFactory.
+  static ProfileProtoDBFactory<T>* GetInstance();
+
+  // Acquire ProtoDB - there is one per profile.
+  static ProfileProtoDB<T>* GetForProfile(content::BrowserContext* context);
+
+  // Call the parent Disassociate which is a protected method.
+  void Disassociate(content::BrowserContext* context) {
+    BrowserContextKeyedServiceFactory::Disassociate(context);
+  }
+
+ private:
+  friend class base::NoDestructor<ProfileProtoDBFactory<T>>;
+
+  ProfileProtoDBFactory();
+  ~ProfileProtoDBFactory() override;
+
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+};
+
+// static
+template <typename T>
+ProfileProtoDBFactory<T>* ProfileProtoDBFactory<T>::GetInstance() {
+  static_assert(
+      std::is_base_of<persisted_state_db::PersistedStateContentProto, T>::
+          value /** subsequent supported templates will be ORed in here */,
+      "Provided template is not supported. To support implement a factory "
+      "method similar to GetPersistedStateProfileProtoDBFactory and add to "
+      "list of "
+      "supported templates");
+  return GetPersistedStateProfileProtoDBFactory();
+}
+
+// static
+template <typename T>
+ProfileProtoDB<T>* ProfileProtoDBFactory<T>::GetForProfile(
+    content::BrowserContext* context) {
+  // Incognito is currently not supported
+  if (context->IsOffTheRecord())
+    return nullptr;
+
+  return static_cast<ProfileProtoDB<T>*>(
+      GetInstance()->GetServiceForBrowserContext(context, true));
+}
+
+template <typename T>
+ProfileProtoDBFactory<T>::ProfileProtoDBFactory()
+    : BrowserContextKeyedServiceFactory(
+          "ProfileProtoDBFactory",
+          BrowserContextDependencyManager::GetInstance()) {}
+
+template <typename T>
+ProfileProtoDBFactory<T>::~ProfileProtoDBFactory() = default;
+
+template <typename T>
+KeyedService* ProfileProtoDBFactory<T>::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  DCHECK(!context->IsOffTheRecord());
+
+  leveldb_proto::ProtoDatabaseProvider* proto_database_provider =
+      content::BrowserContext::GetDefaultStoragePartition(context)
+          ->GetProtoDatabaseProvider();
+
+  static_assert(
+      std::is_base_of<persisted_state_db::PersistedStateContentProto, T>::value,
+      "Provided template is not supported. To support add unique folder in the "
+      "below proto -> folder name mapping below this assert.");
+  // The following will become a proto -> dir and proto ->
+  // leveldb_proto::ProtoDbType mapping as more protos are added.
+  if (std::is_base_of<persisted_state_db::PersistedStateContentProto,
+                      T>::value) {
+    return new ProfileProtoDB<T>(
+        context, proto_database_provider,
+        context->GetPath().AppendASCII(kPersistedStateDBFolder),
+        leveldb_proto::ProtoDbType::PERSISTED_STATE_DATABASE);
+  } else {
+    // Must add in leveldb_proto::ProtoDbType and database directory folder for
+    // new protos
+    DCHECK(false);
+  }
+}
+
+#endif  // CHROME_BROWSER_PERSISTED_STATE_DB_PROFILE_PROTO_DB_FACTORY_H_

chrome/browser/persisted_state_db/persisted_state_db_factory_unittest.cc → chrome/browser/persisted_state_db/profile_proto_db_factory_unittest.cc

@@ -2,18 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/persisted_state_db/persisted_state_db_factory.h"
+#include "chrome/browser/persisted_state_db/profile_proto_db_factory.h"
 
 #include <memory>
 
 #include "base/files/scoped_temp_dir.h"
+#include "chrome/browser/persisted_state_db/persisted_state_db_content.pb.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-class PersistedStateDBFactoryTest : public testing::Test {
+class ProfileProtoDBFactoryTest : public testing::Test {
  public:
-  PersistedStateDBFactoryTest() = default;
+  ProfileProtoDBFactoryTest() = default;
+  ProfileProtoDBFactoryTest(const ProfileProtoDBFactoryTest&) = delete;
+  ProfileProtoDBFactoryTest& operator=(const ProfileProtoDBFactoryTest&) =
+      delete;
 
   void SetUp() override {
     ASSERT_TRUE(profile_dir_.CreateUniqueTempDir());
@@ -37,22 +41,29 @@ class PersistedStateDBFactoryTest : public testing::Test {
   content::BrowserTaskEnvironment task_environment_;
   std::unique_ptr<TestingProfile> profile_;
   std::unique_ptr<TestingProfile> different_profile_;
-
-  DISALLOW_COPY_AND_ASSIGN(PersistedStateDBFactoryTest);
 };
 
-TEST_F(PersistedStateDBFactoryTest, TestIncognitoProfile) {
-  EXPECT_EQ(nullptr, PersistedStateDBFactory::GetInstance()->GetForProfile(
-                         profile()->GetPrimaryOTRProfile()));
+TEST_F(ProfileProtoDBFactoryTest, TestIncognitoProfile) {
+  EXPECT_EQ(nullptr,
+            ProfileProtoDBFactory<
+                persisted_state_db::PersistedStateContentProto>::GetInstance()
+                ->GetForProfile(profile()->GetPrimaryOTRProfile()));
 }
 
-TEST_F(PersistedStateDBFactoryTest, TestSameProfile) {
-  EXPECT_EQ(PersistedStateDBFactory::GetInstance()->GetForProfile(profile()),
-            PersistedStateDBFactory::GetInstance()->GetForProfile(profile()));
+TEST_F(ProfileProtoDBFactoryTest, TestSameProfile) {
+  EXPECT_EQ(ProfileProtoDBFactory<
+                persisted_state_db::PersistedStateContentProto>::GetInstance()
+                ->GetForProfile(profile()),
+            ProfileProtoDBFactory<
+                persisted_state_db::PersistedStateContentProto>::GetInstance()
+                ->GetForProfile(profile()));
 }
 
-TEST_F(PersistedStateDBFactoryTest, TestDifferentProfile) {
-  EXPECT_NE(PersistedStateDBFactory::GetInstance()->GetForProfile(
-                different_profile()),
-            PersistedStateDBFactory::GetInstance()->GetForProfile(profile()));
+TEST_F(ProfileProtoDBFactoryTest, TestDifferentProfile) {
+  EXPECT_NE(ProfileProtoDBFactory<
+                persisted_state_db::PersistedStateContentProto>::GetInstance()
+                ->GetForProfile(different_profile()),
+            ProfileProtoDBFactory<
+                persisted_state_db::PersistedStateContentProto>::GetInstance()
+                ->GetForProfile(profile()));
 }

chrome/browser/persisted_state_db/profile_proto_db_test_proto.proto

+// Copyright 2020 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.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+package profile_proto_db;
+
+// Proto used for testing purposes.
+message ProfileProtoDBTestProto {
+  // A field
+  optional string key = 1;
+
+  // Another field
+  optional int32 b = 2;
+}

chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

@@ -54,7 +54,8 @@
 #include "chrome/browser/permissions/adaptive_quiet_notification_permission_ui_enabler.h"
 #include "chrome/browser/permissions/last_tab_standing_tracker_factory.h"
 #include "chrome/browser/permissions/permission_auditing_service_factory.h"
-#include "chrome/browser/persisted_state_db/persisted_state_db_factory.h"
+#include "chrome/browser/persisted_state_db/persisted_state_db_content.pb.h"
+#include "chrome/browser/persisted_state_db/profile_proto_db_factory.h"
 #include "chrome/browser/plugins/plugin_prefs_factory.h"
 #include "chrome/browser/policy/cloud/user_cloud_policy_invalidator_factory.h"
 #include "chrome/browser/predictors/autocomplete_action_predictor_factory.h"
@@ -343,7 +344,8 @@ void ChromeBrowserMainExtraPartsProfiles::
 #endif
   PasswordStoreFactory::GetInstance();
   PermissionAuditingServiceFactory::GetInstance();
-  PersistedStateDBFactory::GetInstance();
+  ProfileProtoDBFactory<
+      persisted_state_db::PersistedStateContentProto>::GetInstance();
 #if !defined(OS_ANDROID)
   PinnedTabServiceFactory::GetInstance();
 #endif

chrome/test/BUILD.gn

@@ -3610,8 +3610,8 @@ test("unit_tests") {
     "../browser/permissions/permission_context_base_feature_policy_unittest.cc",
     "../browser/permissions/prediction_based_permission_ui_selector_unittest.cc",
     "../browser/permissions/pref_notification_permission_ui_selector_unittest.cc",
-    "../browser/persisted_state_db/persisted_state_db_factory_unittest.cc",
-    "../browser/persisted_state_db/persisted_state_db_unittest.cc",
+    "../browser/persisted_state_db/profile_proto_db_factory_unittest.cc",
+    "../browser/persisted_state_db/profile_proto_db_unittest.cc",
     "../browser/plugins/pdf_iframe_navigation_throttle_unittest.cc",
     "../browser/policy/boolean_disabling_policy_handler_unittest.cc",
     "../browser/policy/chrome_browser_policy_connector_unittest.cc",
@@ -4148,6 +4148,7 @@ test("unit_tests") {
 
   deps = [
     ":messaging_test_proto",
+    ":profile_proto_db_test_proto",
     ":test_support",
     ":test_support_unit",
     "//base:base_stack_sampling_profiler_test_util",
@@ -7726,3 +7727,8 @@ if (!is_android) {
 proto_library("messaging_test_proto") {
   sources = [ "//chrome/browser/policy/messaging_layer/proto/test.proto" ]
 }
+
+proto_library("profile_proto_db_test_proto") {
+  sources =
+      [ "../browser/persisted_state_db/profile_proto_db_test_proto.proto" ]
+}