From 60fe010482cd97485bd8145b3d1516f0b8940cb2 Mon Sep 17 00:00:00 2001 From: Max Karolinskiy Date: Thu, 14 Aug 2025 13:42:44 -0400 Subject: [PATCH] [cr141] IdentityManager shouldn't be created too early. The upstream change was causing a crash in BraveSpeedFeatureProcessorBrowserTest.Default browser test due to CHECK for early creation of IdentityManager. To avoid the check split --enable-brave-features-for-perf-testing switch handler into two parts: 1. Changes prefs 2. Instantiates services (rewards service that causes creation of IdentityManager) and components. Chromium change: https://source.chromium.org/chromium/chromium/src/+/21435fb93f29b1ece473baf8d62d36b978fcb1a8 commit 21435fb93f29b1ece473baf8d62d36b978fcb1a8 Author: Mihai Sardarescu Date: Tue Aug 5 03:09:57 2025 -0700 Check that IdentityManager is not created too early This CL adds an explicit CHECK in the profile initialization to ensure that the IdentityManager is not created before the profile keyed services are created. This ensures that the IdentityManager can be properly overrides with a mock/fake in unit and browser tests. Note: This check is currently if-deffed out on Android as on this platform the IdentityManager is created by mistake when initializing the storage partition. This will be fixed in a follow-up CL. Bug: 436208345 --- browser/perf/brave_perf_features_processor.cc | 21 +++++++++++++------ browser/perf/brave_perf_features_processor.h | 12 +++++++++-- browser/profiles/brave_profile_manager.cc | 7 +++---- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/browser/perf/brave_perf_features_processor.cc b/browser/perf/brave_perf_features_processor.cc index 8db5c796b13..14b5b15eca3 100644 --- a/browser/perf/brave_perf_features_processor.cc +++ b/browser/perf/brave_perf_features_processor.cc @@ -53,7 +53,7 @@ void EnableAdblockCookieList(base::WeakPtr profile) { namespace perf { -void MaybeEnableBraveFeatureForPerfTesting(Profile* profile) { +void MaybeEnableBraveFeaturesPrefsForPerfTesting(Profile* profile) { auto* cmd = base::CommandLine::ForCurrentProcess(); if (!cmd->HasSwitch(switches::kEnableBraveFeaturesForPerfTesting) || !cmd->HasSwitch(::switches::kUserDataDir)) { @@ -64,11 +64,6 @@ void MaybeEnableBraveFeatureForPerfTesting(Profile* profile) { profile->GetPrefs()->SetBoolean(brave_ads::prefs::kOptedInToNotificationAds, true); - // Rewards - auto* rewards_service = - brave_rewards::RewardsServiceFactory::GetForProfile(profile); - rewards_service->CreateRewardsWallet("US", base::BindOnce(&FakeCallback)); - // Brave news profile->GetPrefs()->SetBoolean(brave_news::prefs::kNewTabPageShowToday, true); @@ -86,6 +81,20 @@ void MaybeEnableBraveFeatureForPerfTesting(Profile* profile) { base::Time::Now()); profile->GetPrefs()->SetBoolean( ai_chat::prefs::kBraveChatAutocompleteProviderEnabled, true); +} + +void MaybeEnableBraveFeaturesServicesAndComponentsForPerfTesting( + Profile* profile) { + auto* cmd = base::CommandLine::ForCurrentProcess(); + if (!cmd->HasSwitch(switches::kEnableBraveFeaturesForPerfTesting) || + !cmd->HasSwitch(::switches::kUserDataDir)) { + return; + } + + // Rewards + auto* rewards_service = + brave_rewards::RewardsServiceFactory::GetForProfile(profile); + rewards_service->CreateRewardsWallet("US", base::BindOnce(&FakeCallback)); // Adblock EnableAdblockCookieList(profile->GetWeakPtr()); diff --git a/browser/perf/brave_perf_features_processor.h b/browser/perf/brave_perf_features_processor.h index d2f8ca9a3d0..01b268427cb 100644 --- a/browser/perf/brave_perf_features_processor.h +++ b/browser/perf/brave_perf_features_processor.h @@ -10,8 +10,16 @@ class Profile; namespace perf { -// A handler for --enable-brave-features-for-perf-testing switch. -void MaybeEnableBraveFeatureForPerfTesting(Profile* profile); +// Handlers for --enable-brave-features-for-perf-testing switch. +// They are split into two because the first is invoked from +// BraveProfileManager::InitProfileUserPrefs and cannot instantiate services +// that rely on identity manager because ProfileImpl::OnLocaleReady expects +// identity manager not to be initialized before browser context services are +// created. The second one is invoked from +// BraveProfileManager::DoFinalInitForServices. +void MaybeEnableBraveFeaturesPrefsForPerfTesting(Profile* profile); +void MaybeEnableBraveFeaturesServicesAndComponentsForPerfTesting( + Profile* profile); } // namespace perf diff --git a/browser/profiles/brave_profile_manager.cc b/browser/profiles/brave_profile_manager.cc index 90ae5b62348..5ea5fc9bed9 100644 --- a/browser/profiles/brave_profile_manager.cc +++ b/browser/profiles/brave_profile_manager.cc @@ -107,8 +107,7 @@ void RecordInitialP3AValues(Profile* profile) { } // namespace BraveProfileManager::BraveProfileManager(const base::FilePath& user_data_dir) - : ProfileManager(user_data_dir) { -} + : ProfileManager(user_data_dir) {} size_t BraveProfileManager::GetNumberOfProfiles() { size_t count = ProfileManager::GetNumberOfProfiles(); @@ -157,7 +156,7 @@ void BraveProfileManager::InitProfileUserPrefs(Profile* profile) { RecordInitialP3AValues(profile); brave::SetDefaultSearchVersion(profile, profile->IsNewProfile()); brave::SetDefaultThirdPartyCookieBlockValue(profile); - perf::MaybeEnableBraveFeatureForPerfTesting(profile); + perf::MaybeEnableBraveFeaturesPrefsForPerfTesting(profile); MigrateHttpsUpgradeSettings(profile); } @@ -166,6 +165,7 @@ void BraveProfileManager::DoFinalInitForServices(Profile* profile, ProfileManager::DoFinalInitForServices(profile, go_off_the_record); if (!do_final_services_init_) return; + perf::MaybeEnableBraveFeaturesServicesAndComponentsForPerfTesting(profile); brave_ads::AdsServiceFactory::GetForProfile(profile); brave_rewards::RewardsServiceFactory::GetForProfile(profile); brave_wallet::BraveWalletServiceFactory::GetServiceForContext(profile); @@ -230,7 +230,6 @@ void BraveProfileManager::SetNonPersonalProfilePrefs(Profile* profile) { prefs->SetBoolean(bookmarks::prefs::kShowBookmarkBar, false); } - BraveProfileManagerWithoutInit::BraveProfileManagerWithoutInit( const base::FilePath& user_data_dir) : BraveProfileManager(user_data_dir) {