0

Update UserManagerImpl::UserLoggedIn condition.

The condition to check whether or not multi-sign-in is updated.
Some assumption that UserManagerImpl has is not explicit assertion
with CHECK.

This is preparation to switch Fake{,Chrome}UserManager into
UserManagerImpl.

BUG=278643115
TEST=tryjob

Change-Id: I80d47a588dc8dbec1fb93cd27008b0ddaf0e9eee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6196999
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1412459}
This commit is contained in:
Hidehiko Abe
2025-01-28 11:49:18 -08:00
committed by Chromium LUCI CQ
parent 5fa48150ab
commit f45e2828c4
14 changed files with 207 additions and 90 deletions

@@ -36,6 +36,7 @@
#include "chromeos/ash/components/browser_context_helper/browser_context_types.h"
#include "components/media_router/browser/test/mock_media_router.h"
#include "components/session_manager/core/session_manager.h"
#include "components/user_manager/test_helper.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/test/browser_test.h"
@@ -466,14 +467,26 @@ class MultiProfileDriveFileSystemExtensionApiTest
"false");
}
void SetUpLocalStatePrefService(PrefService* local_state) override {
InProcessBrowserTest::SetUpLocalStatePrefService(local_state);
// Register persisted users.
user_manager::TestHelper::RegisterPersistedUser(
*local_state,
AccountId::FromUserEmailGaiaId("testuser@gmail.com", GaiaId("123456")));
user_manager::TestHelper::RegisterPersistedUser(
*local_state, AccountId::FromUserEmailGaiaId(kSecondProfileAccount,
kSecondProfileGaiaId));
}
void SetUpOnMainThread() override {
// Set up the secondary user session.
base::FilePath user_data_directory;
base::PathService::Get(chrome::DIR_USER_DATA, &user_data_directory);
session_manager::SessionManager::Get()->CreateSession(
AccountId::FromUserEmailGaiaId(kSecondProfileAccount,
kSecondProfileGaiaId),
kSecondProfileHash, false);
// Set up the secondary profile.
base::FilePath profile_dir = user_data_directory.AppendASCII(
ash::BrowserContextHelper::GetUserBrowserContextDirName(
kSecondProfileHash));

@@ -35,6 +35,7 @@
#include "chromeos/ash/components/login/auth/public/user_context.h"
#include "components/account_id/account_id.h"
#include "components/policy/proto/chrome_settings.pb.h"
#include "components/user_manager/test_helper.h"
#include "components/user_manager/user_names.h"
#include "content/public/test/browser_test.h"
#include "testing/gmock/include/gmock/gmock.h"
@@ -105,6 +106,18 @@ class ShillProfileLoadingTest : public LoginManagerTest {
~ShillProfileLoadingTest() override = default;
// LoginManagerTest:
void SetUpLocalStatePrefService(PrefService* local_state) override {
LoginManagerTest::SetUpLocalStatePrefService(local_state);
// Register a persisted user.
user_manager::TestHelper::RegisterPersistedUser(*local_state,
unmanaged_user_.account_id);
user_manager::TestHelper::RegisterPersistedUser(
*local_state, secondary_unmanaged_user_.account_id);
user_manager::TestHelper::RegisterPersistedUser(*local_state,
managed_user_.account_id);
}
void SetUpInProcessBrowserTestFixture() override {
LoginManagerTest::SetUpInProcessBrowserTestFixture();

@@ -48,6 +48,7 @@
#include "components/prefs/pref_service.h"
#include "components/user_manager/known_user.h"
#include "components/user_manager/scoped_user_manager.h"
#include "components/user_manager/test_helper.h"
#include "components/user_manager/user.h"
#include "components/user_manager/user_manager_impl.h"
#include "components/user_manager/user_manager_pref_names.h"
@@ -514,10 +515,17 @@ TEST_F(UserManagerTest, RemoveUser) {
false /* browser_restart */,
false /* is_child */);
// Recreate the user manager to log out all accounts.
ResetUserManager();
// Create non-owner account and login in.
user_manager_->UserLoggedIn(kAccountId0, kAccountId0.GetUserEmail(),
false /* browser_restart */,
false /* is_child */);
// Log-in owner account.
user_manager_->UserLoggedIn(kOwnerAccountId, kOwnerAccountId.GetUserEmail(),
false /* browser_restart */,
false /* is_child */);
ASSERT_EQ(2U, user_manager_->GetUsers().size());

@@ -124,6 +124,18 @@ void ScalableIphBrowserTestBase::SetUp() {
CustomizableTestEnvBrowserTestBase::SetUp();
}
void ScalableIphBrowserTestBase::SetUpInProcessBrowserTestFixture() {
CustomizableTestEnvBrowserTestBase::SetUpInProcessBrowserTestFixture();
if (enable_multi_user_) {
// Add a secondary user.
LoginManagerMixin* login_manager_mixin = GetLoginManagerMixin();
CHECK(login_manager_mixin);
login_manager_mixin->AppendRegularUsers(1);
CHECK_EQ(login_manager_mixin->users().size(), 2ul);
}
}
// `SetUpOnMainThread` is called just before a test body. Do the mock set up in
// this function as `browser()` is not available in `SetUp` above.
void ScalableIphBrowserTestBase::SetUpOnMainThread() {
@@ -150,12 +162,6 @@ void ScalableIphBrowserTestBase::SetUpOnMainThread() {
}
if (enable_multi_user_) {
// Add a secondary user.
LoginManagerMixin* login_manager_mixin = GetLoginManagerMixin();
CHECK(login_manager_mixin);
login_manager_mixin->AppendRegularUsers(1);
CHECK_EQ(login_manager_mixin->users().size(), 2ul);
// By default, `MultiUserWindowManager` is created with multi profile off.
// Re-create for multi profile tests. This has to be done after
// `SetUpOnMainThread` of a base class as the original multi-profile-off

@@ -56,6 +56,7 @@ class ScalableIphBrowserTestBase : public CustomizableTestEnvBrowserTestBase {
// CustomizableTestEnvBrowserTestBase:
void SetUp() override;
void SetUpInProcessBrowserTestFixture() override;
void SetUpOnMainThread() override;
void TearDownOnMainThread() override;

@@ -70,19 +70,15 @@
#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "ash/constants/ash_features.h"
#include "ash/constants/ash_switches.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/user_manager/user_manager.h"
#include "components/user_manager/user_manager_pref_names.h"
#elif BUILDFLAG(IS_CHROMEOS_LACROS)
#include "chromeos/startup/browser_init_params.h"
#endif
namespace {
enum {
DUMMY_ACCOUNT_INDEX = 0,
PRIMARY_ACCOUNT_INDEX = 1,
SECONDARY_ACCOUNT_INDEX_START = 2,
};
#if BUILDFLAG(IS_CHROMEOS_ASH)
// Structure to describe an account info.
struct TestAccountInfo {
@@ -1047,9 +1043,9 @@ class MultiProfileDownloadNotificationTest
// Logs in to a dummy profile.
command_line->AppendSwitchASCII(ash::switches::kLoginUser,
kTestAccounts[DUMMY_ACCOUNT_INDEX].email);
kTestAccounts[0].email);
command_line->AppendSwitchASCII(ash::switches::kLoginProfile,
kTestAccounts[DUMMY_ACCOUNT_INDEX].hash);
kTestAccounts[0].hash);
// Don't require policy for our sessions - this is required because
// this test creates a secondary profile synchronously, so we need to
// let the policy code know not to expect cached policy.
@@ -1057,24 +1053,24 @@ class MultiProfileDownloadNotificationTest
"false");
}
// Logs in to the primary profile.
void SetUpOnMainThread() override {
const TestAccountInfo& info = kTestAccounts[PRIMARY_ACCOUNT_INDEX];
void SetUpLocalStatePrefService(PrefService* local_state) override {
InProcessBrowserTest::SetUpLocalStatePrefService(local_state);
AddUser(info, true);
DownloadNotificationTestBase::SetUpOnMainThread();
// Register a persisted user.
ScopedListPrefUpdate prefs_user_update(
local_state, user_manager::prefs::kRegularUsersPref);
for (const auto& test_account : kTestAccounts) {
prefs_user_update->Append(test_account.email);
}
}
// Loads all users to the current session and sets up necessary fields.
// This is used for preparing all accounts in PRE_ test setup, and for testing
// actual login behavior.
void AddAllUsers() {
for (size_t i = 0; i < std::size(kTestAccounts); ++i) {
// The primary account was already set up in SetUpOnMainThread, so skip it
// here.
if (i == PRIMARY_ACCOUNT_INDEX)
continue;
AddUser(kTestAccounts[i], i >= SECONDARY_ACCOUNT_INDEX_START);
// Logs in to the primary profile.
void SetUpOnMainThread() override {
DownloadNotificationTestBase::SetUpOnMainThread();
// Add all users, except the first one, which is already logged in.
for (size_t i = 1; i < std::size(kTestAccounts); ++i) {
AddUser(kTestAccounts[i]);
}
}
@@ -1085,12 +1081,11 @@ class MultiProfileDownloadNotificationTest
}
// Adds a new user for testing to the current session.
void AddUser(const TestAccountInfo& info, bool log_in) {
if (log_in) {
void AddUser(const TestAccountInfo& info) {
session_manager::SessionManager::Get()->CreateSession(
AccountId::FromUserEmailGaiaId(info.email, GaiaId(info.gaia_id)),
info.hash, false);
}
user_manager::UserManager::Get()->SaveUserDisplayName(
AccountId::FromUserEmailGaiaId(info.email, GaiaId(info.gaia_id)),
base::UTF8ToUTF16(info.display_name));
@@ -1109,15 +1104,8 @@ class MultiProfileDownloadNotificationTest
std::unique_ptr<NotificationDisplayServiceTester> display_service2_;
};
IN_PROC_BROWSER_TEST_F(MultiProfileDownloadNotificationTest,
PRE_DownloadMultipleFiles) {
AddAllUsers();
}
IN_PROC_BROWSER_TEST_F(MultiProfileDownloadNotificationTest,
DownloadMultipleFiles) {
AddAllUsers();
GURL url(SlowDownloadInterceptor::kUnknownSizeUrl);
Profile* profile1 = GetProfileByIndex(1);

@@ -224,6 +224,7 @@
#include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/policy_service.h"
#include "components/session_manager/core/session_manager.h"
#include "components/user_manager/test_helper.h"
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
using content::WebContents;
@@ -5655,6 +5656,13 @@ IN_PROC_BROWSER_TEST_F(SSLUITestNoCert, NewCertificateAuthority) {
// into their NSS databases.
class SSLUITestCustomCACerts : public SSLUITestNoCert {
public:
static inline constexpr char kPrimaryUserAccount[] = "test1@test.com";
static inline constexpr GaiaId::Literal kPrimaryUserGaiaId{"1234567890"};
static inline constexpr char kPrimaryUserHash[] = "test1-hash";
static inline constexpr char kSecondaryUserAccount[] = "test2@test.com";
static inline constexpr GaiaId::Literal kSecondaryUserGaiaId{"9876543210"};
static inline constexpr char kSecondaryUserHash[] = "test2-hash";
SSLUITestCustomCACerts() = default;
SSLUITestCustomCACerts(const SSLUITestCustomCACerts&) = delete;
@@ -5668,6 +5676,23 @@ class SSLUITestCustomCACerts : public SSLUITestNoCert {
// code knows not to expect cached policy for the secondary profile.
command_line->AppendSwitchASCII(ash::switches::kProfileRequiresPolicy,
"false");
command_line->AppendSwitchASCII(ash::switches::kLoginUser,
kPrimaryUserAccount);
command_line->AppendSwitchASCII(ash::switches::kLoginProfile,
kPrimaryUserHash);
}
void SetUpLocalStatePrefService(PrefService* local_state) override {
SSLUITestNoCert::SetUpLocalStatePrefService(local_state);
// Register a persisted user.
user_manager::TestHelper::RegisterPersistedUser(
*local_state, AccountId::FromUserEmailGaiaId(kPrimaryUserAccount,
kPrimaryUserGaiaId));
user_manager::TestHelper::RegisterPersistedUser(
*local_state, AccountId::FromUserEmailGaiaId(kSecondaryUserAccount,
kSecondaryUserGaiaId));
}
void SetUpOnMainThread() override {
@@ -5677,10 +5702,6 @@ class SSLUITestCustomCACerts : public SSLUITestNoCert {
// Create a second profile.
{
static const char kSecondProfileAccount[] = "profile2@test.com";
static const char kSecondProfileGaiaId[] = "9876543210";
static const char kSecondProfileHash[] = "testProfile2";
ON_CALL(policy_for_profile_2_, IsInitializationComplete(testing::_))
.WillByDefault(testing::Return(true));
ON_CALL(policy_for_profile_2_, IsFirstPolicyLoadComplete(testing::_))
@@ -5691,12 +5712,12 @@ class SSLUITestCustomCACerts : public SSLUITestNoCert {
base::FilePath user_data_directory;
base::PathService::Get(chrome::DIR_USER_DATA, &user_data_directory);
session_manager::SessionManager::Get()->CreateSession(
AccountId::FromUserEmailGaiaId(kSecondProfileAccount,
GaiaId(kSecondProfileGaiaId)),
kSecondProfileHash, false);
AccountId::FromUserEmailGaiaId(kSecondaryUserAccount,
kSecondaryUserGaiaId),
kSecondaryUserHash, false);
// Set up the secondary profile.
base::FilePath profile_dir = user_data_directory.Append(
ash::ProfileHelper::GetUserProfileDir(kSecondProfileHash).BaseName());
ash::ProfileHelper::GetUserProfileDir(kSecondaryUserHash).BaseName());
profile_2_ =
g_browser_process->profile_manager()->GetProfile(profile_dir);
}

@@ -117,8 +117,6 @@ IN_PROC_BROWSER_TEST_F(ChromeNewWindowClientBrowserTest,
}
IN_PROC_BROWSER_TEST_F(ChromeNewWindowClientBrowserTest, IncognitoDisabled) {
CreateAndStartUserSession(
AccountId::FromUserEmailGaiaId(kTestUserName1, GaiaId(kTestUser2GaiaId)));
Profile* profile = ProfileManager::GetActiveUserProfile();
EXPECT_EQ(1u, chrome::GetTotalBrowserCount());

@@ -40,6 +40,7 @@
#include "chromeos/ash/components/browser_context_helper/browser_context_helper.h"
#include "components/account_id/account_id.h"
#include "components/session_manager/core/session_manager.h"
#include "components/user_manager/test_helper.h"
#endif
namespace web_app {
@@ -128,13 +129,19 @@ class WebAppProfileDeletionBrowserTest : public WebAppBrowserTestBase {
}
#if BUILDFLAG(IS_CHROMEOS)
void CreateSession(const AccountId& account_id) {
auto* session_manager = session_manager::SessionManager::Get();
session_manager->CreateSession(account_id, account_id.GetUserEmail(),
false);
void SetUpLocalStatePrefService(PrefService* local_state) override {
WebAppBrowserTestBase::SetUpLocalStatePrefService(local_state);
// Register a persisted user.
user_manager::TestHelper::RegisterPersistedUser(*local_state,
test_account_id_);
}
Profile& StartUserSession(const AccountId& account_id) {
auto* session_manager = session_manager::SessionManager::Get();
session_manager->CreateSession(account_id, account_id.GetUserEmail(),
false);
ProfileManager* profile_manager = g_browser_process->profile_manager();
Profile& profile = profiles::testing::CreateProfileSync(
profile_manager,
@@ -143,7 +150,6 @@ class WebAppProfileDeletionBrowserTest : public WebAppBrowserTestBase {
->FindUser(account_id)
->username_hash()));
auto* session_manager = session_manager::SessionManager::Get();
session_manager->NotifyUserProfileLoaded(account_id);
session_manager->SessionStarted();
return profile;
@@ -176,7 +182,6 @@ IN_PROC_BROWSER_TEST_F(WebAppProfileDeletionBrowserTest, OsIntegrationRemoved) {
/// Create a new profile and install a web app.
ProfileManager* profile_manager = g_browser_process->profile_manager();
#if BUILDFLAG(IS_CHROMEOS)
CreateSession(test_account_id_);
Profile& profile_to_delete = StartUserSession(test_account_id_);
#else
base::FilePath profile_path_to_delete =
@@ -210,7 +215,6 @@ IN_PROC_BROWSER_TEST_F(WebAppProfileDeletionBrowserTest,
// Create a new profile.
ProfileManager* profile_manager = g_browser_process->profile_manager();
#if BUILDFLAG(IS_CHROMEOS)
CreateSession(test_account_id_);
Profile& profile_to_delete = StartUserSession(test_account_id_);
#else
base::FilePath profile_path_to_delete =
@@ -250,7 +254,6 @@ IN_PROC_BROWSER_TEST_F(WebAppProfileDeletionBrowserTest_WebAppPublisher,
/// Create a new profile and install a web app.
ProfileManager* profile_manager = g_browser_process->profile_manager();
#if BUILDFLAG(IS_CHROMEOS)
CreateSession(test_account_id_);
Profile& profile_to_delete = StartUserSession(test_account_id_);
#else
base::FilePath profile_path_to_delete =

@@ -11,6 +11,7 @@
#include "ash/constants/ash_features.h"
#include "ash/constants/ash_pref_names.h"
#include "ash/constants/ash_switches.h"
#include "base/containers/adapters.h"
#include "chrome/browser/ash/accessibility/accessibility_manager.h"
#include "chrome/browser/ash/input_method/mock_input_method_engine.h"
@@ -24,6 +25,7 @@
#include "components/language/core/browser/pref_names.h"
#include "components/language/core/common/locale_util.h"
#include "components/prefs/pref_service.h"
#include "components/user_manager/test_helper.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/test_web_ui.h"
#include "google_apis/gaia/gaia_id.h"
@@ -42,7 +44,8 @@ namespace {
// Use a real domain to avoid policy loading problems.
constexpr char kTestUserName[] = "owner@gmail.com";
constexpr char kTestUserGaiaId[] = "9876543210";
constexpr char kTestUserHash[] = "1234567890-hash";
constexpr GaiaId::Literal kTestUserGaiaId{"9876543210"};
} // namespace
@@ -65,6 +68,20 @@ class AccessibilityHandlerTest : public InProcessBrowserTest {
void SetUpCommandLine(base::CommandLine* command_line) override {
scoped_feature_list_.InitWithFeatures(
{features::kOnDeviceSpeechRecognition}, {});
// Use a persisted user.
command_line->AppendSwitchASCII(ash::switches::kLoginUser, kTestUserName);
command_line->AppendSwitchASCII(ash::switches::kLoginProfile,
kTestUserHash);
}
void SetUpLocalStatePrefService(PrefService* local_state) override {
InProcessBrowserTest::SetUpLocalStatePrefService(local_state);
// Register a persisted user.
user_manager::TestHelper::RegisterPersistedUser(
*local_state,
AccountId::FromUserEmailGaiaId(kTestUserName, kTestUserGaiaId));
}
void SetUpOnMainThread() override {
@@ -140,25 +157,6 @@ class AccessibilityHandlerTest : public InProcessBrowserTest {
prefs::kAccessibilityDictationLocale, locale);
}
void CreateSession(const AccountId& account_id) {
auto* session_manager = session_manager::SessionManager::Get();
session_manager->CreateSession(account_id, account_id.GetUserEmail(),
false);
}
void StartUserSession(const AccountId& account_id) {
profiles::testing::CreateProfileSync(
g_browser_process->profile_manager(),
BrowserContextHelper::Get()->GetBrowserContextPathByUserIdHash(
user_manager::UserManager::Get()
->FindUser(account_id)
->username_hash()));
auto* session_manager = session_manager::SessionManager::Get();
session_manager->NotifyUserProfileLoaded(account_id);
session_manager->SessionStarted();
}
speech::SodaInstaller* soda_installer() {
return speech::SodaInstaller::GetInstance();
}
@@ -169,9 +167,6 @@ class AccessibilityHandlerTest : public InProcessBrowserTest {
std::unique_ptr<input_method::MockInputMethodEngine> mock_ime_engine_handler_;
const AccountId test_account_id_ =
AccountId::FromUserEmailGaiaId(kTestUserName, GaiaId(kTestUserGaiaId));
private:
std::unique_ptr<TestingProfile> profile_;
std::unique_ptr<TestAccessibilityHandler> handler_;
@@ -344,8 +339,6 @@ IN_PROC_BROWSER_TEST_F(AccessibilityHandlerTest,
}
IN_PROC_BROWSER_TEST_F(AccessibilityHandlerTest, GetStartupSoundEnabled) {
CreateSession(test_account_id_);
StartUserSession(test_account_id_);
AccessibilityManager::Get()->SetStartupSoundEnabled(true);
size_t call_data_count_before_call = web_ui()->call_data().size();

@@ -90,6 +90,8 @@ if (is_chromeos) {
"fake_user_manager.h",
"fake_user_manager_delegate.cc",
"fake_user_manager_delegate.h",
"test_helper.cc",
"test_helper.h",
]
public_deps = [
@@ -99,7 +101,10 @@ if (is_chromeos) {
"//components/account_id",
]
deps = [ "//chromeos/ash/components/settings" ]
deps = [
"//chromeos/ash/components/settings",
"//components/prefs",
]
}
source_set("unit_tests") {

@@ -0,0 +1,27 @@
// Copyright 2025 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/user_manager/test_helper.h"
#include "components/account_id/account_id.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/user_manager/known_user.h"
#include "components/user_manager/user_manager_pref_names.h"
namespace user_manager {
// static
void TestHelper::RegisterPersistedUser(PrefService& local_state,
const AccountId& account_id) {
{
ScopedListPrefUpdate update(&local_state, prefs::kRegularUsersPref);
update->Append(account_id.GetUserEmail());
}
{
KnownUser known_user(&local_state);
known_user.UpdateId(account_id);
}
}
} // namespace user_manager

@@ -0,0 +1,32 @@
// Copyright 2025 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_USER_MANAGER_TEST_HELPER_H_
#define COMPONENTS_USER_MANAGER_TEST_HELPER_H_
class AccountId;
class PrefService;
namespace user_manager {
// Utilities to set up UserManager related environment.
class TestHelper {
public:
// Records the `account_id` as a persisted user to the given `local_state`.
// `local_state` must be properly set up, specifically it needs UserManager
// related registration.
// In most cases, this registration needs to be done before UserManager
// is created. Specifically, for browser_tests, SetUpLocalStatePrefService()
// is a recommended function to call this.
static void RegisterPersistedUser(PrefService& local_state,
const AccountId& account_id);
private:
// Currently, this has only static methods.
TestHelper() = delete;
};
} // namespace user_manager
#endif // COMPONENTS_USER_MANAGER_TEST_HELPER_H_

@@ -275,9 +275,13 @@ void UserManagerImpl::UserLoggedIn(const AccountId& account_id,
User* user = FindUserInListAndModify(account_id);
const UserType user_type =
CalculateUserType(account_id, user, browser_restart, is_child);
if (active_user_ && user) {
if (!logged_in_users_.empty()) {
// Handle multi sign-in case. For multi-sign in, there already should be
// active_user_ set, and the user to be logged in should be found
// in the persistent user list.
CHECK(active_user_);
CHECK(user);
user->set_is_logged_in(true);
user->set_username_hash(username_hash);
logged_in_users_.push_back(user);
@@ -300,7 +304,12 @@ void UserManagerImpl::UserLoggedIn(const AccountId& account_id,
return;
}
// There are no logged in users. `active_user_` should not be set.
CHECK(!active_user_);
// Ensure User is there.
const UserType user_type =
CalculateUserType(account_id, user, browser_restart, is_child);
switch (user_type) {
case UserType::kRegular:
case UserType::kChild: