Commit 53f95b21 authored by Takashi Sakamoto's avatar Takashi Sakamoto Committed by Commit Bot

Revert "ash: Keep active window uncovered on display change"

This reverts commit 92ecc62f.

Reason for revert: suspect ash_unittests failure on Linux Chromium OS ASan LSan Tests
https://ci.chromium.org/p/chromium/builders/ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/32865
ExtendedDesktopTest.KeyEventsOnLockScreen
OverviewSessionTest.RemoveDisplay
OverviewSessionTest.RemoveDisplayWithAnimation
PersistentWindowControllerTest.DisconnectDisplay
PersistentWindowControllerTest.MixedMirrorMode
PersistentWindowControllerTest.NormalMirrorMode
PersistentWindowControllerTest.ReconnectOnLockScreen
PersistentWindowControllerTest.RecordNumOfWindowsRestored
PersistentWindowControllerTest.ThreeDisplays
PersistentWindowControllerTest.WindowMovedByAccel
...

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8915703982173055536/+/steps/ash_unittests/0/logs/Deterministic_failure:_ExtendedDesktopTest.KeyEventsOnLockScreen__status_CRASH_/0
==31193==ERROR: AddressSanitizer: container-overflow on address 0x6020001da810 at pc 0x55d92be6ea70 bp 0x7ffe03af0a30 sp 0x7ffe03af0a28
READ of size 8 at 0x6020001da810 thread T0
    #0 0x55d92be6ea6f in ReparentAllWindows ./../../ash/root_window_controller.cc:256:41
    #1 0x55d92be6ea6f in ash::RootWindowController::MoveWindowsTo(aura::Window*) ./../../ash/root_window_controller.cc:622:0
    #2 0x55d92bc99cf5 in ash::WindowTreeHostManager::DeleteHost(ash::AshWindowTreeHost*) ./../../ash/display/window_tree_host_manager.cc:563:15

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}

TBR=xiyuan@chromium.org,oshima@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 945754
Change-Id: Ibf3b8cb823388837f6b3d3052e8258b78fd9a9b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1577223Reviewed-by: default avatarTakashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#652818}
parent af808596
......@@ -160,15 +160,12 @@ class FocusActivationStore {
if (active_ && focused_ != active_)
tracker_.Add(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);
// Deactivate the window to close menu / bubble windows.
if (clear_focus)
activation_client_->DeactivateWindow(active_);
// 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
......
......@@ -244,16 +244,15 @@ 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.rbegin();
while (iter != src_container_children.rend() &&
auto iter = src_container_children.begin();
while (iter != src_container_children.end() &&
SystemModalContainerLayoutManager::IsModalBackground(*iter)) {
++iter;
}
// If the entire window list is modal background windows then stop.
if (iter == src_container_children.rend())
if (iter == src_container_children.end())
break;
ReparentWindow(*iter, dst_container);
dst_container->StackChildAtBottom(*iter);
}
}
}
......
......@@ -348,50 +348,6 @@ 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();
......
......@@ -652,14 +652,8 @@ void OverviewSession::OnWindowActivating(
::wm::ActivationChangeObserver::ActivationReason reason,
aura::Window* gained_active,
aura::Window* lost_active) {
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();
if (ignore_activations_ || !gained_active ||
gained_active == GetOverviewFocusWindow()) {
return;
}
......
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