Skip to content

Commit

Permalink
splitview: Support asynchronous window state change in auto snapping
Browse files Browse the repository at this point in the history
This CL improves table split view’s auto snapping to support for clients
(e.g. ARC app) which handles window state changes asynchronously.

In tablet split view mode, when a snappable window is activated, the
window is auto snapped in OnWindowActivated event.
This is fine for Chrome windows because its wm is synchronized, but this
behavior actually does not take some clients (e.g. ARC app), which
handles window state changes asynchronously, into account.

For example, when trying to snap a minimzied ARC app from shelf, a
visible but minimized window state triggers an implicit un-minimizing by
someone like WorkspaceLayoutManager.
So when un-minimizing a snappable window, WorkspaceLayoutManager emits a
window state restore event before SplitViewController triggers auto
snapping on "activated".
This causes unexpected window state change because the first restore
event causes actually restoring the window state before snapped.

Therefore, this CL allow SplitViewController to send a snap request
before WorkspaceLayoutManager’s handling and filter out events emitted
while the window is in a transitional snapped state.

design: go/async-tablet-splitview

BUG=b:154752540
TEST=SplitViewControllerTest.AutoSnapFromMinimizedState
TEST=ClientControlledStateTest.IgnoreWmEventWhenWindowIsInTransitionalSnappedState

Change-Id: I3109d5a7ff8744e6fbc7aabe5274dd9fcbfd1b32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2287193
Commit-Queue: Toshiki Kikuchi <toshikikikuchi@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788999}
  • Loading branch information
Toshiki Kikuchi authored and Commit Bot committed Jul 16, 2020
1 parent 08f9a99 commit 58412b2
Show file tree
Hide file tree
Showing 5 changed files with 298 additions and 94 deletions.
17 changes: 12 additions & 5 deletions ash/wm/client_controlled_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ash/shell.h"
#include "ash/wm/pip/pip_positioner.h"
#include "ash/wm/screen_pinning_controller.h"
#include "ash/wm/splitview/split_view_controller.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "ash/wm/window_positioning_utils.h"
#include "ash/wm/window_state.h"
Expand Down Expand Up @@ -82,11 +83,18 @@ void ClientControlledState::HandleTransitionEvents(WindowState* window_state,
return;
}

auto* window = window_state->window();
switch (event->type()) {
case WM_EVENT_NORMAL:
case WM_EVENT_MAXIMIZE:
case WM_EVENT_MINIMIZE:
case WM_EVENT_FULLSCREEN: {
// Clients handle a window state change asynchronously. So in the case
// that the window is in a transitional state (already snapped but not
// applied to its window state yet), we here skip to pass WM_EVENT.
if (SplitViewController::Get(window)->IsWindowInTransitionalState(window))
return;

// Reset window state
window_state->UpdateWindowPropertiesFromStateType();
WindowStateType next_state =
Expand All @@ -102,15 +110,14 @@ void ClientControlledState::HandleTransitionEvents(WindowState* window_state,
if (window_state->CanSnap()) {
// Get the desired window bounds for the snap state.
gfx::Rect bounds = GetSnappedWindowBoundsInParent(
window_state->window(), event->type() == WM_EVENT_SNAP_LEFT
? WindowStateType::kLeftSnapped
: WindowStateType::kRightSnapped);
window, event->type() == WM_EVENT_SNAP_LEFT
? WindowStateType::kLeftSnapped
: WindowStateType::kRightSnapped);
window_state->set_bounds_changed_by_user(true);

// We don't want Unminimize() to restore the pre-snapped state during
// the transition.
window_state->window()->ClearProperty(
aura::client::kPreMinimizedShowStateKey);
window->ClearProperty(aura::client::kPreMinimizedShowStateKey);

window_state->UpdateWindowPropertiesFromStateType();
WindowStateType next_state =
Expand Down
32 changes: 32 additions & 0 deletions ash/wm/client_controlled_state_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "ash/wm/desks/desks_util.h"
#include "ash/wm/pip/pip_positioner.h"
#include "ash/wm/screen_pinning_controller.h"
#include "ash/wm/splitview/split_view_controller.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "ash/wm/window_state.h"
#include "ash/wm/window_util.h"
Expand Down Expand Up @@ -588,4 +589,35 @@ TEST_F(ClientControlledStateTest,
EXPECT_EQ(WindowStateType::kMaximized, delegate()->new_state());
}

TEST_F(ClientControlledStateTest,
IgnoreWmEventWhenWindowIsInTransitionalSnappedState) {
auto* split_view_controller =
SplitViewController::Get(window_state()->window());

widget_delegate()->EnableSnap();
split_view_controller->SnapWindow(window_state()->window(),
SplitViewController::SnapPosition::RIGHT);

EXPECT_EQ(WindowStateType::kRightSnapped, delegate()->new_state());
EXPECT_FALSE(window_state()->IsSnapped());

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

// Ignores WMEvent if in a transitional state.
widget()->Maximize();
EXPECT_NE(WindowStateType::kMaximized, delegate()->new_state());

// Applies snap request.
state()->EnterNextState(window_state(), delegate()->new_state());
EXPECT_TRUE(window_state()->IsSnapped());

// After exiting the transitional state, works normally.
widget()->Maximize();
EXPECT_EQ(WindowStateType::kMaximized, delegate()->new_state());
}

} // namespace ash

0 comments on commit 58412b2

Please sign in to comment.