Commit 6ea2d57c authored by Ryan Hansberry's avatar Ryan Hansberry Committed by Commit Bot

[SmartLock] Remove outdated and incorrect test hacks in test.

Also only Initialize the EasyUnlockService in the test once the test
Profile object is ready.

This change is high-priority because without it, the fix for
crbug.com/953027 causes this test to fail (the incomplete Profile
caused DCHECKs in known_user.h to fail).

Bug: 857494, 953027
Change-Id: I5a389b3017e6428136295ccd6899ffc05c6c354b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1669762
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672096}
parent 07100c7f
...@@ -295,10 +295,10 @@ AccountId EasyUnlockServiceRegular::GetAccountId() const { ...@@ -295,10 +295,10 @@ AccountId EasyUnlockServiceRegular::GetAccountId() const {
DCHECK(identity_manager); DCHECK(identity_manager);
const CoreAccountInfo account_info = const CoreAccountInfo account_info =
identity_manager->GetPrimaryAccountInfo(); identity_manager->GetPrimaryAccountInfo();
// A regular signed-in (i.e., non-login) profile should always have an email. // A regular signed-in (i.e., non-login) profile should always have an email.
// TODO(crbug.com/857494): Enable this DCHECK once all browser tests create DCHECK(!account_info.email.empty());
// correctly signed in profiles.
// DCHECK(!account_info.email.empty());
return AccountIdFromAccountInfo(account_info); return AccountIdFromAccountInfo(account_info);
} }
...@@ -364,17 +364,16 @@ void EasyUnlockServiceRegular::InitializeInternal() { ...@@ -364,17 +364,16 @@ void EasyUnlockServiceRegular::InitializeInternal() {
pref_manager_.reset(new proximity_auth::ProximityAuthProfilePrefManager( pref_manager_.reset(new proximity_auth::ProximityAuthProfilePrefManager(
profile()->GetPrefs(), multidevice_setup_client_)); profile()->GetPrefs(), multidevice_setup_client_));
// TODO(tengs): Due to badly configured browser_tests, Chrome crashes during // TODO(crbug.com/857494): Thousands of unrelated browser_tests provide a
// shutdown. Revisit this condition after migration is fully completed. // profile with an empty email to this service, causing crashes further on.
// Investigate what this service is doing so differently (i.e., incorrectly)
// from others that causes such issues with so many browser_tests.
if (!base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) { if (!base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
// Note: There is no local state in tests.
if (g_browser_process->local_state()) {
pref_manager_->StartSyncingToLocalState(g_browser_process->local_state(), pref_manager_->StartSyncingToLocalState(g_browser_process->local_state(),
GetAccountId()); GetAccountId());
} }
LoadRemoteDevices(); LoadRemoteDevices();
}
registrar_.Init(profile()->GetPrefs()); registrar_.Init(profile()->GetPrefs());
registrar_.Add( registrar_.Add(
...@@ -394,6 +393,8 @@ void EasyUnlockServiceRegular::ShutdownInternal() { ...@@ -394,6 +393,8 @@ void EasyUnlockServiceRegular::ShutdownInternal() {
device_sync_client_->RemoveObserver(this); device_sync_client_->RemoveObserver(this);
multidevice_setup_client_->RemoveObserver(this); multidevice_setup_client_->RemoveObserver(this);
weak_ptr_factory_.InvalidateWeakPtrs();
} }
bool EasyUnlockServiceRegular::IsAllowedInternal() const { bool EasyUnlockServiceRegular::IsAllowedInternal() const {
......
...@@ -83,14 +83,10 @@ GetDefaultMultiDeviceSetupClient() { ...@@ -83,14 +83,10 @@ GetDefaultMultiDeviceSetupClient() {
// EasyUnlockService factory function injected into testing profiles. // EasyUnlockService factory function injected into testing profiles.
std::unique_ptr<KeyedService> CreateEasyUnlockServiceForTest( std::unique_ptr<KeyedService> CreateEasyUnlockServiceForTest(
content::BrowserContext* context) { content::BrowserContext* context) {
std::unique_ptr<EasyUnlockServiceRegular> service( return std::make_unique<EasyUnlockServiceRegular>(
new EasyUnlockServiceRegular( Profile::FromBrowserContext(context), nullptr /* secure_channel_client */,
Profile::FromBrowserContext(context),
nullptr /* secure_channel_client */,
std::make_unique<MockEasyUnlockNotificationController>(), std::make_unique<MockEasyUnlockNotificationController>(),
GetDefaultDeviceSyncClient(), GetDefaultMultiDeviceSetupClient())); GetDefaultDeviceSyncClient(), GetDefaultMultiDeviceSetupClient());
service->Initialize();
return std::move(service);
} }
} // namespace } // namespace
...@@ -165,6 +161,10 @@ class EasyUnlockServiceTest : public testing::Test { ...@@ -165,6 +161,10 @@ class EasyUnlockServiceTest : public testing::Test {
AccountId::FromUserEmailGaiaId(email, *gaia_id)); AccountId::FromUserEmailGaiaId(email, *gaia_id));
profile.get()->set_profile_name(email); profile.get()->set_profile_name(email);
// Only initialize the service once the profile is completely ready. If done
// earlier, indirect usage of user_manager::KnownUser would crash.
EasyUnlockService::Get(profile.get())->Initialize();
return profile; return profile;
} }
......
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