Skip to content

Commit

Permalink
Reland "ash: Keep active window uncovered on display change"
Browse files Browse the repository at this point in the history
This is a reland of 92ecc62

Original change's description:
> ash: Keep active window uncovered on display change
> 
> - FocusActivationStore uses ActivateWindow(nullptr) for deactivating
>   to avoid an arbitrary window covering the active window before
>   Restore().
> - RootWindowController puts the moved windows under existing ones
>   on display change.
> 
> Bug: 945754
> Change-Id: I3cacfd3427b8c98c9ce9dd2da05977120c1c1d65
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1574824
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#652678}

Bug: 945754
Change-Id: If8b796e8b1736e17e35f34fd2e671689e3a88ed4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1577561
Reviewed-by: Mitsuru Oshima (Slow 4/22-26) <oshima@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652931}
  • Loading branch information
Xiyuan Xia authored and Commit Bot committed Apr 22, 2019
1 parent b995030 commit de847a6
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 9 deletions.
9 changes: 6 additions & 3 deletions ash/display/window_tree_host_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,15 @@ class FocusActivationStore {
if (active_ && focused_ != active_)
tracker_.Add(active_);

// Deactivate the window to close menu / bubble windows.
if (clear_focus)
activation_client_->DeactivateWindow(active_);
// Deactivate the window to close menu / bubble windows. Deactivating by
// setting active window to nullptr to avoid side effects of activating an
// arbitrary window, such as covering |active_| before Restore().
if (clear_focus && active_)
activation_client_->ActivateWindow(nullptr);

// Release capture if any.
capture_client_->SetCapture(nullptr);

// Clear the focused window if any. This is necessary because a
// window may be deleted when losing focus (fullscreen flash for
// example). If the focused window is still alive after move, it'll
Expand Down
12 changes: 8 additions & 4 deletions ash/root_window_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,19 @@ void ReparentAllWindows(aura::Window* src, aura::Window* dst) {
// may change as a result of moving other windows.
const aura::Window::Windows& src_container_children =
src_container->children();
auto iter = src_container_children.begin();
while (iter != src_container_children.end() &&
auto iter = src_container_children.rbegin();
while (iter != src_container_children.rend() &&
SystemModalContainerLayoutManager::IsModalBackground(*iter)) {
++iter;
}
// If the entire window list is modal background windows then stop.
if (iter == src_container_children.end())
if (iter == src_container_children.rend())
break;
ReparentWindow(*iter, dst_container);

// |iter| is invalidated after ReparentWindow. Cache it to use afterwards.
aura::Window* const window = *iter;
ReparentWindow(window, dst_container);
dst_container->StackChildAtBottom(window);
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions ash/root_window_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,50 @@ TEST_F(RootWindowControllerTest, MoveWindows_LockWindowsInUnified) {
EXPECT_EQ("0,0 600x500", lock_screen->GetNativeWindow()->bounds().ToString());
}

// Tests that the moved windows (except the active one) are put under existing
// ones.
TEST_F(RootWindowControllerTest, MoveWindows_UnderExisting) {
UpdateDisplay("600x600,300x300");

display::Screen* screen = display::Screen::GetScreen();
const display::Display primary_display = screen->GetPrimaryDisplay();
const display::Display secondary_display = GetSecondaryDisplay();

views::Widget* existing = CreateTestWidget(gfx::Rect(0, 10, 100, 100));
ASSERT_EQ(primary_display.id(),
screen->GetDisplayNearestWindow(existing->GetNativeWindow()).id());

views::Widget* moved = CreateTestWidget(gfx::Rect(650, 10, 100, 100));
ASSERT_EQ(secondary_display.id(),
screen->GetDisplayNearestWindow(moved->GetNativeWindow()).id());

views::Widget* active = CreateTestWidget(gfx::Rect(650, 10, 100, 100));
ASSERT_TRUE(active->IsActive());
ASSERT_EQ(secondary_display.id(),
screen->GetDisplayNearestWindow(active->GetNativeWindow()).id());

// Switch to single display.
UpdateDisplay("600x500");

// |active| is still active.
ASSERT_TRUE(active->IsActive());

// |moved| should be put under |existing|.
ASSERT_EQ(moved->GetNativeWindow()->parent(),
existing->GetNativeWindow()->parent());
bool found_existing = false;
bool found_moved = false;
for (auto* child : moved->GetNativeWindow()->parent()->children()) {
if (child == existing->GetNativeWindow())
found_existing = true;
if (child == moved->GetNativeWindow()) {
found_moved = true;
break;
}
}
EXPECT_TRUE(found_moved && !found_existing);
}

TEST_F(RootWindowControllerTest, ModalContainer) {
UpdateDisplay("600x600");
RootWindowController* controller = Shell::GetPrimaryRootWindowController();
Expand Down
10 changes: 8 additions & 2 deletions ash/wm/overview/overview_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -650,8 +650,14 @@ void OverviewSession::OnWindowActivating(
::wm::ActivationChangeObserver::ActivationReason reason,
aura::Window* gained_active,
aura::Window* lost_active) {
if (ignore_activations_ || !gained_active ||
gained_active == GetOverviewFocusWindow()) {
if (ignore_activations_ || gained_active == GetOverviewFocusWindow())
return;

if (!gained_active) {
// Cancel overview session and do not restore focus when active window is
// set to nullptr. This happens when removing a display.
ResetFocusRestoreWindow(false);
CancelSelection();
return;
}

Expand Down

0 comments on commit de847a6

Please sign in to comment.