Revert "Add a LocalSiteCharacteristicsDataStore factory"

This reverts commit 6a1485d2.

Reason for revert: breaks Linux CFI, crbug.com/846492

Original change's description:
> 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}

TBR=bauerb@chromium.org,fdoray@chromium.org,erg@chromium.org,sebmarchand@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 773382
Change-Id: I9216725dd36891661b75a3d03a93d5298ea2dc30
Reviewed-on: https://chromium-review.googlesource.com/1072787
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561694}

chrome/browser/BUILD.gn

@@ -2655,8 +2655,6 @@ 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,7 +93,6 @@
 #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"
@@ -339,9 +338,6 @@ void ChromeBrowserMainExtraPartsProfiles::
   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,7 +4,6 @@
 
 #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"
@@ -12,7 +11,6 @@
 #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 {
@@ -25,8 +23,6 @@ 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));
 
@@ -61,10 +57,6 @@ LocalSiteCharacteristicsDataStore::GetWriterForOrigin(
   return base::WrapUnique(data_writer);
 }
 
-bool LocalSiteCharacteristicsDataStore::IsRecordingForTesting() {
-  return true;
-}
-
 internal::LocalSiteCharacteristicsDataImpl*
 LocalSiteCharacteristicsDataStore::GetOrCreateFeatureImpl(
     const std::string& origin_str) {
@@ -121,9 +113,4 @@ 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.
 //
-// This class should never be used for off the record profiles, the
-// LocalSiteCharacteristicsNonRecordingDataStore class should be used instead.
+// TODO(sebmarchand): Expose a method to receive a dummy writer that doesn't
+// record anything, for use in incognito sessions.
 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,8 +75,6 @@ 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,11 +5,9 @@
 #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"
@@ -44,11 +42,8 @@ class MockLocalSiteCharacteristicsDatabase
 class LocalSiteCharacteristicsDataStoreTest : public ::testing::Test {
  public:
   LocalSiteCharacteristicsDataStoreTest()
-      : scoped_set_tick_clock_for_testing_(&test_clock_) {
-    scoped_feature_list_.InitAndEnableFeature(
-        features::kProactiveTabDiscarding);
-    data_store_ =
-        std::make_unique<LocalSiteCharacteristicsDataStore>(&profile_);
+      : scoped_set_tick_clock_for_testing_(&test_clock_),
+        data_store_(&profile_) {
     test_clock_.SetNowTicks(base::TimeTicks::UnixEpoch());
     test_clock_.Advance(base::TimeDelta::FromHours(1));
     WaitForAsyncOperationsToComplete();
@@ -64,18 +59,17 @@ 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_;
-  std::unique_ptr<LocalSiteCharacteristicsDataStore> data_store_;
+  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());
@@ -87,40 +81,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);
@@ -136,12 +130,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();
@@ -161,8 +155,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
@@ -181,7 +175,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,8 +35,4 @@ 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,7 +27,6 @@ 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,9 +4,7 @@
 
 #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"
@@ -17,8 +15,6 @@ 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,15 +11,14 @@
 #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 : public KeyedService {
+class SiteCharacteristicsDataStore {
  public:
   SiteCharacteristicsDataStore() = default;
-  ~SiteCharacteristicsDataStore() override {}
+  virtual ~SiteCharacteristicsDataStore() {}
 
   // Returns a SiteCharacteristicsDataReader for the given origin.
   virtual std::unique_ptr<SiteCharacteristicsDataReader> GetReaderForOrigin(
@@ -29,10 +28,6 @@ class SiteCharacteristicsDataStore : public KeyedService {
   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

@@ -695,7 +695,6 @@ 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/render_process_probe_browsertest.cc",
       "../browser/resource_coordinator/tab_activity_watcher_browsertest.cc",
       "../browser/resource_coordinator/tab_manager_browsertest.cc",