Add a LocalSiteCharacteristicsDataStore factory

This follows the guidelines from this page: https://www.chromium.org/developers/design-documents/profile-architecture

Bug: 773382
Change-Id: I8517975f9881e3fa2d52fb26b16008d780255eef
Reviewed-on: https://chromium-review.googlesource.com/1047725
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561176}

chrome/browser/BUILD.gn

@@ -2659,6 +2659,8 @@ jumbo_split_static_library("browser") {
       "resource_coordinator/local_site_characteristics_data_reader.h",
       "resource_coordinator/local_site_characteristics_data_store.cc",
       "resource_coordinator/local_site_characteristics_data_store.h",
+      "resource_coordinator/local_site_characteristics_data_store_factory.cc",
+      "resource_coordinator/local_site_characteristics_data_store_factory.h",
       "resource_coordinator/local_site_characteristics_data_writer.cc",
       "resource_coordinator/local_site_characteristics_data_writer.h",
       "resource_coordinator/local_site_characteristics_database.h",

chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

@@ -93,6 +93,7 @@
 #include "chrome/browser/android/data_usage/data_use_ui_tab_model_factory.h"
 #include "chrome/browser/android/search_permissions/search_permissions_service.h"
 #else
+#include "chrome/browser/resource_coordinator/local_site_characteristics_data_store_factory.h"
 #include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/ui/global_error/global_error_service_factory.h"
 #include "chrome/browser/ui/webui/media_router/media_router_ui_service_factory.h"
@@ -172,11 +173,9 @@ void AddProfilesExtraParts(ChromeBrowserMainParts* main_parts) {
 
 }  // namespace chrome
 
-ChromeBrowserMainExtraPartsProfiles::ChromeBrowserMainExtraPartsProfiles() {
-}
+ChromeBrowserMainExtraPartsProfiles::ChromeBrowserMainExtraPartsProfiles() {}
 
-ChromeBrowserMainExtraPartsProfiles::~ChromeBrowserMainExtraPartsProfiles() {
-}
+ChromeBrowserMainExtraPartsProfiles::~ChromeBrowserMainExtraPartsProfiles() {}
 
 // This method gets the instance of each ServiceFactory. We do this so that
 // each ServiceFactory initializes itself and registers its dependencies with
@@ -190,7 +189,7 @@ ChromeBrowserMainExtraPartsProfiles::~ChromeBrowserMainExtraPartsProfiles() {
 //
 // static
 void ChromeBrowserMainExtraPartsProfiles::
-EnsureBrowserContextKeyedServiceFactoriesBuilt() {
+    EnsureBrowserContextKeyedServiceFactoriesBuilt() {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   apps::EnsureBrowserContextKeyedServiceFactoriesBuilt();
   extensions::EnsureBrowserContextKeyedServiceFactoriesBuilt();
@@ -342,6 +341,9 @@ EnsureBrowserContextKeyedServiceFactoriesBuilt() {
   ProtocolHandlerRegistryFactory::GetInstance();
   PolicyBlacklistFactory::GetInstance()->SetBlacklistOverride(
       base::BindRepeating(policy::OverrideBlacklistForURL));
+#if !defined(OS_ANDROID)
+  resource_coordinator::LocalSiteCharacteristicsDataStoreFactory::GetInstance();
+#endif
 #if defined(OS_ANDROID)
   SearchPermissionsService::Factory::GetInstance();
 #endif

chrome/browser/resource_coordinator/local_site_characteristics_data_store.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_store.h"
 
+#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "chrome/browser/history/history_service_factory.h"
@@ -11,6 +12,7 @@
 #include "chrome/browser/resource_coordinator/leveldb_site_characteristics_database.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_reader.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_writer.h"
+#include "chrome/browser/resource_coordinator/tab_manager_features.h"
 #include "components/history/core/browser/history_service.h"
 
 namespace resource_coordinator {
@@ -23,6 +25,8 @@ constexpr char kSiteCharacteristicsDirectoryName[] =
 LocalSiteCharacteristicsDataStore::LocalSiteCharacteristicsDataStore(
     Profile* profile)
     : history_observer_(this) {
+  DCHECK(base::FeatureList::IsEnabled(features::kProactiveTabDiscarding));
+
   database_ = std::make_unique<LevelDBSiteCharacteristicsDatabase>(
       profile->GetPath().AppendASCII(kSiteCharacteristicsDirectoryName));
 
@@ -57,6 +61,10 @@ LocalSiteCharacteristicsDataStore::GetWriterForOrigin(
   return base::WrapUnique(data_writer);
 }
 
+bool LocalSiteCharacteristicsDataStore::IsRecordingForTesting() {
+  return true;
+}
+
 internal::LocalSiteCharacteristicsDataImpl*
 LocalSiteCharacteristicsDataStore::GetOrCreateFeatureImpl(
     const std::string& origin_str) {
@@ -113,4 +121,9 @@ void LocalSiteCharacteristicsDataStore::OnURLsDeleted(
   }
 }
 
+void LocalSiteCharacteristicsDataStore::HistoryServiceBeingDeleted(
+    history::HistoryService* history_service) {
+  history_observer_.Remove(history_service);
+}
+
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_store.h

@@ -25,8 +25,8 @@ class LocalSiteCharacteristicsDatabase;
 // Implementation of a SiteCharacteristicsDataStore that use the local site
 // characteristics database as a backend.
 //
-// TODO(sebmarchand): Expose a method to receive a dummy writer that doesn't
-// record anything, for use in incognito sessions.
+// This class should never be used for off the record profiles, the
+// LocalSiteCharacteristicsNonRecordingDataStore class should be used instead.
 class LocalSiteCharacteristicsDataStore
     : public SiteCharacteristicsDataStore,
       public internal::LocalSiteCharacteristicsDataImpl::OnDestroyDelegate,
@@ -41,9 +41,9 @@ class LocalSiteCharacteristicsDataStore
   // SiteCharacteristicDataStore:
   std::unique_ptr<SiteCharacteristicsDataReader> GetReaderForOrigin(
       const std::string& origin_str) override;
-
   std::unique_ptr<SiteCharacteristicsDataWriter> GetWriterForOrigin(
       const std::string& origin_str) override;
+  bool IsRecordingForTesting() override;
 
   const LocalSiteCharacteristicsMap& origin_data_map_for_testing() const {
     return origin_data_map_;
@@ -75,6 +75,8 @@ class LocalSiteCharacteristicsDataStore
   // history::HistoryServiceObserver:
   void OnURLsDeleted(history::HistoryService* history_service,
                      const history::DeletionInfo& deletion_info) override;
+  void HistoryServiceBeingDeleted(
+      history::HistoryService* history_service) override;
 
   // Map a serialized origin to a LocalSiteCharacteristicDataInternal
   // pointer.

chrome/browser/resource_coordinator/local_site_characteristics_data_store_factory.cc

+// Copyright 2018 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/resource_coordinator/local_site_characteristics_data_store_factory.h"
+
+#include "base/feature_list.h"
+#include "chrome/browser/profiles/incognito_helpers.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/resource_coordinator/local_site_characteristics_data_store.h"
+#include "chrome/browser/resource_coordinator/local_site_characteristics_non_recording_data_store.h"
+#include "chrome/browser/resource_coordinator/tab_manager_features.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "content/public/browser/browser_context.h"
+
+namespace resource_coordinator {
+
+// static
+SiteCharacteristicsDataStore*
+LocalSiteCharacteristicsDataStoreFactory::GetForProfile(Profile* profile) {
+  if (base::FeatureList::IsEnabled(features::kProactiveTabDiscarding)) {
+    return static_cast<LocalSiteCharacteristicsDataStore*>(
+        GetInstance()->GetServiceForBrowserContext(profile, true));
+  }
+  return nullptr;
+}
+
+LocalSiteCharacteristicsDataStoreFactory*
+LocalSiteCharacteristicsDataStoreFactory::GetInstance() {
+  static base::NoDestructor<LocalSiteCharacteristicsDataStoreFactory> instance;
+  return instance.get();
+}
+
+LocalSiteCharacteristicsDataStoreFactory::
+    LocalSiteCharacteristicsDataStoreFactory()
+    : BrowserContextKeyedServiceFactory(
+          "LocalSiteCharacteristicsDataStore",
+          BrowserContextDependencyManager::GetInstance()) {}
+
+LocalSiteCharacteristicsDataStoreFactory::
+    ~LocalSiteCharacteristicsDataStoreFactory() = default;
+
+SiteCharacteristicsDataStore*
+LocalSiteCharacteristicsDataStoreFactory::GetExistingDataStoreForContext(
+    content::BrowserContext* context) const {
+  DCHECK(context);
+  // Remove the const attribute from |this| and calls
+  // |GetServiceForBrowserContext| with the |create| parameter set to false.
+  // This is basically a getter that returns the data store associated with a
+  // context without creating it if it doesn't exits.
+  SiteCharacteristicsDataStore* data_store =
+      static_cast<SiteCharacteristicsDataStore*>(
+          const_cast<LocalSiteCharacteristicsDataStoreFactory*>(this)
+              ->GetServiceForBrowserContext(context, false /* create */));
+  DCHECK(data_store);
+  return data_store;
+}
+
+KeyedService* LocalSiteCharacteristicsDataStoreFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  SiteCharacteristicsDataStore* data_store = nullptr;
+  if (context->IsOffTheRecord()) {
+    content::BrowserContext* parent_context =
+        chrome::GetBrowserContextRedirectedInIncognito(context);
+    DCHECK(parent_context);
+    // Off the record profiles correspond to incognito profile and are derived
+    // from a parent profile that is on record.
+    DCHECK(!parent_context->IsOffTheRecord());
+    SiteCharacteristicsDataStore* data_store_for_readers =
+        GetExistingDataStoreForContext(parent_context);
+    DCHECK(data_store_for_readers);
+    data_store = new LocalSiteCharacteristicsNonRecordingDataStore(
+        data_store_for_readers);
+  } else {
+    Profile* profile = Profile::FromBrowserContext(context);
+    DCHECK(profile);
+    data_store = new LocalSiteCharacteristicsDataStore(profile);
+  }
+  DCHECK(data_store);
+  return data_store;
+}
+
+content::BrowserContext*
+LocalSiteCharacteristicsDataStoreFactory::GetBrowserContextToUse(
+    content::BrowserContext* context) const {
+  return chrome::GetBrowserContextOwnInstanceInIncognito(context);
+}
+
+bool LocalSiteCharacteristicsDataStoreFactory::
+    ServiceIsCreatedWithBrowserContext() const {
+  return base::FeatureList::IsEnabled(features::kProactiveTabDiscarding);
+}
+
+bool LocalSiteCharacteristicsDataStoreFactory::ServiceIsNULLWhileTesting()
+    const {
+  return true;
+}
+
+}  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_store_factory.h

+// Copyright 2018 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_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATA_STORE_FACTORY_H_
+#define CHROME_BROWSER_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATA_STORE_FACTORY_H_
+
+#include "base/containers/flat_map.h"
+#include "base/no_destructor.h"
+#include "chrome/browser/resource_coordinator/site_characteristics_data_store.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+class Profile;
+
+namespace resource_coordinator {
+
+// Singleton that owns all the LocalSiteCharacteristicsDataStore instances and
+// associates them with Profiles.
+class LocalSiteCharacteristicsDataStoreFactory
+    : public BrowserContextKeyedServiceFactory {
+ public:
+  static SiteCharacteristicsDataStore* GetForProfile(Profile* profile);
+  static LocalSiteCharacteristicsDataStoreFactory* GetInstance();
+
+ private:
+  friend class base::NoDestructor<LocalSiteCharacteristicsDataStoreFactory>;
+
+  LocalSiteCharacteristicsDataStoreFactory();
+  ~LocalSiteCharacteristicsDataStoreFactory() override;
+
+  // Returns the |SiteCharacteristicsDataStore| instance associated with
+  // |context|. This is basically a wrapper around GetServiceForBrowserContext
+  // that allows calling it from a const function.
+  SiteCharacteristicsDataStore* GetExistingDataStoreForContext(
+      content::BrowserContext* context) const;
+
+  // BrowserContextKeyedServiceFactory:
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+  content::BrowserContext* GetBrowserContextToUse(
+      content::BrowserContext* context) const override;
+  bool ServiceIsCreatedWithBrowserContext() const override;
+  bool ServiceIsNULLWhileTesting() const override;
+};
+
+}  // namespace resource_coordinator
+
+#endif  // CHROME_BROWSER_RESOURCE_COORDINATOR_LOCAL_SITE_CHARACTERISTICS_DATA_STORE_FACTORY_H_

chrome/browser/resource_coordinator/local_site_characteristics_data_store_factory_browsertest.cc

+// Copyright 2018 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/resource_coordinator/local_site_characteristics_data_store_factory.h"
+
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile_window.h"
+#include "chrome/browser/resource_coordinator/tab_manager_features.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(OS_CHROMEOS)
+#include "chromeos/chromeos_switches.h"
+#endif
+
+namespace resource_coordinator {
+
+namespace {
+
+// Ensures that a SiteCharacteristicsDataStore respect the |IsOffTheRecord| of
+// its corresponding profile.
+bool DataStoreRespectsOffTheRecordValue(
+    Profile* profile,
+    SiteCharacteristicsDataStore* data_store) {
+  return profile->IsOffTheRecord() == !data_store->IsRecordingForTesting();
+}
+
+}  // namespace
+
+class LocalSiteCharacteristicsDataStoreFactoryTest
+    : public InProcessBrowserTest {
+ protected:
+  LocalSiteCharacteristicsDataStoreFactoryTest() = default;
+  ~LocalSiteCharacteristicsDataStoreFactoryTest() override = default;
+
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(
+        features::kProactiveTabDiscarding);
+    InProcessBrowserTest::SetUp();
+  }
+
+#if defined(OS_CHROMEOS)
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(
+        chromeos::switches::kIgnoreUserProfileMappingForTests);
+  }
+#endif
+
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(LocalSiteCharacteristicsDataStoreFactoryTest);
+};
+
+IN_PROC_BROWSER_TEST_F(LocalSiteCharacteristicsDataStoreFactoryTest, EndToEnd) {
+  Profile* regular_profile = browser()->profile();
+  ASSERT_TRUE(regular_profile);
+  SiteCharacteristicsDataStore* recording_data_store =
+      LocalSiteCharacteristicsDataStoreFactory::GetForProfile(regular_profile);
+  ASSERT_TRUE(recording_data_store);
+  EXPECT_TRUE(DataStoreRespectsOffTheRecordValue(regular_profile,
+                                                 recording_data_store));
+
+  Profile* incognito_profile = regular_profile->GetOffTheRecordProfile();
+  ASSERT_TRUE(incognito_profile);
+  SiteCharacteristicsDataStore* incognito_data_store =
+      static_cast<SiteCharacteristicsDataStore*>(
+          LocalSiteCharacteristicsDataStoreFactory::GetForProfile(
+              incognito_profile));
+  ASSERT_TRUE(incognito_data_store);
+  EXPECT_NE(recording_data_store, incognito_data_store);
+  EXPECT_TRUE(DataStoreRespectsOffTheRecordValue(incognito_profile,
+                                                 incognito_data_store));
+
+  content::WindowedNotificationObserver browser_creation_observer(
+      chrome::NOTIFICATION_BROWSER_OPENED,
+      content::NotificationService::AllSources());
+  profiles::SwitchToGuestProfile(ProfileManager::CreateCallback());
+  browser_creation_observer.Wait();
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  Profile* guest_profile =
+      profile_manager->GetProfileByPath(ProfileManager::GetGuestProfilePath());
+  ASSERT_TRUE(guest_profile);
+  SiteCharacteristicsDataStore* guest_data_store =
+      static_cast<SiteCharacteristicsDataStore*>(
+          LocalSiteCharacteristicsDataStoreFactory::GetForProfile(
+              guest_profile));
+  ASSERT_TRUE(guest_data_store);
+  EXPECT_NE(recording_data_store, guest_data_store);
+  EXPECT_TRUE(
+      DataStoreRespectsOffTheRecordValue(guest_profile, guest_data_store));
+}
+
+}  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_data_store_unittest.cc

@@ -5,9 +5,11 @@
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_store.h"
 
 #include "base/macros.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_impl.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_unittest_utils.h"
+#include "chrome/browser/resource_coordinator/tab_manager_features.h"
 #include "chrome/browser/resource_coordinator/time.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/history/core/browser/history_types.h"
@@ -42,8 +44,11 @@ class MockLocalSiteCharacteristicsDatabase
 class LocalSiteCharacteristicsDataStoreTest : public ::testing::Test {
  public:
   LocalSiteCharacteristicsDataStoreTest()
-      : scoped_set_tick_clock_for_testing_(&test_clock_),
-        data_store_(&profile_) {
+      : scoped_set_tick_clock_for_testing_(&test_clock_) {
+    scoped_feature_list_.InitAndEnableFeature(
+        features::kProactiveTabDiscarding);
+    data_store_ =
+        std::make_unique<LocalSiteCharacteristicsDataStore>(&profile_);
     test_clock_.SetNowTicks(base::TimeTicks::UnixEpoch());
     test_clock_.Advance(base::TimeDelta::FromHours(1));
     WaitForAsyncOperationsToComplete();
@@ -59,17 +64,18 @@ class LocalSiteCharacteristicsDataStoreTest : public ::testing::Test {
   base::SimpleTestTickClock test_clock_;
   ScopedSetTickClockForTesting scoped_set_tick_clock_for_testing_;
   content::TestBrowserThreadBundle test_browser_thread_bundle_;
+  base::test::ScopedFeatureList scoped_feature_list_;
   TestingProfile profile_;
-  LocalSiteCharacteristicsDataStore data_store_;
+  std::unique_ptr<LocalSiteCharacteristicsDataStore> data_store_;
 };
 
 TEST_F(LocalSiteCharacteristicsDataStoreTest, EndToEnd) {
-  auto reader = data_store_.GetReaderForOrigin(kTestOrigin);
+  auto reader = data_store_->GetReaderForOrigin(kTestOrigin);
   EXPECT_TRUE(reader);
-  auto writer = data_store_.GetWriterForOrigin(kTestOrigin);
+  auto writer = data_store_->GetWriterForOrigin(kTestOrigin);
   EXPECT_TRUE(writer);
 
-  EXPECT_EQ(1U, data_store_.origin_data_map_for_testing().size());
+  EXPECT_EQ(1U, data_store_->origin_data_map_for_testing().size());
 
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,
             reader->UpdatesTitleInBackground());
@@ -81,40 +87,40 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, EndToEnd) {
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureInUse,
             reader->UpdatesTitleInBackground());
 
-  auto reader_copy = data_store_.GetReaderForOrigin(kTestOrigin);
-  EXPECT_EQ(1U, data_store_.origin_data_map_for_testing().size());
-  auto reader2 = data_store_.GetReaderForOrigin(kTestOrigin2);
-  EXPECT_EQ(2U, data_store_.origin_data_map_for_testing().size());
+  auto reader_copy = data_store_->GetReaderForOrigin(kTestOrigin);
+  EXPECT_EQ(1U, data_store_->origin_data_map_for_testing().size());
+  auto reader2 = data_store_->GetReaderForOrigin(kTestOrigin2);
+  EXPECT_EQ(2U, data_store_->origin_data_map_for_testing().size());
   reader2.reset();
 
   WaitForAsyncOperationsToComplete();
-  EXPECT_EQ(1U, data_store_.origin_data_map_for_testing().size());
+  EXPECT_EQ(1U, data_store_->origin_data_map_for_testing().size());
   reader_copy.reset();
 
   reader.reset();
   writer.reset();
-  EXPECT_TRUE(data_store_.origin_data_map_for_testing().empty());
+  EXPECT_TRUE(data_store_->origin_data_map_for_testing().empty());
 
-  data_store_.OnURLsDeleted(nullptr, history::DeletionInfo::ForAllHistory());
+  data_store_->OnURLsDeleted(nullptr, history::DeletionInfo::ForAllHistory());
 }
 
 TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
   // Mock the database to ensure that the delete events get propagated properly.
   ::testing::StrictMock<MockLocalSiteCharacteristicsDatabase>* mock_db =
       new ::testing::StrictMock<MockLocalSiteCharacteristicsDatabase>();
-  data_store_.SetDatabaseForTesting(base::WrapUnique(mock_db));
+  data_store_->SetDatabaseForTesting(base::WrapUnique(mock_db));
 
   // Load a first origin, and then make use of a feature on it.
 
   const std::string kOrigin1Url = GURL(kTestOrigin).GetOrigin().GetContent();
-  auto reader = data_store_.GetReaderForOrigin(kOrigin1Url);
+  auto reader = data_store_->GetReaderForOrigin(kOrigin1Url);
   EXPECT_TRUE(reader);
 
-  auto writer = data_store_.GetWriterForOrigin(kOrigin1Url);
+  auto writer = data_store_->GetWriterForOrigin(kOrigin1Url);
   EXPECT_TRUE(writer);
 
   internal::LocalSiteCharacteristicsDataImpl* data =
-      data_store_.origin_data_map_for_testing().find(kOrigin1Url)->second;
+      data_store_->origin_data_map_for_testing().find(kOrigin1Url)->second;
   EXPECT_NE(nullptr, data);
 
   constexpr base::TimeDelta kDelay = base::TimeDelta::FromHours(1);
@@ -130,12 +136,12 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
 
   // Load a second origin, make use of a feature on it too.
   const std::string kOrigin2Url = GURL(kTestOrigin2).GetOrigin().GetContent();
-  auto reader2 = data_store_.GetReaderForOrigin(kOrigin2Url);
+  auto reader2 = data_store_->GetReaderForOrigin(kOrigin2Url);
   EXPECT_TRUE(reader2);
-  auto writer2 = data_store_.GetWriterForOrigin(kOrigin2Url);
+  auto writer2 = data_store_->GetWriterForOrigin(kOrigin2Url);
   EXPECT_TRUE(writer2);
   internal::LocalSiteCharacteristicsDataImpl* data2 =
-      data_store_.origin_data_map_for_testing().find(kOrigin2Url)->second;
+      data_store_->origin_data_map_for_testing().find(kOrigin2Url)->second;
   EXPECT_NE(nullptr, data2);
   data2->NotifySiteLoaded();
   data2->NotifyUpdatesFaviconInBackground();
@@ -155,8 +161,8 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
       *mock_db,
       RemoveSiteCharacteristicsFromDB(::testing::ContainerEq(
           std::vector<std::string>({kOrigin1Url, kOriginNotInMapURL}))));
-  data_store_.OnURLsDeleted(nullptr, history::DeletionInfo::ForUrls(
-                                         urls_to_delete, std::set<GURL>()));
+  data_store_->OnURLsDeleted(nullptr, history::DeletionInfo::ForUrls(
+                                          urls_to_delete, std::set<GURL>()));
   ::testing::Mock::VerifyAndClear(mock_db);
 
   // The information for this site have been reset, so the last loaded time
@@ -175,7 +181,7 @@ TEST_F(LocalSiteCharacteristicsDataStoreTest, HistoryServiceObserver) {
 
   // Delete all the information stored in the data store.
   EXPECT_CALL(*mock_db, ClearDatabase());
-  data_store_.OnURLsDeleted(nullptr, history::DeletionInfo::ForAllHistory());
+  data_store_->OnURLsDeleted(nullptr, history::DeletionInfo::ForAllHistory());
   ::testing::Mock::VerifyAndClear(mock_db);
 
   EXPECT_EQ(SiteFeatureUsage::kSiteFeatureUsageUnknown,

chrome/browser/resource_coordinator/local_site_characteristics_non_recording_data_store.cc

@@ -35,4 +35,8 @@ LocalSiteCharacteristicsNonRecordingDataStore::GetWriterForOrigin(
   return base::WrapUnique(writer);
 }
 
+bool LocalSiteCharacteristicsNonRecordingDataStore::IsRecordingForTesting() {
+  return false;
+}
+
 }  // namespace resource_coordinator

chrome/browser/resource_coordinator/local_site_characteristics_non_recording_data_store.h

@@ -27,6 +27,7 @@ class LocalSiteCharacteristicsNonRecordingDataStore
       const std::string& origin_str) override;
   std::unique_ptr<SiteCharacteristicsDataWriter> GetWriterForOrigin(
       const std::string& origin_str) override;
+  bool IsRecordingForTesting() override;
 
  private:
   // The data store to use to create the readers served by this data store. E.g.

chrome/browser/resource_coordinator/local_site_characteristics_non_recording_data_store_unittest.cc

@@ -4,7 +4,9 @@
 
 #include "chrome/browser/resource_coordinator/local_site_characteristics_non_recording_data_store.h"
 
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_store.h"
+#include "chrome/browser/resource_coordinator/tab_manager_features.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
@@ -15,6 +17,8 @@ namespace resource_coordinator {
 using LocalSiteCharacteristicsNonRecordingDataStoreTest = ::testing::Test;
 
 TEST_F(LocalSiteCharacteristicsNonRecordingDataStoreTest, EndToEnd) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kProactiveTabDiscarding);
   content::TestBrowserThreadBundle test_browser_thread_bundle;
   TestingProfile profile;
   const char kTestOrigin[] = "http://www.foo.com";

chrome/browser/resource_coordinator/site_characteristics_data_store.h

@@ -11,14 +11,15 @@
 #include "base/macros.h"
 #include "chrome/browser/resource_coordinator/site_characteristics_data_reader.h"
 #include "chrome/browser/resource_coordinator/site_characteristics_data_writer.h"
+#include "components/keyed_service/core/keyed_service.h"
 
 namespace resource_coordinator {
 
 // Pure virtual interface for a site characteristics data store.
-class SiteCharacteristicsDataStore {
+class SiteCharacteristicsDataStore : public KeyedService {
  public:
   SiteCharacteristicsDataStore() = default;
-  virtual ~SiteCharacteristicsDataStore() {}
+  ~SiteCharacteristicsDataStore() override {}
 
   // Returns a SiteCharacteristicsDataReader for the given origin.
   virtual std::unique_ptr<SiteCharacteristicsDataReader> GetReaderForOrigin(
@@ -28,6 +29,10 @@ class SiteCharacteristicsDataStore {
   virtual std::unique_ptr<SiteCharacteristicsDataWriter> GetWriterForOrigin(
       const std::string& origin_str) = 0;
 
+  // Indicate if the SiteCharacteristicsDataWriter served by this data store
+  // actually persist informations.
+  virtual bool IsRecordingForTesting() = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(SiteCharacteristicsDataStore);
 };

chrome/test/BUILD.gn

@@ -694,6 +694,7 @@ test("browser_tests") {
       "../browser/renderer_context_menu/spelling_menu_observer_browsertest.cc",
       "../browser/renderer_host/render_process_host_chrome_browsertest.cc",
       "../browser/repost_form_warning_browsertest.cc",
+      "../browser/resource_coordinator/local_site_characteristics_data_store_factory_browsertest.cc",
       "../browser/resource_coordinator/resource_coordinator_render_process_probe_browsertest.cc",
       "../browser/resource_coordinator/tab_activity_watcher_browsertest.cc",
       "../browser/resource_coordinator/tab_manager_browsertest.cc",