Commit 1ff3c501 authored by Mikel Astiz's avatar Mikel Astiz Committed by Commit Bot

Avoid late keyed services with TestingProfile

This patch migrates tests away from deprecated APIs and adopts
TestingProfile::Builder for tests in
/chrome/browser/bookmarks.

Rationale: creating or overriding keyed services after the profile has
been created is problematic and known to cause hard-to-debug test
flakiness, because it bypasses BrowserContextDependencyManager and often
leading to use-after-free.

This CL was uploaded by git cl split.

R=sky@chromium.org

Bug: 1106699
Change-Id: I5e31d889a94318e01c31d38d8e5c1b5222977dcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2315353
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791316}
parent 48275f11
...@@ -38,17 +38,23 @@ TEST(ManagedBookmarkServiceNoPolicyTest, EmptyManagedNode) { ...@@ -38,17 +38,23 @@ TEST(ManagedBookmarkServiceNoPolicyTest, EmptyManagedNode) {
// Verifies that the managed node is empty and invisible when the policy is // Verifies that the managed node is empty and invisible when the policy is
// not set. // not set.
content::BrowserTaskEnvironment task_environment; content::BrowserTaskEnvironment task_environment;
TestingProfile profile; TestingProfile::Builder profile_builder;
profile_builder.AddTestingFactory(BookmarkModelFactory::GetInstance(),
BookmarkModelFactory::GetDefaultFactory());
profile_builder.AddTestingFactory(
ManagedBookmarkServiceFactory::GetInstance(),
ManagedBookmarkServiceFactory::GetDefaultFactory());
std::unique_ptr<TestingProfile> profile = profile_builder.Build();
// Make sure the policy isn't set. // Make sure the policy isn't set.
ASSERT_EQ(nullptr, profile.GetTestingPrefService()->GetManagedPref( ASSERT_EQ(nullptr, profile->GetTestingPrefService()->GetManagedPref(
bookmarks::prefs::kManagedBookmarks)); bookmarks::prefs::kManagedBookmarks));
profile.CreateBookmarkModel(false); BookmarkModel* model =
BookmarkModel* model = BookmarkModelFactory::GetForBrowserContext(&profile); BookmarkModelFactory::GetForBrowserContext(profile.get());
bookmarks::test::WaitForBookmarkModelToLoad(model); bookmarks::test::WaitForBookmarkModelToLoad(model);
ManagedBookmarkService* managed = ManagedBookmarkService* managed =
ManagedBookmarkServiceFactory::GetForProfile(&profile); ManagedBookmarkServiceFactory::GetForProfile(profile.get());
DCHECK(managed); DCHECK(managed);
ASSERT_TRUE(managed->managed_node()); ASSERT_TRUE(managed->managed_node());
...@@ -62,7 +68,16 @@ class ManagedBookmarkServiceTest : public testing::Test { ...@@ -62,7 +68,16 @@ class ManagedBookmarkServiceTest : public testing::Test {
~ManagedBookmarkServiceTest() override {} ~ManagedBookmarkServiceTest() override {}
void SetUp() override { void SetUp() override {
prefs_ = profile_.GetTestingPrefService(); TestingProfile::Builder profile_builder;
profile_builder.AddTestingFactory(
BookmarkModelFactory::GetInstance(),
BookmarkModelFactory::GetDefaultFactory());
profile_builder.AddTestingFactory(
ManagedBookmarkServiceFactory::GetInstance(),
ManagedBookmarkServiceFactory::GetDefaultFactory());
profile_ = profile_builder.Build();
prefs_ = profile_->GetTestingPrefService();
ASSERT_FALSE(prefs_->HasPrefPath(bookmarks::prefs::kManagedBookmarks)); ASSERT_FALSE(prefs_->HasPrefPath(bookmarks::prefs::kManagedBookmarks));
// TODO(crbug.com/697817): Convert SetManagedPrefs to take a unique_ptr. // TODO(crbug.com/697817): Convert SetManagedPrefs to take a unique_ptr.
...@@ -70,11 +85,10 @@ class ManagedBookmarkServiceTest : public testing::Test { ...@@ -70,11 +85,10 @@ class ManagedBookmarkServiceTest : public testing::Test {
CreateTestTree()); CreateTestTree());
// Create and load the bookmark model. // Create and load the bookmark model.
profile_.CreateBookmarkModel(false); model_ = BookmarkModelFactory::GetForBrowserContext(profile_.get());
model_ = BookmarkModelFactory::GetForBrowserContext(&profile_);
bookmarks::test::WaitForBookmarkModelToLoad(model_); bookmarks::test::WaitForBookmarkModelToLoad(model_);
model_->AddObserver(&observer_); model_->AddObserver(&observer_);
managed_ = ManagedBookmarkServiceFactory::GetForProfile(&profile_); managed_ = ManagedBookmarkServiceFactory::GetForProfile(profile_.get());
DCHECK(managed_); DCHECK(managed_);
// The managed node always exists. // The managed node always exists.
...@@ -152,7 +166,7 @@ class ManagedBookmarkServiceTest : public testing::Test { ...@@ -152,7 +166,7 @@ class ManagedBookmarkServiceTest : public testing::Test {
} }
content::BrowserTaskEnvironment task_environment_; content::BrowserTaskEnvironment task_environment_;
TestingProfile profile_; std::unique_ptr<TestingProfile> profile_;
sync_preferences::TestingPrefServiceSyncable* prefs_; sync_preferences::TestingPrefServiceSyncable* prefs_;
bookmarks::MockBookmarkModelObserver observer_; bookmarks::MockBookmarkModelObserver observer_;
ManagedBookmarkService* managed_; ManagedBookmarkService* managed_;
...@@ -306,14 +320,14 @@ TEST_F(ManagedBookmarkServiceTest, HasDescendantsOfManagedNode) { ...@@ -306,14 +320,14 @@ TEST_F(ManagedBookmarkServiceTest, HasDescendantsOfManagedNode) {
TEST_F(ManagedBookmarkServiceTest, GetManagedBookmarksDomain) { TEST_F(ManagedBookmarkServiceTest, GetManagedBookmarksDomain) {
// Not managed profile // Not managed profile
profile_.set_profile_name("user@google.com"); profile_->set_profile_name("user@google.com");
EXPECT_TRUE( EXPECT_TRUE(
ManagedBookmarkServiceFactory::GetManagedBookmarksDomain(&profile_) ManagedBookmarkServiceFactory::GetManagedBookmarksDomain(profile_.get())
.empty()); .empty());
// Managed profile // Managed profile
profile_.GetProfilePolicyConnector()->OverrideIsManagedForTesting(true); profile_->GetProfilePolicyConnector()->OverrideIsManagedForTesting(true);
EXPECT_EQ( EXPECT_EQ(
"google.com", "google.com",
ManagedBookmarkServiceFactory::GetManagedBookmarksDomain(&profile_)); ManagedBookmarkServiceFactory::GetManagedBookmarksDomain(profile_.get()));
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment