Commit 8aff076a authored by Darren Shen's avatar Darren Shen Committed by Commit Bot

[VK] Fix work area bug in shelf layout manager.

In a previous CL [1], we made changes to the shelf layout manager to
use occluded and displaced bounds. However, we introduced a bug where
the work area would not reset when we hide the accessibility keyboard,
leaving a large gap in the screen.

The code now makes more sense: we should be changing the work area
whenever the displaced bounds change.

We add a test for this. We also removed some calls to |LayoutShelf| in
existing tests; |LayoutShelf| doesn't get called in real situations.
Calling |LayoutShelf| manually will always change the work area,
which is not what happens in practice.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1146530

TBR=jamescook@chromium.org

Bug: 870503
Change-Id: I80a6aa90f1bce8e59e6c7cde2e3f181c31fc6907
Reviewed-on: https://chromium-review.googlesource.com/1160827Reviewed-by: default avatarDarren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580394}
parent 0fcb16c4
......@@ -496,11 +496,12 @@ void ShelfLayoutManager::OnWindowActivated(ActivationReason reason,
void ShelfLayoutManager::OnKeyboardAppearanceChanged(
const keyboard::KeyboardStateDescriptor& state) {
// If displaced bounds changed, then change the work area too.
bool change_work_area = state.displaced_bounds != keyboard_displaced_bounds_;
keyboard_occluded_bounds_ = state.occluded_bounds;
keyboard_displaced_bounds_ = state.displaced_bounds;
// If there are displaced bounds, then change the work area.
bool change_work_area = !keyboard_displaced_bounds_.IsEmpty();
LayoutShelfAndUpdateBounds(change_work_area);
}
......
......@@ -2181,7 +2181,6 @@ TEST_F(ShelfLayoutManagerKeyboardTest, ShelfNotMoveOnKeyboardOpen) {
// Open keyboard in non-sticky mode.
kb_controller->ShowKeyboard(false);
NotifyKeyboardChanging(layout_manager, false, keyboard_bounds());
layout_manager->LayoutShelf();
// Shelf position should not be changed.
EXPECT_EQ(orig_bounds, GetShelfWidget()->GetWindowBoundsInScreen());
......@@ -2200,7 +2199,6 @@ TEST_F(ShelfLayoutManagerKeyboardTest,
// Open keyboard in non-sticky mode.
kb_controller->ShowKeyboard(false);
NotifyKeyboardChanging(layout_manager, false, keyboard_bounds());
layout_manager->LayoutShelf();
// Work area should not be changed.
EXPECT_EQ(orig_work_area,
......@@ -2208,7 +2206,6 @@ TEST_F(ShelfLayoutManagerKeyboardTest,
kb_controller->HideKeyboardExplicitlyBySystem();
NotifyKeyboardChanging(layout_manager, false, gfx::Rect());
layout_manager->LayoutShelf();
EXPECT_EQ(orig_work_area,
display::Screen::GetScreen()->GetPrimaryDisplay().work_area());
}
......@@ -2224,11 +2221,18 @@ TEST_F(ShelfLayoutManagerKeyboardTest, ShelfShouldChangeWorkAreaInStickyMode) {
// Open keyboard in sticky mode.
kb_controller->ShowKeyboard(true);
NotifyKeyboardChanging(layout_manager, true, keyboard_bounds());
layout_manager->LayoutShelf();
// Work area should be changed.
EXPECT_NE(orig_work_area,
display::Screen::GetScreen()->GetPrimaryDisplay().work_area());
// Hide the keyboard.
kb_controller->HideKeyboardByUser();
NotifyKeyboardChanging(layout_manager, true, gfx::Rect());
// Work area should be reset to its original value.
EXPECT_EQ(orig_work_area,
display::Screen::GetScreen()->GetPrimaryDisplay().work_area());
}
} // 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