[Extensions] Avoid late keyed services with TestingProfile

Overriding HistoryService and BookmarkModel after the profile has been
created is problematic and known to cause hard-to-debug test flakiness.

Instead, this patch adopts TestingProfile::Builder for tests in
chrome/browser/extensions.

Change-Id: Ifcf2999a5e12c0a2a3c2c23f06ae86711a54a982
Bug: 1106699,1106339
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310490Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790523}

chrome/browser/extensions/api/bookmark_manager_private/bookmark_manager_private_api_unittest.cc

@@ -22,8 +22,11 @@ class BookmarkManagerPrivateApiUnitTest : public ExtensionServiceTestBase {
 
   void SetUp() override {
     ExtensionServiceTestBase::SetUp();
-    InitializeEmptyExtensionService();
-    profile_->CreateBookmarkModel(false);
+
+    ExtensionServiceInitParams params = CreateDefaultInitParams();
+    params.enable_bookmark_model = true;
+    InitializeExtensionService(params);
+
     model_ = BookmarkModelFactory::GetForBrowserContext(profile());
     bookmarks::test::WaitForBookmarkModelToLoad(model_);
 

chrome/browser/extensions/api/bookmarks/bookmark_api_helpers_unittest.cc

@@ -42,9 +42,17 @@ class ExtensionBookmarksTest : public testing::Test {
         folder_(nullptr) {}
 
   void SetUp() override {
-    profile_.CreateBookmarkModel(false);
-    model_ = BookmarkModelFactory::GetForBrowserContext(&profile_);
-    managed_ = ManagedBookmarkServiceFactory::GetForProfile(&profile_);
+    TestingProfile::Builder profile_builder;
+    profile_builder.AddTestingFactory(
+        BookmarkModelFactory::GetInstance(),
+        BookmarkModelFactory::GetDefaultFactory());
+    profile_builder.AddTestingFactory(
+        ManagedBookmarkServiceFactory::GetInstance(),
+        ManagedBookmarkServiceFactory::GetDefaultFactory());
+
+    profile_ = profile_builder.Build();
+    model_ = BookmarkModelFactory::GetForBrowserContext(profile_.get());
+    managed_ = ManagedBookmarkServiceFactory::GetForProfile(profile_.get());
     bookmarks::test::WaitForBookmarkModelToLoad(model_);
 
     node_ = model_->AddURL(model_->other_node(), 0, base::ASCIIToUTF16("Digg"),
@@ -66,7 +74,7 @@ class ExtensionBookmarksTest : public testing::Test {
   }
 
   content::BrowserTaskEnvironment task_environment_;
-  TestingProfile profile_;
+  std::unique_ptr<TestingProfile> profile_;
   bookmarks::ManagedBookmarkService* managed_;
   BookmarkModel* model_;
   const BookmarkNode* node_;

chrome/browser/extensions/api/bookmarks/bookmarks_api_unittest.cc

@@ -22,8 +22,11 @@ class BookmarksApiUnittest : public ExtensionServiceTestBase {
 
   void SetUp() override {
     ExtensionServiceTestBase::SetUp();
-    InitializeEmptyExtensionService();
-    profile_->CreateBookmarkModel(false);
+
+    ExtensionServiceInitParams params = CreateDefaultInitParams();
+    params.enable_bookmark_model = true;
+    InitializeExtensionService(params);
+
     model_ = BookmarkModelFactory::GetForBrowserContext(profile());
     bookmarks::test::WaitForBookmarkModelToLoad(model_);
 

chrome/browser/extensions/api/declarative_content/declarative_content_condition_tracker_test.cc

@@ -14,9 +14,7 @@
 namespace extensions {
 
 DeclarativeContentConditionTrackerTest::DeclarativeContentConditionTrackerTest()
-    : profile_(new TestingProfile),
-      next_predicate_group_id_(1) {
-}
+    : next_predicate_group_id_(1) {}
 
 DeclarativeContentConditionTrackerTest::
 ~DeclarativeContentConditionTrackerTest() {
@@ -29,8 +27,7 @@ DeclarativeContentConditionTrackerTest::
 std::unique_ptr<content::WebContents>
 DeclarativeContentConditionTrackerTest::MakeTab() {
   std::unique_ptr<content::WebContents> tab(
-      content::WebContentsTester::CreateTestWebContents(profile_.get(),
-                                                        nullptr));
+      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
   content::RenderFrameHostTester::For(tab->GetMainFrame())
       ->InitializeRenderFrameIfNeeded();
   return tab;
@@ -43,6 +40,13 @@ DeclarativeContentConditionTrackerTest::GetMockRenderProcessHost(
       contents->GetMainFrame()->GetProcess());
 }
 
+TestingProfile* DeclarativeContentConditionTrackerTest::profile() {
+  if (!profile_) {
+    profile_ = profile_builder_.Build();
+  }
+  return profile_.get();
+}
+
 const void* DeclarativeContentConditionTrackerTest::GeneratePredicateGroupID() {
   // The group ID is opaque to the trackers.
   return reinterpret_cast<const void*>(next_predicate_group_id_++);

chrome/browser/extensions/api/declarative_content/declarative_content_condition_tracker_test.h

@@ -40,7 +40,11 @@ class DeclarativeContentConditionTrackerTest : public testing::Test {
   content::MockRenderProcessHost* GetMockRenderProcessHost(
       content::WebContents* contents);
 
-  TestingProfile* profile() { return profile_.get(); }
+  // Can only be used before calling profile().
+  TestingProfile::Builder* profile_builder() { return &profile_builder_; }
+
+  // Returns a TestingProfile constructed lazily (upon first call).
+  TestingProfile* profile();
 
   const void* GeneratePredicateGroupID();
 
@@ -50,7 +54,8 @@ class DeclarativeContentConditionTrackerTest : public testing::Test {
   // Enables MockRenderProcessHosts.
   content::RenderViewHostTestEnabler render_view_host_test_enabler_;
 
-  const std::unique_ptr<TestingProfile> profile_;
+  TestingProfile::Builder profile_builder_;
+  std::unique_ptr<TestingProfile> profile_;
 
   uintptr_t next_predicate_group_id_;
 

chrome/browser/extensions/api/declarative_content/declarative_content_is_bookmarked_condition_tracker_unittest.cc

@@ -95,7 +95,9 @@ class DeclarativeContentIsBookmarkedConditionTrackerTest
   };
 
   DeclarativeContentIsBookmarkedConditionTrackerTest() {
-    profile()->CreateBookmarkModel(true);
+    profile_builder()->AddTestingFactory(
+        BookmarkModelFactory::GetInstance(),
+        BookmarkModelFactory::GetDefaultFactory());
     bookmarks::test::WaitForBookmarkModelToLoad(
         BookmarkModelFactory::GetForBrowserContext(profile()));
     bookmark_model_ = BookmarkModelFactory::GetForBrowserContext(profile());

chrome/browser/extensions/extension_service_test_base.cc

@@ -15,6 +15,8 @@
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
+#include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#include "chrome/browser/bookmarks/managed_bookmark_service_factory.h"
 #include "chrome/browser/extensions/component_loader.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_garbage_collector_factory.h"
@@ -80,6 +82,15 @@ std::unique_ptr<TestingProfile> BuildTestingProfile(
 #endif
   }
 
+  if (params.enable_bookmark_model) {
+    profile_builder.AddTestingFactory(
+        BookmarkModelFactory::GetInstance(),
+        BookmarkModelFactory::GetDefaultFactory());
+    profile_builder.AddTestingFactory(
+        ManagedBookmarkServiceFactory::GetInstance(),
+        ManagedBookmarkServiceFactory::GetDefaultFactory());
+  }
+
   profile_builder.AddTestingFactories(
       IdentityTestEnvironmentProfileAdaptor::
           GetIdentityTestEnvironmentFactories());

chrome/browser/extensions/extension_service_test_base.h

@@ -60,6 +60,7 @@ class ExtensionServiceTestBase : public testing::Test {
     bool extensions_enabled = true;
     bool is_first_run = true;
     bool profile_is_supervised = false;
+    bool enable_bookmark_model = false;
 
     // Though you could use this constructor, you probably want to use
     // CreateDefaultInitParams(), and then make a change or two.

chrome/browser/extensions/media_router_extension_access_logger_impl_unittest.cc

@@ -45,9 +45,10 @@ TEST(MediaRouterExtensionAccessLoggerImplTest, LogsUkmCorrectly) {
   ukm::TestAutoSetUkmRecorder ukm_recorder;
   base::HistogramTester uma_recorder;
 
-  std::unique_ptr<TestingProfile> testing_profile =
-      TestingProfile::Builder().Build();
-  ASSERT_TRUE(testing_profile->CreateHistoryService(true, false));
+  TestingProfile::Builder profile_builder;
+  profile_builder.AddTestingFactory(HistoryServiceFactory::GetInstance(),
+                                    HistoryServiceFactory::GetDefaultFactory());
+  std::unique_ptr<TestingProfile> testing_profile = profile_builder.Build();
   history::HistoryService* history_service =
       HistoryServiceFactory::GetForProfile(testing_profile.get(),
                                            ServiceAccessType::EXPLICIT_ACCESS);