Commit 23821832 authored by Meredith Lane's avatar Meredith Lane Committed by Commit Bot

Revert "tablet: Fix double shelf when entering guest mode."

This reverts commit b1af29a5.

Reason for revert: Crashes ash_unittests on linux-chromeos-dbg

First failure: https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/16451

Original change's description:
> 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: Xiaoqian Dai <xdai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#732159}

TBR=xdai@chromium.org,sammiequon@chromium.org

Change-Id: I738e9d46e2a9c7aa00305ba5ecf3c203ac872ecb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1035356
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2004177Reviewed-by: default avatarMeredith Lane <meredithl@chromium.org>
Commit-Queue: Meredith Lane <meredithl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732266}
parent 9a6a9283
...@@ -619,11 +619,6 @@ void TabletModeController::OnLayerAnimationAborted( ...@@ -619,11 +619,6 @@ void TabletModeController::OnLayerAnimationAborted(
void TabletModeController::OnLayerAnimationEnded( void TabletModeController::OnLayerAnimationEnded(
ui::LayerAnimationSequence* sequence) { 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)) if (!fps_counter_ || !IsTransformAnimationSequence(sequence))
return; return;
...@@ -681,28 +676,21 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) { ...@@ -681,28 +676,21 @@ void TabletModeController::SetTabletModeEnabledInternal(bool should_enable) {
state_ = State::kEnteringTabletMode; state_ = State::kEnteringTabletMode;
// Take a screenshot if there is a top window that will get animated. // 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 // TODO(sammiequon): Handle the case where the top window is not on the
// primary display. // primary display.
aura::Window* top_window = TabletModeWindowManager::GetTopWindow(); aura::Window* top_window = TabletModeWindowManager::GetTopWindow();
const bool top_window_on_primary_display = const bool top_window_on_primary_display =
top_window && top_window &&
top_window->GetRootWindow() == Shell::GetPrimaryRootWindow(); 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 = const bool overview_remain_active =
IsClamshellSplitViewModeEnabled() && IsClamshellSplitViewModeEnabled() &&
Shell::Get()->overview_controller()->InOverviewSession(); Shell::Get()->overview_controller()->InOverviewSession();
if (use_screenshot_for_test && top_window_on_primary_display && if (use_screenshot_for_test && top_window_on_primary_display &&
!top_window_animating && !overview_remain_active) { !overview_remain_active) {
TakeScreenshot(top_window); TakeScreenshot(top_window);
} else { } else {
FinishInitTabletMode(); FinishInitTabletMode();
......
...@@ -1685,30 +1685,12 @@ TEST_P(TabletModeControllerScreenshotTest, DISABLED_FromOverviewNoScreenshot) { ...@@ -1685,30 +1685,12 @@ TEST_P(TabletModeControllerScreenshotTest, DISABLED_FromOverviewNoScreenshot) {
EXPECT_FALSE(IsScreenshotShown()); 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 // Tests that the screenshot is visible when a window animation happens when
// entering tablet mode. // entering tablet mode.
TEST_P(TabletModeControllerScreenshotTest, ScreenshotVisibility) { TEST_P(TabletModeControllerScreenshotTest, ScreenshotVisibility) {
auto window = CreateTestWindow(gfx::Rect(200, 200)); auto window = CreateTestWindow(gfx::Rect(200, 200));
auto window2 = CreateTestWindow(gfx::Rect(300, 200)); auto window2 = CreateTestWindow(gfx::Rect(300, 200));
window->layer()->GetAnimator()->StopAnimating();
ui::Layer* layer = window2->layer(); ui::Layer* layer = window2->layer();
layer->GetAnimator()->StopAnimating();
ASSERT_FALSE(IsScreenshotShown()); ASSERT_FALSE(IsScreenshotShown());
TabletMode::Waiter waiter(/*enable=*/true); TabletMode::Waiter waiter(/*enable=*/true);
...@@ -1731,7 +1713,6 @@ TEST_P(TabletModeControllerScreenshotTest, ScreenshotVisibility) { ...@@ -1731,7 +1713,6 @@ TEST_P(TabletModeControllerScreenshotTest, ScreenshotVisibility) {
TEST_P(TabletModeControllerScreenshotTest, NoCrashWhenExitingWithoutWaiting) { TEST_P(TabletModeControllerScreenshotTest, NoCrashWhenExitingWithoutWaiting) {
// One non-maximized window is needed for screenshot to be taken. // One non-maximized window is needed for screenshot to be taken.
auto window = CreateTestWindow(gfx::Rect(200, 200)); auto window = CreateTestWindow(gfx::Rect(200, 200));
window->layer()->GetAnimator()->StopAnimating();
SetTabletMode(true); SetTabletMode(true);
SetTabletMode(false); 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