Skip to content

Commit

Permalink
cros: Change max users to be a constant
Browse files Browse the repository at this point in the history
Changing max users to be a constant instead of a dynamic config to
prepare switching from SessionStateDelegate to SessionController.
The current ash code uses the number in SystemTray::CreateItems
during initialization and does not handle dynamic change. In
SessionController case, this happens before initial SessionInfo is
sent to ash. Ash will then use the default value in SessionController
and whatever sent via SessionInfo later is ignored. This CL removes
max_users from mojom::SessionInfo and make it constant.

BUG=648964

Review-Url: https://codereview.chromium.org/2637403007
Cr-Commit-Position: refs/heads/master@{#445530}
  • Loading branch information
xiyuan authored and Commit bot committed Jan 23, 2017
1 parent 3aee9f2 commit 25ecee8
Show file tree
Hide file tree
Showing 11 changed files with 9 additions and 24 deletions.
3 changes: 1 addition & 2 deletions ash/common/session/session_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ void SessionController::BindRequest(mojom::SessionControllerRequest request) {
}

int SessionController::GetMaximumNumberOfLoggedInUsers() const {
return static_cast<int>(max_users_);
return session_manager::kMaxmiumNumberOfUserSessions;
}

int SessionController::NumberOfLoggedInUsers() const {
Expand Down Expand Up @@ -93,7 +93,6 @@ void SessionController::SetClient(mojom::SessionControllerClientPtr client) {
}

void SessionController::SetSessionInfo(mojom::SessionInfoPtr info) {
max_users_ = info->max_users;
can_lock_ = info->can_lock_screen;
should_lock_screen_automatically_ = info->should_lock_screen_automatically;
add_user_session_policy_ = info->add_user_session_policy;
Expand Down
1 change: 0 additions & 1 deletion ash/common/session/session_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ class ASH_EXPORT SessionController
mojom::SessionControllerClientPtr client_;

// Cached session info.
uint32_t max_users_ = 0u;
bool can_lock_ = false;
bool should_lock_screen_automatically_ = false;
AddUserSessionPolicy add_user_session_policy_ = AddUserSessionPolicy::ALLOWED;
Expand Down
4 changes: 2 additions & 2 deletions ash/common/session/session_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ class TestSessionStateObserver : public SessionStateObserver {
};

void FillDefaultSessionInfo(mojom::SessionInfo* info) {
info->max_users = 123;
info->can_lock_screen = true;
info->should_lock_screen_automatically = true;
info->add_user_session_policy = AddUserSessionPolicy::ALLOWED;
Expand Down Expand Up @@ -124,7 +123,8 @@ TEST_F(SessionControllerTest, SimpleSessionInfo) {
FillDefaultSessionInfo(&info);
SetSessionInfo(info);

EXPECT_EQ(123, controller()->GetMaximumNumberOfLoggedInUsers());
EXPECT_EQ(session_manager::kMaxmiumNumberOfUserSessions,
controller()->GetMaximumNumberOfLoggedInUsers());
EXPECT_TRUE(controller()->CanLockScreen());
EXPECT_TRUE(controller()->ShouldLockScreenAutomatically());

Expand Down
1 change: 0 additions & 1 deletion ash/mus/test/wm_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ void WmTestBase::SimulateUserLogin() {

// Simulate the user session becoming active.
mojom::SessionInfoPtr info = mojom::SessionInfo::New();
info->max_users = 10;
info->can_lock_screen = true;
info->should_lock_screen_automatically = false;
info->add_user_session_policy = AddUserSessionPolicy::ALLOWED;
Expand Down
3 changes: 0 additions & 3 deletions ash/public/interfaces/session_controller.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,6 @@ enum AddUserSessionPolicy {

// Info about an ash session.
struct SessionInfo {
// Maximum possible number of logged in users in ash.
uint32 max_users;

// Whether the screen can be locked.
bool can_lock_screen;

Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/ui/ash/session_controller_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ ash::AddUserSessionPolicy SessionControllerClient::GetAddUserSessionPolicy() {
}

if (UserManager::Get()->GetLoggedInUsers().size() >=
SessionManager::Get()->GetMaximumNumberOfUserSessions())
session_manager::kMaxmiumNumberOfUserSessions)
return ash::AddUserSessionPolicy::ERROR_MAXIMUM_USERS_REACHED;

return ash::AddUserSessionPolicy::ALLOWED;
Expand Down Expand Up @@ -256,7 +256,6 @@ void SessionControllerClient::SendSessionInfoIfChanged() {
SessionManager* const session_manager = SessionManager::Get();

ash::mojom::SessionInfoPtr info = ash::mojom::SessionInfo::New();
info->max_users = session_manager->GetMaximumNumberOfUserSessions();
info->can_lock_screen = CanLockScreen();
info->should_lock_screen_automatically = ShouldLockScreenAutomatically();
info->add_user_session_policy = GetAddUserSessionPolicy();
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/ash/session_state_delegate_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ int SessionStateDelegateChromeos::GetMaximumNumberOfLoggedInUsers() const {
// actually added to a session.
// TODO(nkostylev): Adjust this limitation based on device capabilites.
// http://crbug.com/230865
return 10;
return session_manager::kMaxmiumNumberOfUserSessions;
}

int SessionStateDelegateChromeos::NumberOfLoggedInUsers() const {
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,7 @@ void SystemTrayDelegateChromeOS::ShowUserLogin() {
}

if (user_manager::UserManager::Get()->GetLoggedInUsers().size() >=
session_manager::SessionManager::Get()
->GetMaximumNumberOfUserSessions()) {
session_manager::kMaxmiumNumberOfUserSessions) {
return;
}

Expand Down
5 changes: 0 additions & 5 deletions components/session_manager/core/session_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,6 @@ bool SessionManager::IsScreenLocked() const {
return session_state_ == SessionState::LOCKED;
}

uint32_t SessionManager::GetMaximumNumberOfUserSessions() const {
// Limits the number of logged in users to 10 due to memory constraints.
return 10u;
}

void SessionManager::AddObserver(SessionManagerObserver* observer) {
observers_.AddObserver(observer);
}
Expand Down
5 changes: 0 additions & 5 deletions components/session_manager/core/session_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
#ifndef COMPONENTS_SESSION_MANAGER_CORE_SESSION_MANAGER_H_
#define COMPONENTS_SESSION_MANAGER_CORE_SESSION_MANAGER_H_

#include <stdint.h>

#include <vector>

#include "base/macros.h"
Expand Down Expand Up @@ -55,9 +53,6 @@ class SESSION_EXPORT SessionManager {
bool IsInSecondaryLoginScreen() const;
bool IsScreenLocked() const;

// Returns the maximum number of allowed user sessions.
uint32_t GetMaximumNumberOfUserSessions() const;

void AddObserver(SessionManagerObserver* observer);
void RemoveObserver(SessionManagerObserver* observer);

Expand Down
3 changes: 3 additions & 0 deletions components/session_manager/session_manager_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ struct Session {
AccountId user_account_id;
};

// Limits the number of logged in users to 10 due to memory constraints.
constexpr int kMaxmiumNumberOfUserSessions = 10;

} // namespace session_manager

#endif // COMPONENTS_SESSION_MANAGER_SESSION_MANAGER_TYPES_H_

0 comments on commit 25ecee8

Please sign in to comment.