Commit 43973f65 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Chromium LUCI CQ

Revert "Bento: AltTabMode: Update highlighted window after switching mode."

This reverts commit c83fe85c.

Reason for revert: Made 	ModeSelectionWindowCycleControllerTest.CycleShowsWindowsPerMode flaky.
https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/22082

Original change's description:
> Bento: AltTabMode: Update highlighted window after switching mode.
>
> Regardless of where the highlight position is, after switching
> alt-tab mode, the highlight will be reset to the first or second
> window. This is consistent with highlighting when users first
> enter alt-tab mode:
> - In general case, highlight the second (next) most recently used
> window after the current one.
> - In the special case that the most recently used is minimized,
> highlight itself (the MRU window), so tabbing into it causes it
> to unminimize rather than opening up the next window in the list.
>
> - Track mode switching state in the controller to differentiate
> between normal tab pressing and mode switching.
> - Add Ash unit tests for both cases.
>
> `ash_unittests --gtest_filter=ModeSelectionWindowCycleControllerTest.SwitchingModeUpdates*WindowHighlight`
>
> Bug: 1157100
> Test: Manual test (a video uploaded to crbug) and ash unit tests
> Change-Id: Id85b389ba9d59eb1815b8250d640e5aa9128240b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2604907
> Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
> Reviewed-by: Min Chen <minch@chromium.org>
> Commit-Queue: Cattalyya Nuengsigkapian <cattalyya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#840348}

TBR=xdai@chromium.org,minch@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,cattalyya@chromium.org

Change-Id: I2551bf6393988d2ee1f790f2f39fdeae9cb0abba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1157100
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611090Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840560}
parent 99c5d591
...@@ -89,6 +89,8 @@ void WindowCycleController::HandleCycleWindow(Direction direction) { ...@@ -89,6 +89,8 @@ void WindowCycleController::HandleCycleWindow(Direction direction) {
if (!IsCycling()) if (!IsCycling())
StartCycling(); StartCycling();
// TODO(crbug.com/1157100): Handle window highlighting after switching
// alt-tab mode.
Step(direction); Step(direction);
} }
...@@ -173,23 +175,10 @@ WindowCycleController::WindowList WindowCycleController::CreateWindowList() { ...@@ -173,23 +175,10 @@ WindowCycleController::WindowList WindowCycleController::CreateWindowList() {
} }
void WindowCycleController::SetAltTabMode(DesksMruType alt_tab_mode) { void WindowCycleController::SetAltTabMode(DesksMruType alt_tab_mode) {
DCHECK(features::IsBentoEnabled());
if (alt_tab_mode_ == alt_tab_mode) if (alt_tab_mode_ == alt_tab_mode)
return; return;
alt_tab_mode_ = alt_tab_mode; alt_tab_mode_ = alt_tab_mode;
MaybeResetCycleList(); MaybeResetCycleList();
is_switching_mode_ = true;
// When user first press alt + tab, `HandleCycleForwardMRU` triggers
// `HandleCycleWindow(WindowCycleController::FORWARD)` since it considers
// the initial tab as forward cycling. Therefore, switching the mode
// should imitate the same forward cycling behavior after the cycle is reset.
HandleCycleWindow(WindowCycleController::FORWARD);
is_switching_mode_ = false;
}
bool WindowCycleController::IsSwitchingMode() {
return features::IsBentoEnabled() && is_switching_mode_;
} }
bool WindowCycleController::IsAltTabPerActiveDesk() { bool WindowCycleController::IsAltTabPerActiveDesk() {
......
...@@ -95,12 +95,6 @@ class ASH_EXPORT WindowCycleController { ...@@ -95,12 +95,6 @@ class ASH_EXPORT WindowCycleController {
// to all desk unless LimitAltTabToActiveDesk flag is explicitly enabled. // to all desk unless LimitAltTabToActiveDesk flag is explicitly enabled.
bool IsAltTabPerActiveDesk(); bool IsAltTabPerActiveDesk();
// Returns true while switching the alt-tab mode and Bento flag is enabled.
// This helps `Scroll()` and `Step()` distinguish between pressing tabs and
// switching mode, so they refresh |current_index_| and the highlighted
// window correctly.
bool IsSwitchingMode();
private: private:
// Gets a list of windows from the currently open windows, removing windows // Gets a list of windows from the currently open windows, removing windows
// with transient roots already in the list. The returned list of windows // with transient roots already in the list. The returned list of windows
...@@ -136,9 +130,6 @@ class ASH_EXPORT WindowCycleController { ...@@ -136,9 +130,6 @@ class ASH_EXPORT WindowCycleController {
// TODO(crbug.com/1157105): Store per-desk mode in primary user pref. // TODO(crbug.com/1157105): Store per-desk mode in primary user pref.
DesksMruType alt_tab_mode_ = DesksMruType::kAllDesks; DesksMruType alt_tab_mode_ = DesksMruType::kAllDesks;
// Tracks whether alt-tab mode is currently switching or not.
bool is_switching_mode_ = false;
DISALLOW_COPY_AND_ASSIGN(WindowCycleController); DISALLOW_COPY_AND_ASSIGN(WindowCycleController);
}; };
......
...@@ -1540,132 +1540,4 @@ TEST_F(ModeSelectionWindowCycleControllerTest, NoWindowInActiveDesk) { ...@@ -1540,132 +1540,4 @@ TEST_F(ModeSelectionWindowCycleControllerTest, NoWindowInActiveDesk) {
waiter.Wait(); waiter.Wait();
} }
// Tests that switching between modes correctly reset the alt-tab-highlighted
// window to the second most recently used window, i.e. the next window to tab
// into from the currently used window. Since the window cycle list is ordered
// by MRU, such window is therefore the second window in the MRU list.
TEST_F(ModeSelectionWindowCycleControllerTest,
SwitchingModeUpdatesWindowHighlight) {
WindowCycleController* cycle_controller =
Shell::Get()->window_cycle_controller();
// Create two windows for desk1 and three windows for desk2 in the reversed
// order of the most recently active window.
auto win4 = CreateAppWindow(gfx::Rect(0, 0, 250, 100));
auto win3 = CreateAppWindow(gfx::Rect(50, 50, 200, 200));
auto* desks_controller = DesksController::Get();
desks_controller->NewDesk(DesksCreationRemovalSource::kButton);
ASSERT_EQ(2u, desks_controller->desks().size());
const Desk* desk_2 = desks_controller->desks()[1].get();
ActivateDesk(desk_2);
EXPECT_EQ(desk_2, desks_controller->active_desk());
auto win2 = CreateAppWindow(gfx::Rect(0, 0, 300, 200));
auto win1 = CreateAppWindow(gfx::Rect(10, 30, 400, 200));
auto win0 = CreateAppWindow(gfx::Rect(10, 30, 400, 200));
// Enter the all-desk mode by default with the window order [0, 1, 2, 3 ,4].
cycle_controller->StartCycling();
cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
EXPECT_FALSE(cycle_controller->IsAltTabPerActiveDesk());
auto cycle_windows = GetWindows(cycle_controller);
// The window list is MRU ordered.
EXPECT_EQ(win0.get(), cycle_windows[0]);
EXPECT_EQ(win1.get(), cycle_windows[1]);
EXPECT_EQ(win2.get(), cycle_windows[2]);
EXPECT_EQ(win3.get(), cycle_windows[3]);
EXPECT_EQ(win4.get(), cycle_windows[4]);
// Alt-Tab should highlight the second most recently used window, which is
// the second window in the MRU list, win1.
EXPECT_EQ(win1.get(), GetTargetWindow());
// Step to win2 and win3, so we are now select a window in a non-active desk.
cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
EXPECT_EQ(win2.get(), GetTargetWindow());
cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
EXPECT_EQ(win3.get(), GetTargetWindow());
// Switching from the all-desks mode, which highlights a non-current-desk
// window to the current-desk mode [0, 1, 2] should resolve the highlight
// correctly to win1, the second window in the cycle list.
SwitchPerDeskAltTabMode(true);
EXPECT_EQ(win1.get(), GetTargetWindow());
EXPECT_EQ(win1.get(), cycle_windows[1]);
// Step to win2.
cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
EXPECT_EQ(win2.get(), GetTargetWindow());
// Switching back to the all-desk mode should reset highlight to win1 again.
SwitchPerDeskAltTabMode(false);
EXPECT_EQ(win1.get(), GetTargetWindow());
cycle_controller->CompleteCycling();
}
// Similar to `SwitchingModeUpdatesWindowHighlight`, tests that switching the
// alt-tab mode updates the highlighted window to the first window (most
// recently used) in the special case where all windows are minimized.
// When they are minimized, cycling forward should help unminimize the most
// recently used window rather than trying to open the second most recently
// used window.
TEST_F(ModeSelectionWindowCycleControllerTest,
SwitchingModeUpdatesMinimizedWindowHighlight) {
WindowCycleController* cycle_controller =
Shell::Get()->window_cycle_controller();
// Create two windows for desk1 and three windows for desk2 in the reversed
// order of the most recently active window.
auto win4 = CreateAppWindow(gfx::Rect(0, 0, 250, 100));
auto win3 = CreateAppWindow(gfx::Rect(50, 50, 200, 200));
auto* desks_controller = DesksController::Get();
desks_controller->NewDesk(DesksCreationRemovalSource::kButton);
ASSERT_EQ(2u, desks_controller->desks().size());
const Desk* desk_2 = desks_controller->desks()[1].get();
ActivateDesk(desk_2);
EXPECT_EQ(desk_2, desks_controller->active_desk());
auto win2 = CreateAppWindow(gfx::Rect(0, 0, 300, 200));
auto win1 = CreateAppWindow(gfx::Rect(10, 30, 400, 200));
auto win0 = CreateAppWindow(gfx::Rect(10, 30, 400, 200));
// Minimize all windows to test this special case.
WindowState::Get(win4.get())->Minimize();
WindowState::Get(win3.get())->Minimize();
WindowState::Get(win2.get())->Minimize();
WindowState::Get(win1.get())->Minimize();
WindowState::Get(win0.get())->Minimize();
EXPECT_FALSE(WindowState::Get(win0.get())->IsActive());
EXPECT_FALSE(WindowState::Get(win1.get())->IsActive());
EXPECT_FALSE(WindowState::Get(win2.get())->IsActive());
EXPECT_FALSE(WindowState::Get(win3.get())->IsActive());
EXPECT_FALSE(WindowState::Get(win4.get())->IsActive());
// Enter the all-desk mode by default with the window order [0, 1, 2, 3 ,4].
cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
EXPECT_FALSE(cycle_controller->IsAltTabPerActiveDesk());
auto cycle_windows = GetWindows(cycle_controller);
EXPECT_EQ(5u, GetWindowCycleItemViews().size());
// The window list is MRU ordered.
EXPECT_EQ(win0.get(), cycle_windows[0]);
EXPECT_EQ(win1.get(), cycle_windows[1]);
EXPECT_EQ(win2.get(), cycle_windows[2]);
EXPECT_EQ(win3.get(), cycle_windows[3]);
EXPECT_EQ(win4.get(), cycle_windows[4]);
// Step forward a few times and switch to all-desks mode. This should
// highlight win0, the first window in the current-desk cycle list.
cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
SwitchPerDeskAltTabMode(true);
EXPECT_EQ(3u, GetWindowCycleItemViews().size());
EXPECT_EQ(win0.get(), GetTargetWindow());
EXPECT_EQ(win0.get(), cycle_windows[0]);
// Stepping to win1 and switching back to the all-desk mode should reset
// a highlight to win0 again.
cycle_controller->HandleCycleWindow(WindowCycleController::FORWARD);
EXPECT_EQ(win1.get(), GetTargetWindow());
SwitchPerDeskAltTabMode(false);
EXPECT_EQ(5u, GetWindowCycleItemViews().size());
EXPECT_EQ(win0.get(), GetTargetWindow());
cycle_controller->CompleteCycling();
}
} // namespace ash } // namespace ash
...@@ -690,16 +690,7 @@ void WindowCycleList::Step(WindowCycleController::Direction direction) { ...@@ -690,16 +690,7 @@ void WindowCycleList::Step(WindowCycleController::Direction direction) {
} }
const int offset = direction == WindowCycleController::FORWARD ? 1 : -1; const int offset = direction == WindowCycleController::FORWARD ? 1 : -1;
if (offset == 1 && !wm::IsActiveWindow(windows_[0]) && SetFocusedWindow(windows_[GetOffsettedWindowIndex(offset)]);
Shell::Get()->window_cycle_controller()->IsSwitchingMode()) {
// Similar to `WindowCycleList::Scroll()`, when switching to alt-tab mode,
// if all windows are minimized, the starting window should be the first
// one rather than the second. Note that during entering alt-tab mode,
// `SetFocusedWindow()` does nothing, so we don't need to prevent it here.
SetFocusedWindow(windows_[0]);
} else {
SetFocusedWindow(windows_[GetOffsettedWindowIndex(offset)]);
}
Scroll(offset); Scroll(offset);
} }
...@@ -884,17 +875,16 @@ void WindowCycleList::Scroll(int offset) { ...@@ -884,17 +875,16 @@ void WindowCycleList::Scroll(int offset) {
DCHECK(static_cast<size_t>(current_index_) < windows_.size()); DCHECK(static_cast<size_t>(current_index_) < windows_.size());
// If alt-tab is entered or switched to the other mode, check the following if (!cycle_view_ && current_index_ == 0) {
// special case: user is cycling forward but the MRU window is not active. // Special case the situation where we're cycling forward but the MRU
// This occurs when all windows are minimized. The starting window should be // window is not active. This occurs when all windows are minimized. The
// the first one rather than the second. // starting window should be the first one rather than the second.
if ((!cycle_view_ || if (offset == 1 && !wm::IsActiveWindow(windows_[0]))
Shell::Get()->window_cycle_controller()->IsSwitchingMode()) && current_index_ = -1;
current_index_ == 0 && offset == 1 && !wm::IsActiveWindow(windows_[0])) {
current_index_ = -1;
} }
current_index_ = GetOffsettedWindowIndex(offset); current_index_ = GetOffsettedWindowIndex(offset);
if (ShouldShowUi()) { if (ShouldShowUi()) {
if (current_index_ > 1) if (current_index_ > 1)
InitWindowCycleView(); InitWindowCycleView();
......
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