Skip to content

Commit

Permalink
Do not cancel overview if the to-be-snapped window is activated
Browse files Browse the repository at this point in the history
Background: For client-controlled windows, because all window state
changes need to be asynchronously ack’ed by its client (e.g. ARC), there
is always a very short transitional state between “ash sends a request
to the client” and “ash receives an ack from the client”. We
particularly call the transitional state for a snapped state as
“to-be-snapped”.

In overview mode, if a to-be-snapped window is activated just before the
new snapped state is applied, we don’t want to end the overview.

BUG=b:269689567
BUG=b:237743541
TEST=ClientControlledStateTestClamshellAndTablet.DragOverviewWindowToSnap

Change-Id: Id353bcf5fe247340081517329ee39591a27da19f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4263452
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Toshiki Kikuchi <toshikikikuchi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1107298}
  • Loading branch information
Toshiki Kikuchi authored and Chromium LUCI CQ committed Feb 20, 2023
1 parent 52f3817 commit 1c1ef20
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
48 changes: 48 additions & 0 deletions ash/wm/client_controlled_state_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include "ash/test/ash_test_base.h"
#include "ash/wm/desks/desks_util.h"
#include "ash/wm/float/float_controller.h"
#include "ash/wm/overview/overview_controller.h"
#include "ash/wm/overview/overview_item.h"
#include "ash/wm/overview/overview_test_util.h"
#include "ash/wm/pip/pip_positioner.h"
#include "ash/wm/screen_pinning_controller.h"
#include "ash/wm/splitview/split_view_controller.h"
Expand All @@ -32,6 +35,7 @@
#include "ui/aura/client/aura_constants.h"
#include "ui/events/test/event_generator.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/point_conversions.h"
#include "ui/gfx/geometry/vector2d.h"
#include "ui/views/widget/widget.h"
#include "ui/views/widget/widget_delegate.h"
Expand Down Expand Up @@ -965,4 +969,48 @@ TEST_P(ClientControlledStateTestClamshellAndTablet, FloatWindow) {
EXPECT_NE(kShellWindowId_FloatContainer, window()->parent()->GetId());
}

TEST_P(ClientControlledStateTestClamshellAndTablet, DragOverviewWindowToSnap) {
auto* const overview_controller = Shell::Get()->overview_controller();
auto* const split_view_controller = SplitViewController::Get(window());

widget_delegate()->EnableSnap();

// Create a fake normal window in addition to `window()` (client-controlled
// window) because we need at least two windows to keep overview mode active
// after snapping one of them.
auto fake_uninterested_window = CreateAppWindow();

// Enter overview.
ToggleOverview();
EXPECT_TRUE(overview_controller->InOverviewSession());
EXPECT_FALSE(split_view_controller->InSplitViewMode());

// Drag `window()`'s overview item to snap to left.
auto* const overview_item = GetOverviewItemForWindow(window());
auto* const event_generator = GetEventGenerator();
event_generator->set_current_screen_location(
gfx::ToRoundedPoint(overview_item->target_bounds().CenterPoint()));
event_generator->DragMouseTo(0, 0);

// Ensures the window is in a transitional snapped state.
EXPECT_TRUE(split_view_controller->IsWindowInTransitionalState(window()));
EXPECT_EQ(WindowStateType::kPrimarySnapped, delegate()->new_state());
EXPECT_FALSE(window_state()->IsSnapped());

// Activating window just before accepting the request shouldn't end the
// overview.
widget()->Activate();
EXPECT_TRUE(overview_controller->InOverviewSession());

// Accept the snap request.
state()->EnterNextState(window_state(), delegate()->new_state());
ApplyPendingRequestedBounds();
EXPECT_TRUE(window_state()->IsSnapped());
EXPECT_TRUE(split_view_controller->InSplitViewMode());
EXPECT_EQ(split_view_controller->state(),
SplitViewController::State::kPrimarySnapped);
EXPECT_EQ(split_view_controller->primary_window(), window());
EXPECT_TRUE(overview_controller->InOverviewSession());
}

} // namespace ash
6 changes: 5 additions & 1 deletion ash/wm/overview/overview_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -875,10 +875,14 @@ void OverviewSession::OnWindowActivating(
// handle the window activation change. Check for split view mode without
// using |SplitViewController::state_| which is updated asynchronously when
// snapping an ARC window.
// We also check if `gained_active` is to-be-snapped transitional state. In
// the case, the window has not been attached to SplitViewController yet but
// will be very soon.
SplitViewController* split_view_controller =
SplitViewController::Get(gained_active);
if (split_view_controller->primary_window() ||
split_view_controller->secondary_window()) {
split_view_controller->secondary_window() ||
split_view_controller->IsWindowInTransitionalState(gained_active)) {
RestoreWindowActivation(false);
return;
}
Expand Down

0 comments on commit 1c1ef20

Please sign in to comment.