Add KeyedService for TabStateDB

The TabState databases persists tab metadata across restarts. The
TabState database needs to be per profile. This is achieved using
KeyedService.

Bug: 1061258
Change-Id: If9841b8a815483f8e3cc5bc088136d64da001856
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2138496
Commit-Queue: David Maunder <davidjm@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759843}

chrome/browser/BUILD.gn

@@ -1801,6 +1801,8 @@ jumbo_static_library("browser") {
     "sync/user_event_service_factory.h",
     "tab/state/tab_state_db.cc",
     "tab/state/tab_state_db.h",
+    "tab/state/tab_state_db_factory.cc",
+    "tab/state/tab_state_db_factory.h",
     "tab_contents/navigation_metrics_recorder.cc",
     "tab_contents/navigation_metrics_recorder.h",
     "tab_contents/tab_util.cc",

chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

@@ -77,6 +77,7 @@
 #include "chrome/browser/sync/model_type_store_service_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/user_event_service_factory.h"
+#include "chrome/browser/tab/state/tab_state_db_factory.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/translate/translate_ranker_factory.h"
 #include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h"
@@ -351,6 +352,7 @@ void ChromeBrowserMainExtraPartsProfiles::
   SupervisedUserServiceFactory::GetInstance();
 #endif
   TabRestoreServiceFactory::GetInstance();
+  TabStateDBFactory::GetInstance();
   TemplateURLFetcherFactory::GetInstance();
   TemplateURLServiceFactory::GetInstance();
 #if !defined(OS_ANDROID)

chrome/browser/tab/state/tab_state_db.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/tab/state/tab_state_db.h"
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/strings/string_util.h"
 #include "base/task/post_task.h"
@@ -27,23 +28,6 @@ bool DatabasePrefixFilter(const std::string& key_prefix,
 
 }  // namespace
 
-TabStateDB::TabStateDB(
-    leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
-    const base::FilePath& profile_directory,
-    base::OnceClosure closure)
-    : database_status_(leveldb_proto::Enums::InitStatus::kNotInitialized),
-      storage_database_(
-          proto_database_provider->GetDB<tab_state_db::TabStateContentProto>(
-              leveldb_proto::ProtoDbType::TAB_STATE_DATABASE,
-              profile_directory.AppendASCII(kTabStateDBFolder),
-              base::CreateSequencedTaskRunner(
-                  {base::ThreadPool(), base::MayBlock(),
-                   base::TaskPriority::USER_VISIBLE}))) {
-  storage_database_->Init(base::BindOnce(&TabStateDB::OnDatabaseInitialized,
-                                         weak_ptr_factory_.GetWeakPtr(),
-                                         std::move(closure)));
-}
-
 TabStateDB::~TabStateDB() = default;
 
 bool TabStateDB::IsInitialized() const {
@@ -85,6 +69,22 @@ void TabStateDB::DeleteAllContent(OperationCallback callback) {
   storage_database_->Destroy(std::move(callback));
 }
 
+TabStateDB::TabStateDB(
+    leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
+    const base::FilePath& profile_directory)
+    : database_status_(leveldb_proto::Enums::InitStatus::kNotInitialized),
+      storage_database_(
+          proto_database_provider->GetDB<tab_state_db::TabStateContentProto>(
+              leveldb_proto::ProtoDbType::TAB_STATE_DATABASE,
+              profile_directory.AppendASCII(kTabStateDBFolder),
+              base::CreateSequencedTaskRunner(
+                  {base::ThreadPool(), base::MayBlock(),
+                   base::TaskPriority::USER_VISIBLE}))) {
+  storage_database_->Init(base::BindOnce(&TabStateDB::OnDatabaseInitialized,
+                                         weak_ptr_factory_.GetWeakPtr(),
+                                         base::DoNothing::Once()));
+}
+
 TabStateDB::TabStateDB(
     std::unique_ptr<leveldb_proto::ProtoDatabase<
         tab_state_db::TabStateContentProto>> storage_database,

chrome/browser/tab/state/tab_state_db.h

@@ -11,12 +11,15 @@
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "chrome/browser/tab/state/tab_state_db_content.pb.h"
+#include "components/keyed_service/core/keyed_service.h"
 #include "components/leveldb_proto/public/proto_database.h"
 
 namespace leveldb_proto {
 class ProtoDatabaseProvider;
 }  // namespace leveldb_proto
 
+class TabStateDBFactory;
+class TabStateDBFactoryTest;
 class TabStateDBTest;
 
 // TabStateDatabase is leveldb backend store for NonCriticalPersistedTabData.
@@ -26,8 +29,7 @@ class TabStateDBTest;
 // <NonCriticalPersistedTabData id>_<Tab id>
 
 // NonCriticalPersistedTabData is stored in key/value pairs.
-// TODO(crbug.com/1061258) add a KeyedService so TabStateDB is per profile
-class TabStateDB {
+class TabStateDB : public KeyedService {
  public:
   using KeyAndValue = std::pair<std::string, std::vector<uint8_t>>;
 
@@ -43,12 +45,7 @@ class TabStateDB {
   using ContentEntry = leveldb_proto::ProtoDatabase<
       tab_state_db::TabStateContentProto>::KeyEntryVector;
 
-  // Initializes the database
-  TabStateDB(leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
-             const base::FilePath& profile_directory,
-             base::OnceClosure closure);
-
-  ~TabStateDB();
+  ~TabStateDB() override;
 
   // Returns true if initialization has finished successfully, otherwise false.
   bool IsInitialized() const;
@@ -71,6 +68,16 @@ class TabStateDB {
 
  private:
   friend class ::TabStateDBTest;
+  friend class ::TabStateDBFactory;
+  friend class ::TabStateDBFactoryTest;
+
+  // Initializes the database. Closure is invoked when database initialization
+  // is finished at which point the database can be used.
+  // TODO(crbug.com/1069810) don't block the calling thread on initialization
+  // Queue up operations instead and execute when initialization is finished.
+  TabStateDB(leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
+             const base::FilePath& profile_directory);
+
   // Used for tests
   explicit TabStateDB(std::unique_ptr<leveldb_proto::ProtoDatabase<
                           tab_state_db::TabStateContentProto>> storage_database,

chrome/browser/tab/state/tab_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/tab/state/tab_state_db_factory.h"
+
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/tab/state/tab_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 kTabStateDBFolder[] = "tab_state_db";
+}  // namespace
+
+// static
+TabStateDBFactory* TabStateDBFactory::GetInstance() {
+  return base::Singleton<TabStateDBFactory>::get();
+}
+
+// static
+TabStateDB* TabStateDBFactory::GetForProfile(Profile* profile) {
+  // Incognito is currently not supported
+  if (profile->IsOffTheRecord())
+    return nullptr;
+
+  return static_cast<TabStateDB*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true));
+}
+
+TabStateDBFactory::TabStateDBFactory()
+    : BrowserContextKeyedServiceFactory(
+          "TabStateDBKeyedService",
+          BrowserContextDependencyManager::GetInstance()) {}
+
+TabStateDBFactory::~TabStateDBFactory() = default;
+
+KeyedService* TabStateDBFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  Profile* profile = Profile::FromBrowserContext(context);
+  DCHECK(!profile->IsOffTheRecord());
+
+  leveldb_proto::ProtoDatabaseProvider* proto_database_provider =
+      content::BrowserContext::GetDefaultStoragePartition(profile)
+          ->GetProtoDatabaseProvider();
+  base::FilePath tab_state_db_dir(
+      profile->GetPath().AppendASCII(kTabStateDBFolder));
+  return new TabStateDB(proto_database_provider, tab_state_db_dir);
+}

chrome/browser/tab/state/tab_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_TAB_STATE_TAB_STATE_DB_FACTORY_H_
+#define CHROME_BROWSER_TAB_STATE_TAB_STATE_DB_FACTORY_H_
+
+#include "base/memory/singleton.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+class KeyedService;
+class Profile;
+class TabStateDB;
+
+// Factory to create on TabStateDB per profile. Incognito is currently
+// not supported and the factory will return nullptr for an incognito profile.
+class TabStateDBFactory : public BrowserContextKeyedServiceFactory {
+ public:
+  // Acquire instance of TabStateDBFactory
+  static TabStateDBFactory* GetInstance();
+
+  // Acquire TabStateDB - there is one per profile.
+  static TabStateDB* GetForProfile(Profile* profile);
+
+ private:
+  friend struct base::DefaultSingletonTraits<TabStateDBFactory>;
+
+  TabStateDBFactory();
+  ~TabStateDBFactory() override;
+
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+};
+
+#endif  // CHROME_BROWSER_TAB_STATE_TAB_STATE_DB_FACTORY_H_

chrome/browser/tab/state/tab_state_db_factory_unittest.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/tab/state/tab_state_db_factory.h"
+
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class TabStateDBFactoryTest : public testing::Test {
+ public:
+  TabStateDBFactoryTest() = default;
+
+  Profile* profile() { return &profile_; }
+  Profile* different_profile() { return &different_profile_; }
+
+ private:
+  content::BrowserTaskEnvironment task_environment_;
+  TestingProfile profile_;
+  TestingProfile different_profile_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabStateDBFactoryTest);
+};
+
+TEST_F(TabStateDBFactoryTest, TestIncognitoProfile) {
+  EXPECT_EQ(nullptr, TabStateDBFactory::GetInstance()->GetForProfile(
+                         profile()->GetOffTheRecordProfile()));
+}
+
+TEST_F(TabStateDBFactoryTest, TestSameProfile) {
+  EXPECT_EQ(TabStateDBFactory::GetInstance()->GetForProfile(profile()),
+            TabStateDBFactory::GetInstance()->GetForProfile(profile()));
+}
+
+TEST_F(TabStateDBFactoryTest, TestDifferentProfile) {
+  EXPECT_NE(
+      TabStateDBFactory::GetInstance()->GetForProfile(different_profile()),
+      TabStateDBFactory::GetInstance()->GetForProfile(profile()));
+}

chrome/test/BUILD.gn

@@ -3351,6 +3351,7 @@ test("unit_tests") {
     "../browser/resource_coordinator/tab_load_tracker_unittest.cc",
     "../browser/resources_util_unittest.cc",
     "../browser/search/contextual_search_policy_handler_android_unittest.cc",
+    "../browser/tab/state/tab_state_db_factory_unittest.cc",
     "../browser/tab/state/tab_state_db_unittest.cc",
 
     # TODO(hashimoto): those tests should be componentized and moved to