Reland cros: Views login backend for removing users.
Relands https://chromium-review.googlesource.com/c/chromium/src/+/935675. It was
reverted in https://chromium-review.googlesource.com/c/chromium/src/+/969228
TBR: xiyuan@chromium.org
TBR: dcheng@chromium.org
Bug: 809637
Change-Id: I9e44d96cbe26bc090e177460e18542b8b4d387b2
Reviewed-on: https://chromium-review.googlesource.com/971161
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544423}diff --git a/ash/login/login_screen_controller.cc b/ash/login/login_screen_controller.cc
index e024c81..1f4f88c 100644
--- a/ash/login/login_screen_controller.cc
+++ b/ash/login/login_screen_controller.cc
@@ -312,6 +312,18 @@
login_screen_client_->ShowGaiaSignin();
}
+void LoginScreenController::OnRemoveUserWarningShown() {
+ if (!login_screen_client_)
+ return;
+ login_screen_client_->OnRemoveUserWarningShown();
+}
+
+void LoginScreenController::RemoveUser(const AccountId& account_id) {
+ if (!login_screen_client_)
+ return;
+ login_screen_client_->RemoveUser(account_id);
+}
+
void LoginScreenController::AddLockScreenAppsFocusObserver(
LockScreenAppsFocusObserver* observer) {
lock_screen_apps_focus_observers_.AddObserver(observer);
diff --git a/ash/login/login_screen_controller.h b/ash/login/login_screen_controller.h
index f9460a75..c1a24b2 100644
--- a/ash/login/login_screen_controller.h
+++ b/ash/login/login_screen_controller.h
@@ -92,6 +92,8 @@
void OnMaxIncorrectPasswordAttempted(const AccountId& account_id);
void FocusLockScreenApps(bool reverse);
void ShowGaiaSignin();
+ void OnRemoveUserWarningShown();
+ void RemoveUser(const AccountId& account_id);
// Methods to manage lock screen apps focus observers.
// The observers will be notified when lock screen apps focus changes are
diff --git a/ash/login/mock_login_screen_client.cc b/ash/login/mock_login_screen_client.cc
index d70995c8..2ad25b4 100644
--- a/ash/login/mock_login_screen_client.cc
+++ b/ash/login/mock_login_screen_client.cc
@@ -36,9 +36,9 @@
}
std::unique_ptr<MockLoginScreenClient> BindMockLoginScreenClient() {
- LoginScreenController* controller = Shell::Get()->login_screen_controller();
auto client = std::make_unique<MockLoginScreenClient>();
- controller->SetClient(client->CreateInterfacePtrAndBind());
+ Shell::Get()->login_screen_controller()->SetClient(
+ client->CreateInterfacePtrAndBind());
return client;
}
diff --git a/ash/login/mock_login_screen_client.h b/ash/login/mock_login_screen_client.h
index c5026b16..5a9ce6d 100644
--- a/ash/login/mock_login_screen_client.h
+++ b/ash/login/mock_login_screen_client.h
@@ -59,6 +59,8 @@
void(const AccountId& account_id));
MOCK_METHOD1(FocusLockScreenApps, void(bool reverse));
MOCK_METHOD0(ShowGaiaSignin, void());
+ MOCK_METHOD0(OnRemoveUserWarningShown, void());
+ MOCK_METHOD1(RemoveUser, void(const AccountId& account_id));
private:
bool authenticate_user_callback_result_ = true;
diff --git a/ash/login/ui/lock_contents_view.cc b/ash/login/ui/lock_contents_view.cc
index 90b346d..b3a9ad4 100644
--- a/ash/login/ui/lock_contents_view.cc
+++ b/ash/login/ui/lock_contents_view.cc
@@ -814,7 +814,7 @@
void LockContentsView::SwapToAuthUser(int user_index) {
DCHECK(users_list_);
- auto* view = users_list_->GetUserViewAtIndex(user_index);
+ LoginUserView* view = users_list_->user_view_at(user_index);
DCHECK(view);
mojom::LoginUserInfoPtr previous_auth_user =
primary_auth_->current_user()->Clone();
@@ -826,6 +826,34 @@
OnAuthUserChanged();
}
+void LockContentsView::OnRemoveUserWarningShown(bool is_primary) {
+ Shell::Get()->login_screen_controller()->OnRemoveUserWarningShown();
+}
+
+void LockContentsView::RemoveUser(bool is_primary) {
+ LoginAuthUserView* to_remove =
+ is_primary ? primary_auth_ : opt_secondary_auth_;
+ DCHECK(to_remove->current_user()->can_remove);
+ AccountId user = to_remove->current_user()->basic_user_info->account_id;
+
+ // Ask chrome to remove the user.
+ Shell::Get()->login_screen_controller()->RemoveUser(user);
+
+ // Display the new user list less |user|.
+ std::vector<mojom::LoginUserInfoPtr> new_users;
+ if (!is_primary)
+ new_users.push_back(primary_auth_->current_user()->Clone());
+ if (is_primary && opt_secondary_auth_)
+ new_users.push_back(opt_secondary_auth_->current_user()->Clone());
+ if (users_list_) {
+ for (int i = 0; i < users_list_->user_count(); ++i) {
+ new_users.push_back(
+ users_list_->user_view_at(i)->current_user()->Clone());
+ }
+ }
+ data_dispatcher_->NotifyUsers(new_users);
+}
+
void LockContentsView::OnAuthUserChanged() {
const AccountId new_auth_user =
CurrentAuthUserView()->current_user()->basic_user_info->account_id;
@@ -961,15 +989,22 @@
LoginAuthUserView* LockContentsView::AllocateLoginAuthUserView(
const mojom::LoginUserInfoPtr& user,
bool is_primary) {
- return new LoginAuthUserView(
- user,
- base::Bind(&LockContentsView::OnAuthenticate, base::Unretained(this)),
- base::Bind(&LockContentsView::SwapActiveAuthBetweenPrimaryAndSecondary,
- base::Unretained(this), is_primary),
- base::Bind(&LockContentsView::OnEasyUnlockIconHovered,
- base::Unretained(this)),
- base::Bind(&LockContentsView::OnEasyUnlockIconTapped,
- base::Unretained(this)));
+ LoginAuthUserView::Callbacks callbacks;
+ callbacks.on_auth = base::BindRepeating(&LockContentsView::OnAuthenticate,
+ base::Unretained(this)),
+ callbacks.on_tap = base::BindRepeating(
+ &LockContentsView::SwapActiveAuthBetweenPrimaryAndSecondary,
+ base::Unretained(this), is_primary),
+ callbacks.on_remove_warning_shown =
+ base::BindRepeating(&LockContentsView::OnRemoveUserWarningShown,
+ base::Unretained(this), is_primary);
+ callbacks.on_remove = base::BindRepeating(&LockContentsView::RemoveUser,
+ base::Unretained(this), is_primary);
+ callbacks.on_easy_unlock_icon_hovered = base::BindRepeating(
+ &LockContentsView::OnEasyUnlockIconHovered, base::Unretained(this));
+ callbacks.on_easy_unlock_icon_tapped = base::BindRepeating(
+ &LockContentsView::OnEasyUnlockIconTapped, base::Unretained(this));
+ return new LoginAuthUserView(user, callbacks);
}
LoginAuthUserView* LockContentsView::TryToFindAuthUser(
diff --git a/ash/login/ui/lock_contents_view.h b/ash/login/ui/lock_contents_view.h
index 4c0ee10..a0d2eee 100644
--- a/ash/login/ui/lock_contents_view.h
+++ b/ash/login/ui/lock_contents_view.h
@@ -213,6 +213,11 @@
// the actual user may change.
void SwapToAuthUser(int user_index);
+ // Warning to remove a user is shown.
+ void OnRemoveUserWarningShown(bool is_primary);
+ // Remove one of the auth users.
+ void RemoveUser(bool is_primary);
+
// Called after the auth user change has taken place.
void OnAuthUserChanged();
diff --git a/ash/login/ui/lock_screen_sanity_unittest.cc b/ash/login/ui/lock_screen_sanity_unittest.cc
index bcb5723f..2ecbecb 100644
--- a/ash/login/ui/lock_screen_sanity_unittest.cc
+++ b/ash/login/ui/lock_screen_sanity_unittest.cc
@@ -8,6 +8,8 @@
#include "ash/login/mock_login_screen_client.h"
#include "ash/login/ui/fake_login_detachable_base_model.h"
#include "ash/login/ui/lock_contents_view.h"
+#include "ash/login/ui/login_auth_user_view.h"
+#include "ash/login/ui/login_bubble.h"
#include "ash/login/ui/login_test_base.h"
#include "ash/login/ui/login_test_utils.h"
#include "ash/public/cpp/config.h"
@@ -113,8 +115,9 @@
std::unique_ptr<views::Widget> widget = CreateWidgetWithContent(contents);
// Textfield should have focus.
- EXPECT_EQ(MakeLoginPasswordTestApi(contents).textfield(),
- contents->GetFocusManager()->GetFocusedView());
+ EXPECT_EQ(
+ MakeLoginPasswordTestApi(contents, AuthTarget::kPrimary).textfield(),
+ contents->GetFocusManager()->GetFocusedView());
}
// Verifies submitting the password invokes mojo lock screen client.
@@ -153,7 +156,7 @@
SetUserCount(1);
std::unique_ptr<views::Widget> widget = CreateWidgetWithContent(contents);
LoginPasswordView::TestApi password_test_api =
- MakeLoginPasswordTestApi(contents);
+ MakeLoginPasswordTestApi(contents, AuthTarget::kPrimary);
MockLoginScreenClient::AuthenticateUserCallback callback;
auto submit_password = [&]() {
@@ -370,4 +373,73 @@
EXPECT_TRUE(VerifyFocused(shelf));
}
+TEST_F(LockScreenSanityTest, RemoveUser) {
+ std::unique_ptr<MockLoginScreenClient> client = BindMockLoginScreenClient();
+ LoginScreenController* controller =
+ ash::Shell::Get()->login_screen_controller();
+
+ auto* contents = new LockContentsView(
+ mojom::TrayActionState::kAvailable, data_dispatcher(),
+ std::make_unique<FakeLoginDetachableBaseModel>(data_dispatcher()));
+
+ // Add two users, the first of which can be removed.
+ users().push_back(CreateUser("test1@test"));
+ users()[0]->can_remove = true;
+ users().push_back(CreateUser("test2@test"));
+ data_dispatcher()->NotifyUsers(users());
+
+ std::unique_ptr<views::Widget> widget = CreateWidgetWithContent(contents);
+
+ auto primary = [&]() {
+ return LoginUserView::TestApi(
+ MakeLoginAuthTestApi(contents, AuthTarget::kPrimary).user_view());
+ };
+ auto secondary = [&]() {
+ return LoginUserView::TestApi(
+ MakeLoginAuthTestApi(contents, AuthTarget::kSecondary).user_view());
+ };
+
+ // Fires a return and validates that mock expectations have been satisfied.
+ auto submit = [&]() {
+ GetEventGenerator().PressKey(ui::VKEY_RETURN, 0);
+ controller->FlushForTesting();
+ testing::Mock::VerifyAndClearExpectations(client.get());
+ };
+ auto focus_and_submit = [&](views::View* view) {
+ view->RequestFocus();
+ DCHECK(view->HasFocus());
+ submit();
+ };
+
+ // The secondary user is not removable (as configured above) so showing the
+ // dropdown does not result in an interactive/focusable view.
+ focus_and_submit(secondary().dropdown());
+ EXPECT_TRUE(secondary().menu());
+ EXPECT_FALSE(
+ HasFocusInAnyChildView(secondary().menu()->bubble_view_for_test()));
+ // TODO(jdufault): Run submit() and then EXPECT_FALSE(secondary().menu()); to
+ // verify that double-enter closes the bubble.
+
+ // The primary user is removable, so the menu is interactive. Submitting the
+ // first time shows the remove user warning, submitting the second time
+ // actually removes the user. Removing the user triggers a mojo API call as
+ // well as removes the user from the UI.
+ focus_and_submit(primary().dropdown());
+ EXPECT_TRUE(primary().menu());
+ EXPECT_TRUE(HasFocusInAnyChildView(primary().menu()->bubble_view_for_test()));
+ EXPECT_CALL(*client, OnRemoveUserWarningShown()).Times(1);
+ submit();
+ EXPECT_CALL(*client, RemoveUser(users()[0]->basic_user_info->account_id))
+ .Times(1);
+ submit();
+
+ // Secondary auth should be gone because it is now the primary auth.
+ EXPECT_FALSE(MakeLockContentsViewTestApi(contents).opt_secondary_auth());
+ EXPECT_TRUE(MakeLockContentsViewTestApi(contents)
+ .primary_auth()
+ ->current_user()
+ ->basic_user_info->account_id ==
+ users()[1]->basic_user_info->account_id);
+}
+
} // namespace ash
diff --git a/ash/login/ui/login_auth_user_view.cc b/ash/login/ui/login_auth_user_view.cc
index 9e7dca8..ddf306b 100644
--- a/ash/login/ui/login_auth_user_view.cc
+++ b/ash/login/ui/login_auth_user_view.cc
@@ -101,23 +101,31 @@
return view_->pin_view_;
}
-LoginAuthUserView::LoginAuthUserView(
- const mojom::LoginUserInfoPtr& user,
- const OnAuthCallback& on_auth,
- const LoginUserView::OnTap& on_tap,
- const OnEasyUnlockIconHovered& on_easy_unlock_icon_hovered,
- const OnEasyUnlockIconTapped& on_easy_unlock_icon_tapped)
+LoginAuthUserView::Callbacks::Callbacks() {}
+
+LoginAuthUserView::Callbacks::~Callbacks() {}
+
+LoginAuthUserView::LoginAuthUserView(const mojom::LoginUserInfoPtr& user,
+ const Callbacks& callbacks)
: NonAccessibleView(kLoginAuthUserViewClassName),
- on_auth_(on_auth),
- on_tap_(on_tap),
+ on_auth_(callbacks.on_auth),
+ on_tap_(callbacks.on_tap),
weak_factory_(this) {
+ DCHECK(callbacks.on_auth);
+ DCHECK(callbacks.on_tap);
+ DCHECK(callbacks.on_remove_warning_shown);
+ DCHECK(callbacks.on_remove);
+ DCHECK(callbacks.on_easy_unlock_icon_hovered);
+ DCHECK(callbacks.on_easy_unlock_icon_tapped);
DCHECK_NE(user->basic_user_info->type,
user_manager::USER_TYPE_PUBLIC_ACCOUNT);
// Build child views.
user_view_ = new LoginUserView(
LoginDisplayStyle::kLarge, true /*show_dropdown*/, false /*show_domain*/,
- base::Bind(&LoginAuthUserView::OnUserViewTap, base::Unretained(this)));
+ base::BindRepeating(&LoginAuthUserView::OnUserViewTap,
+ base::Unretained(this)),
+ callbacks.on_remove_warning_shown, callbacks.on_remove);
password_view_ = new LoginPasswordView();
password_view_->SetPaintToLayer(); // Needed for opacity animation.
@@ -137,7 +145,8 @@
base::Bind(&LoginAuthUserView::OnAuthSubmit, base::Unretained(this)),
base::Bind(&LoginPinView::OnPasswordTextChanged,
base::Unretained(pin_view_)),
- on_easy_unlock_icon_hovered, on_easy_unlock_icon_tapped);
+ callbacks.on_easy_unlock_icon_hovered,
+ callbacks.on_easy_unlock_icon_tapped);
// Child views animate outside view bounds.
SetPaintToLayer(ui::LayerType::LAYER_NOT_DRAWN);
diff --git a/ash/login/ui/login_auth_user_view.h b/ash/login/ui/login_auth_user_view.h
index 6441f86e0..806aba53 100644
--- a/ash/login/ui/login_auth_user_view.h
+++ b/ash/login/ui/login_auth_user_view.h
@@ -44,6 +44,30 @@
LoginAuthUserView* const view_;
};
+ using auth_success)>;
+ using >
+ using >
+
+ struct Callbacks {
+ Callbacks();
+ ~Callbacks();
+
+ // Executed whenever an authentication result is available, such as when the
+ // user submits a password or taps the user icon when AUTH_TAP is enabled.
+ OnAuthCallback on_auth;
+ // Called when the user taps the user view and AUTH_TAP is not enabled.
+ LoginUserView::OnTap on_tap;
+ // Called when the remove user warning message has been shown.
+ LoginUserView::OnRemoveWarningShown on_remove_warning_shown;
+ // Called when the user should be removed. The callback should do the actual
+ // removal.
+ LoginUserView::OnRemove on_remove;
+ // Called when the easy unlock icon is hovered.
+ OnEasyUnlockIconHovered on_easy_unlock_icon_hovered;
+ // Called when the easy unlock icon is tapped.
+ OnEasyUnlockIconTapped on_easy_unlock_icon_tapped;
+ };
+
// Flags which describe the set of currently visible auth methods.
enum AuthMethods {
AUTH_NONE = 0, // No extra auth methods.
@@ -52,23 +76,8 @@
AUTH_TAP = 1 << 2, // Tap to unlock.
};
- using auth_success)>;
- using >
- using >
-
- // |on_auth| is executed whenever an authentication result is available, such
- // as when the user submits a password or taps the user icon when AUTH_TAP is
- // enabled.
- //
- // |on_tap| is called when the user taps the user view and AUTH_TAP is not
- // enabled.
- //
- // None of the callbacks can be null.
LoginAuthUserView(const mojom::LoginUserInfoPtr& user,
- const OnAuthCallback& on_auth,
- const LoginUserView::OnTap& on_tap,
- const OnEasyUnlockIconHovered& on_easy_unlock_icon_hovered,
- const OnEasyUnlockIconTapped& on_easy_unlock_icon_tapped);
+ const Callbacks& callbacks);
~LoginAuthUserView() override;
// Set the displayed set of auth methods. |auth_methods| contains or-ed
diff --git a/ash/login/ui/login_auth_user_view_unittest.cc b/ash/login/ui/login_auth_user_view_unittest.cc
index e85c624..fcfe0f4 100644
--- a/ash/login/ui/login_auth_user_view_unittest.cc
+++ b/ash/login/ui/login_auth_user_view_unittest.cc
@@ -24,8 +24,15 @@
LoginTestBase::SetUp();
user_ = CreateUser("user@domain.com");
- view_ = new LoginAuthUserView(user_, base::DoNothing(), base::DoNothing(),
- base::DoNothing(), base::DoNothing());
+
+ LoginAuthUserView::Callbacks auth_callbacks;
+ auth_callbacks.on_auth = base::DoNothing();
+ auth_callbacks.on_easy_unlock_icon_hovered = base::DoNothing();
+ auth_callbacks.on_easy_unlock_icon_tapped = base::DoNothing();
+ auth_callbacks.on_tap = base::DoNothing();
+ auth_callbacks.on_remove_warning_shown = base::DoNothing();
+ auth_callbacks.on_remove = base::DoNothing();
+ view_ = new LoginAuthUserView(user_, auth_callbacks);
// We proxy |view_| inside of |container_| so we can control layout.
container_ = new views::View();
diff --git a/ash/login/ui/login_bubble.cc b/ash/login/ui/login_bubble.cc
index 50867ed..8e1d5ce 100644
--- a/ash/login/ui/login_bubble.cc
+++ b/ash/login/ui/login_bubble.cc
@@ -155,10 +155,12 @@
bool is_owner,
views::View* anchor_view,
bool show_remove_user,
- base::OnceClosure do_remove_user)
+ base::OnceClosure on_remove_user_warning_shown,
+ base::OnceClosure on_remove_user_requested)
: LoginBaseBubbleView(anchor_view),
bubble_(bubble),
- do_remove_user_(std::move(do_remove_user)) {
+ on_remove_user_warning_shown_(std::move(on_remove_user_warning_shown)),
+ on_remove_user_requested_(std::move(on_remove_user_requested)) {
// This view has content the user can interact with if the remove user
// button is displayed.
set_can_activate(show_remove_user);
@@ -308,17 +310,23 @@
SizeToContents();
GetWidget()->SetSize(size());
Layout();
+ if (on_remove_user_warning_shown_)
+ std::move(on_remove_user_warning_shown_).Run();
return;
}
- if (do_remove_user_)
- std::move(do_remove_user_).Run();
+ // Close the bubble before calling |on_remove_user_requested_|. |bubble_| is
+ // an unowned reference; |on_remove_user_requested_| may delete it.
bubble_->Close();
+
+ if (on_remove_user_requested_)
+ std::move(on_remove_user_requested_).Run();
}
private:
LoginBubble* bubble_ = nullptr;
- base::OnceClosure do_remove_user_;
+ base::OnceClosure on_remove_user_warning_shown_;
+ base::OnceClosure on_remove_user_requested_;
views::View* remove_user_confirm_data_ = nullptr;
views::Label* remove_user_label_ = nullptr;
ButtonWithContent* remove_user_button_ = nullptr;
@@ -382,19 +390,19 @@
views::View* anchor_view,
LoginButton* bubble_opener,
bool show_remove_user,
- base::OnceClosure do_remove_user) {
+ base::OnceClosure on_remove_user_warning_shown,
+ base::OnceClosure on_remove_user_requested) {
if (bubble_view_)
CloseImmediately();
flags_ = kFlagsNone;
bubble_opener_ = bubble_opener;
- bubble_view_ =
- new LoginUserMenuView(this, username, email, type, is_owner, anchor_view,
- show_remove_user, std::move(do_remove_user));
+ bubble_view_ = new LoginUserMenuView(this, username, email, type, is_owner,
+ anchor_view, show_remove_user,
+ std::move(on_remove_user_warning_shown),
+ std::move(on_remove_user_requested));
bool had_focus = bubble_opener_->HasFocus();
-
Show();
-
if (had_focus) {
// Try to focus the bubble view only if the tooltip was focused.
bubble_view_->RequestFocus();
diff --git a/ash/login/ui/login_bubble.h b/ash/login/ui/login_bubble.h
index 52be89c..ff1903a 100644
--- a/ash/login/ui/login_bubble.h
+++ b/ash/login/ui/login_bubble.h
@@ -52,7 +52,8 @@
views::View* anchor_view,
LoginButton* bubble_opener,
bool show_remove_user,
- base::OnceClosure do_remove_user);
+ base::OnceClosure on_remove_user_warning_shown,
+ base::OnceClosure on_remove_user_requested);
// Shows a tooltip.
void ShowTooltip(const base::string16& message, views::View* anchor_view);
diff --git a/ash/login/ui/login_bubble_unittest.cc b/ash/login/ui/login_bubble_unittest.cc
index cbf07da7..1f4b83c3 100644
--- a/ash/login/ui/login_bubble_unittest.cc
+++ b/ash/login/ui/login_bubble_unittest.cc
@@ -69,12 +69,14 @@
LoginTestBase::TearDown();
}
- void ShowUserMenu(base::OnceClosure on_remove) {
+ void ShowUserMenu(base::OnceClosure on_remove_show_warning,
+ base::OnceClosure on_remove) {
bool show_remove_user = !on_remove.is_null();
bubble_->ShowUserMenu(
base::string16() /*username*/, base::string16() /*email*/,
user_manager::UserType::USER_TYPE_REGULAR, false /*is_owner*/,
- container_, bubble_opener_, show_remove_user, std::move(on_remove));
+ container_, bubble_opener_, show_remove_user,
+ std::move(on_remove_show_warning), std::move(on_remove));
}
// Owned by test widget view hierarchy.
@@ -118,7 +120,7 @@
EXPECT_FALSE(bubble_->IsVisible());
// Verifies that key event on the bubble opener view won't close the bubble.
- ShowUserMenu(base::OnceClosure());
+ ShowUserMenu(base::OnceClosure(), base::OnceClosure());
EXPECT_TRUE(bubble_->IsVisible());
bubble_opener_->RequestFocus();
generator.PressKey(ui::KeyboardCode::VKEY_A, ui::EF_NONE);
@@ -141,7 +143,7 @@
EXPECT_FALSE(bubble_->IsVisible());
// Verifies that mouse event on the bubble opener view won't close the bubble.
- ShowUserMenu(base::OnceClosure());
+ ShowUserMenu(base::OnceClosure(), base::OnceClosure());
EXPECT_TRUE(bubble_->IsVisible());
generator.MoveMouseTo(bubble_opener_->GetBoundsInScreen().CenterPoint());
generator.ClickLeftButton();
@@ -170,7 +172,7 @@
// Verifies that gesture event on the bubble opener view won't close the
// bubble.
- ShowUserMenu(base::OnceClosure());
+ ShowUserMenu(base::OnceClosure(), base::OnceClosure());
EXPECT_TRUE(bubble_->IsVisible());
generator.GestureTapAt(bubble_opener_->GetBoundsInScreen().CenterPoint());
EXPECT_TRUE(bubble_->IsVisible());
@@ -192,7 +194,7 @@
views::InkDropHostView::InkDropMode::ON);
// Show the bubble to activate the ripple effect.
- ShowUserMenu(base::OnceClosure());
+ ShowUserMenu(base::OnceClosure(), base::OnceClosure());
EXPECT_TRUE(bubble_->IsVisible());
EXPECT_TRUE(ink_drop_api.HasInkDrop());
EXPECT_EQ(ink_drop_api.GetInkDrop()->GetTargetInkDropState(),
@@ -216,9 +218,14 @@
// callback.
TEST_F(LoginBubbleTest, RemoveUserRequiresTwoActivations) {
// Show the user menu.
+ bool remove_warning_called = false;
bool remove_called = false;
- ShowUserMenu(base::BindOnce(
- [](bool* remove_called) { *remove_called = true; }, &remove_called));
+ ShowUserMenu(
+ base::BindOnce(
+ [](bool* remove_warning_called) { *remove_warning_called = true; },
+ &remove_warning_called),
+ base::BindOnce([](bool* remove_called) { *remove_called = true; },
+ &remove_called));
EXPECT_TRUE(bubble_->IsVisible());
// Focus the remove user button.
@@ -228,14 +235,20 @@
remove_user_button->RequestFocus();
EXPECT_TRUE(remove_user_button->HasFocus());
- // Click it twice. Verify only the second click fires the callback.
auto click = [&]() {
EXPECT_TRUE(remove_user_button->HasFocus());
GetEventGenerator().PressKey(ui::KeyboardCode::VKEY_RETURN, 0);
};
+
+ // First click calls remove warning.
EXPECT_NO_FATAL_FAILURE(click());
+ EXPECT_TRUE(remove_warning_called);
EXPECT_FALSE(remove_called);
+ remove_warning_called = false;
+
+ // Second click calls remove.
EXPECT_NO_FATAL_FAILURE(click());
+ EXPECT_FALSE(remove_warning_called);
EXPECT_TRUE(remove_called);
}
diff --git a/ash/login/ui/login_test_base.h b/ash/login/ui/login_test_base.h
index 205acfc6..7605055 100644
--- a/ash/login/ui/login_test_base.h
+++ b/ash/login/ui/login_test_base.h
@@ -40,6 +40,7 @@
// Changes the active number of users. Fires an event on |data_dispatcher()|.
void SetUserCount(size_t count);
+ std::vector<mojom::LoginUserInfoPtr>& users() { return users_; }
const std::vector<mojom::LoginUserInfoPtr>& users() const { return users_; }
LoginDataDispatcher* data_dispatcher() { return &data_dispatcher_; }
diff --git a/ash/login/ui/login_test_utils.cc b/ash/login/ui/login_test_utils.cc
index 8e8c23c9..46a5522 100644
--- a/ash/login/ui/login_test_utils.cc
+++ b/ash/login/ui/login_test_utils.cc
@@ -11,14 +11,24 @@
return LockContentsView::TestApi(view);
}
-LoginAuthUserView::TestApi MakeLoginPrimaryAuthTestApi(LockContentsView* view) {
- return LoginAuthUserView::TestApi(
- MakeLockContentsViewTestApi(view).primary_auth());
+LoginAuthUserView::TestApi MakeLoginAuthTestApi(LockContentsView* view,
+ AuthTarget target) {
+ switch (target) {
+ case AuthTarget::kPrimary:
+ return LoginAuthUserView::TestApi(
+ MakeLockContentsViewTestApi(view).primary_auth());
+ case AuthTarget::kSecondary:
+ return LoginAuthUserView::TestApi(
+ MakeLockContentsViewTestApi(view).opt_secondary_auth());
+ }
+
+ NOTREACHED();
}
-LoginPasswordView::TestApi MakeLoginPasswordTestApi(LockContentsView* view) {
+LoginPasswordView::TestApi MakeLoginPasswordTestApi(LockContentsView* view,
+ AuthTarget target) {
return LoginPasswordView::TestApi(
- MakeLoginPrimaryAuthTestApi(view).password_view());
+ MakeLoginAuthTestApi(view, target).password_view());
}
mojom::LoginUserInfoPtr CreateUser(const std::string& email) {
diff --git a/ash/login/ui/login_test_utils.h b/ash/login/ui/login_test_utils.h
index ad02dbf0..91fc4da 100644
--- a/ash/login/ui/login_test_utils.h
+++ b/ash/login/ui/login_test_utils.h
@@ -12,10 +12,14 @@
namespace ash {
+enum class AuthTarget { kPrimary, kSecondary };
+
// Helpers for constructing TestApi instances.
LockContentsView::TestApi MakeLockContentsViewTestApi(LockContentsView* view);
-LoginAuthUserView::TestApi MakeLoginPrimaryAuthTestApi(LockContentsView* view);
-LoginPasswordView::TestApi MakeLoginPasswordTestApi(LockContentsView* view);
+LoginAuthUserView::TestApi MakeLoginAuthTestApi(LockContentsView* view,
+ AuthTarget auth);
+LoginPasswordView::TestApi MakeLoginPasswordTestApi(LockContentsView* view,
+ AuthTarget auth);
// Utility method to create a new |mojom::UserInfoPtr| instance.
mojom::LoginUserInfoPtr CreateUser(const std::string& email);
diff --git a/ash/login/ui/login_user_view.cc b/ash/login/ui/login_user_view.cc
index 7210fad..400dc67 100644
--- a/ash/login/ui/login_user_view.cc
+++ b/ash/login/ui/login_user_view.cc
@@ -294,6 +294,14 @@
return view_->tap_button_;
}
+views::View* LoginUserView::TestApi::dropdown() const {
+ return view_->user_dropdown_;
+}
+
+LoginBubble* LoginUserView::TestApi::menu() const {
+ return view_->user_menu_.get();
+}
+
bool LoginUserView::TestApi::is_opaque() const {
return view_->is_opaque_;
}
@@ -313,15 +321,25 @@
return 0;
}
-LoginUserView::LoginUserView(LoginDisplayStyle style,
- bool show_dropdown,
- bool show_domain,
- const OnTap& on_tap)
- : on_tap_(on_tap), display_style_(style) {
- // show_dropdown and show_domain can only be true when the user view is
- // rendering in large mode.
+LoginUserView::LoginUserView(
+ LoginDisplayStyle style,
+ bool show_dropdown,
+ bool show_domain,
+ const OnTap& on_tap,
+ const OnRemoveWarningShown& on_remove_warning_shown,
+ const OnRemove& on_remove)
+ : on_tap_(on_tap),
+ on_remove_warning_shown_(on_remove_warning_shown),
+ on_remove_(on_remove),
+ display_style_(style) {
+ // show_dropdown can only be true when the user view is rendering in large
+ // mode.
DCHECK(!show_dropdown || style == LoginDisplayStyle::kLarge);
DCHECK(!show_domain || style == LoginDisplayStyle::kLarge);
+ // |on_remove_warning_shown| and |on_remove| is only available iff
+ // |show_dropdown| is true.
+ DCHECK(show_dropdown == !!on_remove_warning_shown);
+ DCHECK(show_dropdown == !!on_remove);
user_image_ = new UserImage(GetImageSize(style));
user_label_ = new UserLabel(style);
@@ -478,7 +496,7 @@
current_user_->basic_user_info->type, current_user_->is_device_owner,
user_dropdown_ /*anchor_view*/, user_dropdown_ /*bubble_opener*/,
current_user_->can_remove /*show_remove_user*/,
- base::OnceClosure() /*do_remove_user*/);
+ on_remove_warning_shown_, on_remove_);
} else {
user_menu_->Close();
}
diff --git a/ash/login/ui/login_user_view.h b/ash/login/ui/login_user_view.h
index 4d27549c..45241b62 100644
--- a/ash/login/ui/login_user_view.h
+++ b/ash/login/ui/login_user_view.h
@@ -35,6 +35,8 @@
views::View* user_label() const;
views::View* tap_button() const;
+ views::View* dropdown() const;
+ LoginBubble* menu() const;
bool is_opaque() const;
@@ -43,6 +45,8 @@
};
using >
+ using >
+ using >
// Returns the width of this view for the given display style.
static int WidthForLayoutStyle(LoginDisplayStyle style);
@@ -50,7 +54,9 @@
LoginUserView(LoginDisplayStyle style,
bool show_dropdown,
bool show_domain,
- const OnTap& on_tap);
+ const OnTap& on_tap,
+ const OnRemoveWarningShown& on_remove_warning_shown,
+ const OnRemove& on_remove);
~LoginUserView() override;
// Update the user view to display the given user information.
@@ -91,6 +97,10 @@
// Executed when the user view is pressed.
OnTap on_tap_;
+ // Executed when the user has seen the remove user warning.
+ OnRemoveWarningShown on_remove_warning_shown_;
+ // Executed when a user-remove has been requested.
+ OnRemove on_remove_;
// The user that is currently being displayed (or will be displayed when an
// animation completes).
@@ -102,8 +112,10 @@
LoginDisplayStyle display_style_;
UserImage* user_image_ = nullptr;
UserLabel* user_label_ = nullptr;
+ // TODO(jdufault): Rename user_dropdown_ to dropdown_.
LoginButton* user_dropdown_ = nullptr;
TapButton* tap_button_ = nullptr;
+ // TODO(jdufault): Rename user_menu_ to menu_ or popup_menu_.
std::unique_ptr<LoginBubble> user_menu_;
// Show the domain information for public account user.
diff --git a/ash/login/ui/login_user_view_unittest.cc b/ash/login/ui/login_user_view_unittest.cc
index 65406597..65faed8 100644
--- a/ash/login/ui/login_user_view_unittest.cc
+++ b/ash/login/ui/login_user_view_unittest.cc
@@ -25,10 +25,20 @@
LoginUserView* AddUserView(LoginDisplayStyle display_style,
bool show_dropdown) {
// TODO(crbug.com/809635): Add test case for show_domain.
+ LoginUserView::OnRemoveWarningShown on_remove_warning_shown;
+ LoginUserView::OnRemove on_remove;
+ if (show_dropdown) {
+ on_remove_warning_shown = base::BindRepeating(
+ &LoginUserViewUnittest::OnRemoveWarningShown, base::Unretained(this));
+ on_remove = base::BindRepeating(&LoginUserViewUnittest::OnRemove,
+ base::Unretained(this));
+ }
+
auto* view =
new LoginUserView(display_style, show_dropdown, false /*show_domain*/,
base::BindRepeating(&LoginUserViewUnittest::OnTapped,
- base::Unretained(this)));
+ base::Unretained(this)),
+ on_remove_warning_shown, on_remove);
mojom::LoginUserInfoPtr user = CreateUser("foo@foo.com");
view->UpdateForUser(user, false /*animate*/);
container_->AddChildView(view);
@@ -52,11 +62,15 @@
}
int tap_count_ = 0;
+ int remove_show_warning_count_ = 0;
+ int remove_count_ = 0;
views::View* container_ = nullptr; // Owned by test widget view hierarchy.
private:
void OnTapped() { ++tap_count_; }
+ void OnRemoveWarningShown() { ++remove_show_warning_count_; }
+ void OnRemove() { ++remove_count_; }
DISALLOW_COPY_AND_ASSIGN(LoginUserViewUnittest);
};
diff --git a/ash/login/ui/scrollable_users_list_view.cc b/ash/login/ui/scrollable_users_list_view.cc
index 4ad0497d..08e87b8b 100644
--- a/ash/login/ui/scrollable_users_list_view.cc
+++ b/ash/login/ui/scrollable_users_list_view.cc
@@ -117,7 +117,7 @@
ScrollableUsersListView::ScrollableUsersListView(
const std::vector<mojom::LoginUserInfoPtr>& users,
- const OnUserViewTap& on_user_view_tap,
+ const ActionWithUser& on_tap_user,
LoginDisplayStyle display_style)
: views::ScrollView() {
layout_params_ = GetLayoutParams(display_style);
@@ -136,8 +136,8 @@
for (std::size_t i = 1u; i < users.size(); ++i) {
auto* view = new LoginUserView(
layout_params_.display_style, false /*show_dropdown*/,
- false /*show_domain*/,
- base::BindRepeating(on_user_view_tap, i - 1) /*on_tap*/);
+ false /*show_domain*/, base::BindRepeating(on_tap_user, i - 1),
+ base::RepeatingClosure(), base::RepeatingClosure());
user_views_.push_back(view);
view->UpdateForUser(users[i], false /*animate*/);
contents->AddChildView(view);
@@ -158,11 +158,6 @@
ScrollableUsersListView::~ScrollableUsersListView() = default;
-LoginUserView* ScrollableUsersListView::GetUserViewAtIndex(int index) {
- return static_cast<size_t>(index) < user_views_.size() ? user_views_[index]
- : nullptr;
-}
-
void ScrollableUsersListView::Layout() {
DCHECK(layout_);
bool should_show_landscape =
diff --git a/ash/login/ui/scrollable_users_list_view.h b/ash/login/ui/scrollable_users_list_view.h
index 6a4e91a..99e433e 100644
--- a/ash/login/ui/scrollable_users_list_view.h
+++ b/ash/login/ui/scrollable_users_list_view.h
@@ -9,6 +9,7 @@
#include "ash/ash_export.h"
#include "ash/login/ui/login_display_style.h"
+#include "ash/login/ui/login_user_view.h"
#include "ash/public/interfaces/login_user_info.mojom.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/views/controls/scroll_view.h"
@@ -21,7 +22,6 @@
namespace ash {
class HoverNotifier;
-class LoginUserView;
class ScrollBar;
// Scrollable list of the users. Stores the list of login user views. Can be
@@ -41,18 +41,24 @@
ScrollableUsersListView* const view_;
};
- using >
+ // TODO(jdufault): Pass AccountId or LoginUserView* instead of index.
+ using ActionWithUser = base::RepeatingCallback<void(int)>;
// Initializes users list with rows for all |users|. The |display_style| is
// used to determine layout and sizings. |on_user_view_tap| callback is
// invoked whenever user row is tapped.
ScrollableUsersListView(const std::vector<mojom::LoginUserInfoPtr>& users,
- const OnUserViewTap& on_user_view_tap,
+ const ActionWithUser& on_tap_user,
LoginDisplayStyle display_style);
~ScrollableUsersListView() override;
// Returns user view at |index| if it exists or nullptr otherwise.
- LoginUserView* GetUserViewAtIndex(int index);
+ int user_count() const { return static_cast<int>(user_views_.size()); }
+ LoginUserView* user_view_at(int index) {
+ DCHECK_GE(index, 0);
+ DCHECK_LT(index, user_count());
+ return user_views_[index];
+ }
// views::View:
void Layout() override;
@@ -104,4 +110,4 @@
} // namespace ash
-#endif // ASH_LOGIN_UI_SCROLLABLE_USERS_LIST_VIEW_H_
\ No newline at end of file
+#endif // ASH_LOGIN_UI_SCROLLABLE_USERS_LIST_VIEW_H_
diff --git a/ash/metrics/login_metrics_recorder_unittest.cc b/ash/metrics/login_metrics_recorder_unittest.cc
index ff437773..5a8224b 100644
--- a/ash/metrics/login_metrics_recorder_unittest.cc
+++ b/ash/metrics/login_metrics_recorder_unittest.cc
@@ -169,7 +169,7 @@
EXPECT_EQ(test_api.primary_auth()->auth_methods(),
(auth_method | LoginAuthUserView::AUTH_TAP));
EXPECT_CALL(*client, AttemptUnlock(primary_user));
- generator.MoveMouseTo(MakeLoginPrimaryAuthTestApi(contents)
+ generator.MoveMouseTo(MakeLoginAuthTestApi(contents, AuthTarget::kPrimary)
.user_view()
->GetBoundsInScreen()
.CenterPoint());
diff --git a/ash/public/interfaces/login_screen.mojom b/ash/public/interfaces/login_screen.mojom
index 5b50918..c614cc58 100644
--- a/ash/public/interfaces/login_screen.mojom
+++ b/ash/public/interfaces/login_screen.mojom
@@ -166,4 +166,10 @@
// Show Gaia signin dialog in chrome.
ShowGaiaSignin();
+
+ // Notification that the remove user warning was shown.
+ OnRemoveUserWarningShown();
+
+ // Try to remove |account_id|.
+ RemoveUser(signin.mojom.AccountId account_id);
};
diff --git a/chrome/browser/ui/ash/login_screen_client.cc b/chrome/browser/ui/ash/login_screen_client.cc
index 66332a88..7142062 100644
--- a/chrome/browser/ui/ash/login_screen_client.cc
+++ b/chrome/browser/ui/ash/login_screen_client.cc
@@ -11,7 +11,9 @@
#include "chrome/browser/chromeos/login/reauth_stats.h"
#include "chrome/browser/chromeos/login/ui/login_display_host.h"
#include "chrome/browser/chromeos/login/ui/user_adding_screen.h"
+#include "chrome/browser/profiles/profile_metrics.h"
#include "chrome/browser/ui/ash/wallpaper_controller_client.h"
+#include "components/user_manager/remove_user_delegate.h"
#include "content/public/common/service_manager_connection.h"
#include "services/service_manager/public/cpp/connector.h"
@@ -115,6 +117,18 @@
}
}
+void LoginScreenClient::OnRemoveUserWarningShown() {
+ ProfileMetrics::LogProfileDeleteUser(
+ ProfileMetrics::DELETE_PROFILE_USER_MANAGER_SHOW_WARNING);
+}
+
+void LoginScreenClient::RemoveUser(const AccountId& account_id) {
+ ProfileMetrics::LogProfileDeleteUser(
+ ProfileMetrics::DELETE_PROFILE_USER_MANAGER);
+ user_manager::UserManager::Get()->RemoveUser(account_id,
+ nullptr /*delegate*/);
+}
+
void LoginScreenClient::LoadWallpaper(const AccountId& account_id) {
WallpaperControllerClient::Get()->ShowUserWallpaper(account_id);
}
diff --git a/chrome/browser/ui/ash/login_screen_client.h b/chrome/browser/ui/ash/login_screen_client.h
index 2fedbd4..2a186f60 100644
--- a/chrome/browser/ui/ash/login_screen_client.h
+++ b/chrome/browser/ui/ash/login_screen_client.h
@@ -74,6 +74,8 @@
void OnMaxIncorrectPasswordAttempted(const AccountId& account_id) override;
void FocusLockScreenApps(bool reverse) override;
void ShowGaiaSignin() override;
+ void OnRemoveUserWarningShown() override;
+ void RemoveUser(const AccountId& account_id) override;
private:
// Lock screen mojo service in ash.