Commit 3a7250fc authored by Tim Song's avatar Tim Song Committed by Commit Bot

Login Screen: Add nullptr checks for that there is a next focusable view.

If there are no focusable views in the LockContentsView a crash will occur when
toggling focus. Although this doesn't currently cause a crash, it was a problem
in the past and we need to prevent it from happening in case of UI changes in
the future.

TEST=unit test
BUG=914103

Change-Id: Iee5f23d98aa202998ccd91c743e1f0279c162c81
Reviewed-on: https://chromium-review.googlesource.com/c/1493446Reviewed-by: default avatarJacob Dufault <jdufault@chromium.org>
Commit-Queue: Tim Song <tengs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636541}
parent bddf57ca
...@@ -134,15 +134,15 @@ class AuthErrorLearnMoreButton : public views::Button, ...@@ -134,15 +134,15 @@ class AuthErrorLearnMoreButton : public views::Button,
DISALLOW_COPY_AND_ASSIGN(AuthErrorLearnMoreButton); DISALLOW_COPY_AND_ASSIGN(AuthErrorLearnMoreButton);
}; };
// Returns the first or last focusable child of |root|. If |reverse| is false, // Focuses the first or last focusable child of |root|. If |reverse| is false,
// this returns the first focusable child. If |reverse| is true, this returns // this focuses the first focusable child. If |reverse| is true, this focuses
// the last focusable child. // the last focusable child.
views::View* FindFirstOrLastFocusableChild(views::View* root, bool reverse) { void FocusFirstOrLastFocusableChild(views::View* root, bool reverse) {
views::FocusSearch search(root, reverse /*cycle*/, views::FocusSearch search(root, reverse /*cycle*/,
false /*accessibility_mode*/); false /*accessibility_mode*/);
views::FocusTraversable* dummy_focus_traversable; views::FocusTraversable* dummy_focus_traversable;
views::View* dummy_focus_traversable_view; views::View* dummy_focus_traversable_view;
return search.FindNextFocusableView( views::View* focusable_view = search.FindNextFocusableView(
root, root,
reverse ? views::FocusSearch::SearchDirection::kBackwards reverse ? views::FocusSearch::SearchDirection::kBackwards
: views::FocusSearch::SearchDirection::kForwards, : views::FocusSearch::SearchDirection::kForwards,
...@@ -150,6 +150,8 @@ views::View* FindFirstOrLastFocusableChild(views::View* root, bool reverse) { ...@@ -150,6 +150,8 @@ views::View* FindFirstOrLastFocusableChild(views::View* root, bool reverse) {
views::FocusSearch::StartingViewPolicy::kSkipStartingView, views::FocusSearch::StartingViewPolicy::kSkipStartingView,
views::FocusSearch::AnchoredDialogPolicy::kCanGoIntoAnchoredDialog, views::FocusSearch::AnchoredDialogPolicy::kCanGoIntoAnchoredDialog,
&dummy_focus_traversable, &dummy_focus_traversable_view); &dummy_focus_traversable, &dummy_focus_traversable_view);
if (focusable_view)
focusable_view->RequestFocus();
} }
// Make a section of the text bold. // Make a section of the text bold.
...@@ -1092,7 +1094,7 @@ void LockContentsView::OnFocusLeavingLockScreenApps(bool reverse) { ...@@ -1092,7 +1094,7 @@ void LockContentsView::OnFocusLeavingLockScreenApps(bool reverse) {
if (!reverse || lock_screen_apps_active_) if (!reverse || lock_screen_apps_active_)
FocusNextWidget(reverse); FocusNextWidget(reverse);
else else
FindFirstOrLastFocusableChild(this, reverse)->RequestFocus(); FocusFirstOrLastFocusableChild(this, reverse);
} }
void LockContentsView::OnOobeDialogStateChanged(mojom::OobeDialogState state) { void LockContentsView::OnOobeDialogStateChanged(mojom::OobeDialogState state) {
...@@ -1108,7 +1110,7 @@ void LockContentsView::OnFocusLeavingSystemTray(bool reverse) { ...@@ -1108,7 +1110,7 @@ void LockContentsView::OnFocusLeavingSystemTray(bool reverse) {
// from the system shelf (or tray) - lock shelf view expect the focus to be // from the system shelf (or tray) - lock shelf view expect the focus to be
// taken when it passes it to lock screen view, and can misbehave in case the // taken when it passes it to lock screen view, and can misbehave in case the
// focus is kept in it. // focus is kept in it.
FindFirstOrLastFocusableChild(this, reverse)->RequestFocus(); FocusFirstOrLastFocusableChild(this, reverse);
if (lock_screen_apps_active_) { if (lock_screen_apps_active_) {
Shell::Get()->login_screen_controller()->FocusLockScreenApps(reverse); Shell::Get()->login_screen_controller()->FocusLockScreenApps(reverse);
......
...@@ -2184,4 +2184,18 @@ TEST_F(LockContentsViewUnitTest, RightAndLeftAcceleratorsWithNoUser) { ...@@ -2184,4 +2184,18 @@ TEST_F(LockContentsViewUnitTest, RightAndLeftAcceleratorsWithNoUser) {
lock->AcceleratorPressed(ui::Accelerator(ui::VKEY_LEFT, 0)); lock->AcceleratorPressed(ui::Accelerator(ui::VKEY_LEFT, 0));
} }
TEST_F(LockContentsViewUnitTest, OnFocusLeavingSystemTrayWithNoUsers) {
auto* lock = new LockContentsView(
mojom::TrayActionState::kNotAvailable, LockScreen::ScreenType::kLock,
DataDispatcher(),
std::make_unique<FakeLoginDetachableBaseModel>(DataDispatcher()));
SetWidget(CreateWidgetWithContent(lock));
// Check that there is always a focusable view after transitioning focus.
lock->OnFocusLeavingSystemTray(false /* reverse */);
EXPECT_TRUE(lock->GetFocusManager()->GetFocusedView());
lock->OnFocusLeavingSystemTray(true /* reverse */);
EXPECT_TRUE(lock->GetFocusManager()->GetFocusedView());
}
} // namespace ash } // namespace ash
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