Commit b1af29a5 authored by Sammie Quon's avatar Sammie Quon Committed by Commit Bot

tablet: Fix double shelf when entering guest mode.

The issue was the window was created on startup, then OnGetSwitchStates
triggers TabletMode to start and create the screenshot since it detects
a window animating. But the window doesn't destroy the screenshot because
a |fps_counter_| is not detected on animation end because it is created
in on animation schedules which happened before entering tablet mode.

This CL aims to prevent this and similar bugs by not taking the screenshot
if a window is already animating, so the only observed animation is the
one triggered by entering tablet mode. This may have missed data or
performance in some cases, but will prevent similar bugs.

Test: ash_unittests TabletModeControllerScreenshotTest.EnterTabletModeWhileAnimating
Bug: 1035356
Change-Id: I601d08ef50f214dbd262e5eea6baed40e5596270
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1999241
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: default avatarXiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732159}
parent c8af0e6f
......@@ -619,6 +619,11 @@ void TabletModeController::OnLayerAnimationAborted(
void TabletModeController::OnLayerAnimationEnded(
ui::LayerAnimationSequence* sequence) {
// This may be called before |OnLayerAnimationScheduled()| if tablet is
// entered/exited while an animation is in progress, so we won't get
// stats/screenshot in those cases.
// TODO(sammiequon): We may want to remove the |fps_counter_| check and
// simplify things since those are edge cases.
if (!fps_counter_ || !IsTransformAnimationSequence(sequence))
return;
......@@ -676,21 +681,28 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) {
state_ = State::kEnteringTabletMode;
// Take a screenshot if there is a top window that will get animated.
// Since with ash::features::kDragToSnapInClamshellMode enabled, we'll keep
// overview active after clamshell <-> tablet mode transition if it was
// active before transition, do not take screenshot if overview is active
// in this case.
// TODO(sammiequon): Handle the case where the top window is not on the
// primary display.
aura::Window* top_window = TabletModeWindowManager::GetTopWindow();
const bool top_window_on_primary_display =
top_window &&
top_window->GetRootWindow() == Shell::GetPrimaryRootWindow();
// If the top window was already animating (eg. tablet mode event received
// while create window animation still running), skip taking the screenshot.
// It will take a performance hit but will remove cases where the screenshot
// might not get deleted because of the extra animation observer methods
// getting fired.
const bool top_window_animating =
top_window && top_window->layer()->GetAnimator()->is_animating();
// Since with ash::features::kDragToSnapInClamshellMode enabled, we'll keep
// overview active after clamshell <-> tablet mode transition if it was
// active before transition, do not take screenshot if overview is active
// in this case.
const bool overview_remain_active =
IsClamshellSplitViewModeEnabled() &&
Shell::Get()->overview_controller()->InOverviewSession();
if (use_screenshot_for_test && top_window_on_primary_display &&
!overview_remain_active) {
!top_window_animating && !overview_remain_active) {
TakeScreenshot(top_window);
} else {
FinishInitTabletMode();
......
......@@ -1685,12 +1685,30 @@ TEST_P(TabletModeControllerScreenshotTest, DISABLED_FromOverviewNoScreenshot) {
EXPECT_FALSE(IsScreenshotShown());
}
// Regression test for screenshot staying visible when entering tablet mode when
// a window creation animation is still underway. See https://crbug.com/1035356.
TEST_P(TabletModeControllerScreenshotTest, EnterTabletModeWhileAnimating) {
auto window = CreateTestWindow(gfx::Rect(200, 200));
ASSERT_TRUE(window->layer()->GetAnimator()->is_animating());
// Enter tablet mode.
TabletMode::Waiter waiter(/*enable=*/true);
SetTabletMode(true);
EXPECT_FALSE(IsScreenshotShown());
waiter.Wait();
EXPECT_FALSE(IsScreenshotShown());
}
// Tests that the screenshot is visible when a window animation happens when
// entering tablet mode.
TEST_P(TabletModeControllerScreenshotTest, ScreenshotVisibility) {
auto window = CreateTestWindow(gfx::Rect(200, 200));
auto window2 = CreateTestWindow(gfx::Rect(300, 200));
window->layer()->GetAnimator()->StopAnimating();
ui::Layer* layer = window2->layer();
layer->GetAnimator()->StopAnimating();
ASSERT_FALSE(IsScreenshotShown());
TabletMode::Waiter waiter(/*enable=*/true);
......@@ -1713,6 +1731,7 @@ TEST_P(TabletModeControllerScreenshotTest, ScreenshotVisibility) {
TEST_P(TabletModeControllerScreenshotTest, NoCrashWhenExitingWithoutWaiting) {
// One non-maximized window is needed for screenshot to be taken.
auto window = CreateTestWindow(gfx::Rect(200, 200));
window->layer()->GetAnimator()->StopAnimating();
SetTabletMode(true);
SetTabletMode(false);
......
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