Revert "Enable the SiteDataCacheFacadeFactory in tests"

This reverts commit 59f71ba9.

Reason for revert: It turns out that we can't do this as this is causing UAFs in tests using a BrowserTaskEnvironment and a TestingProfile:
- Creating a TestingProfile forces the creation of all the BrowserContextKeyedServiceFactory associated with this profile. These factories are singletons and are leaked at the end of the test.
- Destroying the BrowserTaskEnvironment at the end of the test causes the ThreadPool to shutdown.
- If a BrowserContextKeyedServiceFactory ends up maintaining a reference to a task runner (like this factory does) then the second test that tries to use it is guaranteed to fail as it'll use a reference to a task runner that has been released.

Original change's description:
> Enable the SiteDataCacheFacadeFactory in tests
> 
> Change-Id: Idea598dc21a96c4297841db2a733a2721f2d08e5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2261212
> Auto-Submit: Sébastien Marchand <sebmarchand@chromium.org>
> Commit-Queue: Chris Hamilton <chrisha@chromium.org>
> Reviewed-by: Chris Hamilton <chrisha@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#781543}

TBR=chrisha@chromium.org,sebmarchand@chromium.org

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

Change-Id: I9972b136a93824fc959d94c9a74122c41cce5123
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274198Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Auto-Submit: Sébastien Marchand <sebmarchand@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783922}

chrome/browser/performance_manager/persistence/site_data/site_data_cache_facade_factory.cc

@@ -69,7 +69,8 @@ bool SiteDataCacheFacadeFactory::ServiceIsCreatedWithBrowserContext() const {
 }
 
 bool SiteDataCacheFacadeFactory::ServiceIsNULLWhileTesting() const {
-  return false;
+  // Tests that want to use this factory will have to explicitly enable it.
+  return true;
 }
 
 }  // namespace performance_manager

chrome/browser/performance_manager/persistence/site_data/site_data_cache_facade_unittest.cc

@@ -113,8 +113,8 @@ class SiteDataCacheFacadeTest : public testing::TestWithPerformanceManager {
 
   void TearDown() override {
     use_in_memory_db_for_testing_.reset();
-    profile_.reset();
     facade_factory_.reset();
+    profile_.reset();
     testing::TestWithPerformanceManager::TearDown();
   }
 
@@ -142,8 +142,6 @@ class SiteDataCacheFacadeTest : public testing::TestWithPerformanceManager {
     return mock_cache_raw;
   }
 
-  SiteDataCacheFacadeFactory* facade_factory() { return facade_factory_.get(); }
-
  private:
   std::unique_ptr<TestingProfile> profile_;
   std::unique_ptr<SiteDataCacheFacadeFactory> facade_factory_;
@@ -153,12 +151,12 @@ class SiteDataCacheFacadeTest : public testing::TestWithPerformanceManager {
 TEST_F(SiteDataCacheFacadeTest, IsDataCacheRecordingForTesting) {
   bool cache_is_recording = false;
 
-  auto* facade = facade_factory()->GetForProfile(profile());
-  facade->WaitUntilCacheInitializedForTesting();
+  SiteDataCacheFacade data_cache_facade(profile());
+  data_cache_facade.WaitUntilCacheInitializedForTesting();
   {
     base::RunLoop run_loop;
     auto quit_closure = run_loop.QuitClosure();
-    facade->IsDataCacheRecordingForTesting(
+    data_cache_facade.IsDataCacheRecordingForTesting(
         base::BindLambdaForTesting([&](bool is_recording) {
           cache_is_recording = is_recording;
           std::move(quit_closure).Run();
@@ -167,12 +165,12 @@ TEST_F(SiteDataCacheFacadeTest, IsDataCacheRecordingForTesting) {
   }
   EXPECT_TRUE(cache_is_recording);
 
-  auto* off_record_data_cache_facade =
-      facade_factory()->GetForProfile(profile()->GetPrimaryOTRProfile());
+  SiteDataCacheFacade off_record_data_cache_facade(
+      profile()->GetPrimaryOTRProfile());
   {
     base::RunLoop run_loop;
     auto quit_closure = run_loop.QuitClosure();
-    off_record_data_cache_facade->IsDataCacheRecordingForTesting(
+    off_record_data_cache_facade.IsDataCacheRecordingForTesting(
         base::BindLambdaForTesting([&](bool is_recording) {
           cache_is_recording = is_recording;
           quit_closure.Run();
@@ -196,13 +194,13 @@ TEST_F(SiteDataCacheFacadeTest, OnURLsDeleted_Partial_OriginNotReferenced) {
       {TestOrigin2().GetURL(), {0, base::Time::Now()}},
   });
 
-  auto* facade = facade_factory()->GetForProfile(profile());
-  facade->WaitUntilCacheInitializedForTesting();
+  SiteDataCacheFacade data_cache_facade(profile());
+  data_cache_facade.WaitUntilCacheInitializedForTesting();
 
   auto* mock_cache_raw = SetUpMockCache();
   mock_cache_raw->SetClearSiteDataForOriginsExpectations(
       {TestOrigin(), TestOrigin2()});
-  facade->OnURLsDeleted(nullptr, deletion_info);
+  data_cache_facade.OnURLsDeleted(nullptr, deletion_info);
   mock_cache_raw->WaitForExpectations();
 }
 
@@ -219,26 +217,27 @@ TEST_F(SiteDataCacheFacadeTest, OnURLsDeleted_Partial_OriginStillReferenced) {
       {TestOrigin2().GetURL(), {3, base::Time::Now()}},
   });
 
-  auto* facade = facade_factory()->GetForProfile(profile());
-  facade->WaitUntilCacheInitializedForTesting();
+  SiteDataCacheFacade data_cache_facade(profile());
+  data_cache_facade.WaitUntilCacheInitializedForTesting();
 
   auto* mock_cache_raw = SetUpMockCache();
   // |TestOrigin2()| shouldn't be removed as there's still some references to it
   // in the history.
   mock_cache_raw->SetClearSiteDataForOriginsExpectations({TestOrigin()});
-  facade->OnURLsDeleted(nullptr, deletion_info);
+  data_cache_facade.OnURLsDeleted(nullptr, deletion_info);
   mock_cache_raw->WaitForExpectations();
 }
 
 // Verify that origins are removed from the data cache (in memory and on disk)
 // when the history is completely cleared.
 TEST_F(SiteDataCacheFacadeTest, OnURLsDeleted_Full) {
-  auto* facade = facade_factory()->GetForProfile(profile());
-  facade->WaitUntilCacheInitializedForTesting();
+  SiteDataCacheFacade data_cache_facade(profile());
+  data_cache_facade.WaitUntilCacheInitializedForTesting();
 
   auto* mock_cache_raw = SetUpMockCache();
   mock_cache_raw->SetClearAllSiteDataExpectations();
-  facade->OnURLsDeleted(nullptr, history::DeletionInfo::ForAllHistory());
+  data_cache_facade.OnURLsDeleted(nullptr,
+                                  history::DeletionInfo::ForAllHistory());
   mock_cache_raw->WaitForExpectations();
 }