Commit 2357c735 authored by Sammie Quon's avatar Sammie Quon Committed by Commit Bot

desks: Fix for continuous gestures crash.

The crash reports say that the starting desk screenshot is not taken
when ending a gesture. This CL adds handling for that case, along with
handling for case if the ending desk screenshot is not taken. If the
starting desk screenshot has not been taken yet, the user does not see
the screenshot. This means from the user's point of view, they still
only see the starting desk. In this case, we can just end the animation
without animating, and make sure the delegate will activate the
starting desk again.

Test: manual & added test
Bug: 1134390
Change-Id: I3ff2f5a04f46bea9fb4e6403a969b67afb972d65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506356
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822514}
parent a2676227
......@@ -191,7 +191,6 @@ void RootWindowDeskSwitchAnimator::TakeEndingDeskScreenshot() {
void RootWindowDeskSwitchAnimator::StartAnimation() {
DCHECK(starting_desk_screenshot_taken_);
DCHECK(ending_desk_screenshot_taken_);
DCHECK(!animation_finished_);
// Set a transform so that the ending desk will be visible.
......@@ -322,9 +321,22 @@ bool RootWindowDeskSwitchAnimator::UpdateSwipeAnimation(float scroll_delta_x) {
}
void RootWindowDeskSwitchAnimator::EndSwipeAnimation() {
// TODO(crbug.com/1134390): Convert back to DCHECK when the issue is fixed.
CHECK(starting_desk_screenshot_taken_);
CHECK(ending_desk_screenshot_taken_);
// If the starting screenshot has not finished, just let our delegate know
// that the desk animation is finished (and |this| will soon be deleted), and
// go back to the starting desk.
if (!starting_desk_screenshot_taken_) {
animation_finished_ = true;
ending_desk_index_ = starting_desk_index_;
delegate_->OnDeskSwitchAnimationFinished();
return;
}
// If the ending desk screenshot has not finished,
// GetIndexOfMostVisibleDeskScreenshot will still return a valid desk index
// that we can animate to, but we need to make sure the ending desk screenshot
// callback does not get called.
if (!ending_desk_screenshot_taken_)
weak_ptr_factory_.InvalidateWeakPtrs();
ending_desk_index_ = GetIndexOfMostVisibleDeskScreenshot();
StartAnimation();
......
......@@ -13,6 +13,7 @@
#include "ash/wm/desks/desks_histogram_enums.h"
#include "ash/wm/desks/root_window_desk_switch_animator_test_api.h"
#include "base/callback_forward.h"
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "ui/compositor/layer.h"
#include "ui/compositor/scoped_animation_duration_scale_mode.h"
......@@ -446,4 +447,21 @@ TEST_F(RootWindowDeskSwitchAnimatorTest, EndSwipeAnimation) {
animation_layer));
}
// Test that there is no crash if we end swiping before the desk animation
// screenshots are finished taking. Regression test for
// https://crbug.com/1134390.
TEST_F(RootWindowDeskSwitchAnimatorTest,
EndSwipeAnimationBeforeScreenshotTaken) {
InitAnimator(0, 1);
animator()->TakeStartingDeskScreenshot();
animator()->EndSwipeAnimation();
// Reinitialize the animator as each animator only supports one
// EndSwipeAnimation during its lifetime.
InitAnimator(0, 1);
TakeStartingDeskScreenshotAndWait();
animator()->TakeEndingDeskScreenshot();
animator()->EndSwipeAnimation();
}
} // namespace ash
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