Commit 47ceef80 authored by Ganggui Tang's avatar Ganggui Tang Committed by Commit Bot

Revert "desks: Speculative fix for crash."

This reverts commit 536b412d.

Reason for revert: the test is failing on Linux Chromium OS ASan LSan Tests, crbug.com/1149939

Original change's description:
> desks: Speculative fix for crash.
>
> The previous attempt [1] added checks for whether the starting or ending
> screenshot was taken. In the case of the starting screenshot missing, it
> would signal the animation to remove some RootWindowDeskSwitchAnimators,
> leading to a vector out of bounds.
>
> [1] https://chromium-review.googlesource.com/c/chromium/src/+/2506356
>
> Bug: 1148607
> Test: added regression test
> Change-Id: I7f14df877cb93619a0ca8a85f5d0ad434dda3ad2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2536934
> Commit-Queue: Sammie Quon <sammiequon@chromium.org>
> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#828034}

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

Change-Id: I27cc6811a5edd38a56eb82f20752638351e2b2df
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1148607
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2544443Reviewed-by: default avatarGanggui Tang <gogerald@chromium.org>
Commit-Queue: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828204}
parent e64660e4
...@@ -2256,7 +2256,6 @@ test("ash_unittests") { ...@@ -2256,7 +2256,6 @@ test("ash_unittests") {
"wm/container_finder_unittest.cc", "wm/container_finder_unittest.cc",
"wm/default_window_resizer_unittest.cc", "wm/default_window_resizer_unittest.cc",
"wm/desks/autotest_desks_api_unittests.cc", "wm/desks/autotest_desks_api_unittests.cc",
"wm/desks/desk_animation_impl_unittest.cc",
"wm/desks/desks_unittests.cc", "wm/desks/desks_unittests.cc",
"wm/desks/root_window_desk_switch_animator_unittest.cc", "wm/desks/root_window_desk_switch_animator_unittest.cc",
"wm/drag_window_resizer_unittest.cc", "wm/drag_window_resizer_unittest.cc",
......
...@@ -128,9 +128,6 @@ void DeskAnimationBase::OnDeskSwitchAnimationFinished() { ...@@ -128,9 +128,6 @@ void DeskAnimationBase::OnDeskSwitchAnimationFinished() {
for (auto& observer : controller_->observers_) for (auto& observer : controller_->observers_)
observer.OnDeskSwitchAnimationFinished(); observer.OnDeskSwitchAnimationFinished();
if (skip_notify_controller_on_animation_finished_for_testing_)
return;
controller_->OnAnimationFinished(this); controller_->OnAnimationFinished(this);
// `this` is now deleted. // `this` is now deleted.
} }
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#ifndef ASH_WM_DESKS_DESK_ANIMATION_BASE_H_ #ifndef ASH_WM_DESKS_DESK_ANIMATION_BASE_H_
#define ASH_WM_DESKS_DESK_ANIMATION_BASE_H_ #define ASH_WM_DESKS_DESK_ANIMATION_BASE_H_
#include "ash/ash_export.h"
#include "ash/public/cpp/metrics_util.h" #include "ash/public/cpp/metrics_util.h"
#include "ash/wm/desks/desks_histogram_enums.h" #include "ash/wm/desks/desks_histogram_enums.h"
#include "ash/wm/desks/root_window_desk_switch_animator.h" #include "ash/wm/desks/root_window_desk_switch_animator.h"
...@@ -20,8 +19,7 @@ class DesksController; ...@@ -20,8 +19,7 @@ class DesksController;
// such as DeskActivationAnimation and DeskRemovalAnimation implement the // such as DeskActivationAnimation and DeskRemovalAnimation implement the
// abstract interface of this class to handle the unique operations specific to // abstract interface of this class to handle the unique operations specific to
// each animation type. // each animation type.
class ASH_EXPORT DeskAnimationBase class DeskAnimationBase : public RootWindowDeskSwitchAnimator::Delegate {
: public RootWindowDeskSwitchAnimator::Delegate {
public: public:
DeskAnimationBase(DesksController* controller, DeskAnimationBase(DesksController* controller,
int ending_desk_index, int ending_desk_index,
...@@ -58,10 +56,6 @@ class ASH_EXPORT DeskAnimationBase ...@@ -58,10 +56,6 @@ class ASH_EXPORT DeskAnimationBase
void OnDeskSwitchAnimationFinished() override; void OnDeskSwitchAnimationFinished() override;
void OnVisibleDeskChanged() override; void OnVisibleDeskChanged() override;
void set_skip_notify_controller_on_animation_finished_for_testing(bool val) {
skip_notify_controller_on_animation_finished_for_testing_ = val;
}
RootWindowDeskSwitchAnimator* GetFirstDeskSwitchAnimatorForTesting() const; RootWindowDeskSwitchAnimator* GetFirstDeskSwitchAnimatorForTesting() const;
protected: protected:
...@@ -103,12 +97,6 @@ class ASH_EXPORT DeskAnimationBase ...@@ -103,12 +97,6 @@ class ASH_EXPORT DeskAnimationBase
// ThroughputTracker used for measuring this animation smoothness. // ThroughputTracker used for measuring this animation smoothness.
ui::ThroughputTracker throughput_tracker_; ui::ThroughputTracker throughput_tracker_;
// If true, do not notify |controller_| when
// OnDeskSwitchAnimationFinished() is called. This class and
// DeskController are tied together in production code, but may not be in a
// test scenario.
bool skip_notify_controller_on_animation_finished_for_testing_ = false;
}; };
} // namespace ash } // namespace ash
......
...@@ -182,8 +182,9 @@ bool DeskActivationAnimation::EndSwipeAnimation() { ...@@ -182,8 +182,9 @@ bool DeskActivationAnimation::EndSwipeAnimation() {
// and update their ending desk index. When the animation is finished we will // and update their ending desk index. When the animation is finished we will
// activate that desk. // activate that desk.
for (const auto& animator : desk_switch_animators_) for (const auto& animator : desk_switch_animators_)
ending_desk_index_ = animator->EndSwipeAnimation(); animator->EndSwipeAnimation();
ending_desk_index_ = desk_switch_animators_[0]->ending_desk_index();
return true; return true;
} }
......
...@@ -5,7 +5,6 @@ ...@@ -5,7 +5,6 @@
#ifndef ASH_WM_DESKS_DESK_ANIMATION_IMPL_H_ #ifndef ASH_WM_DESKS_DESK_ANIMATION_IMPL_H_
#define ASH_WM_DESKS_DESK_ANIMATION_IMPL_H_ #define ASH_WM_DESKS_DESK_ANIMATION_IMPL_H_
#include "ash/ash_export.h"
#include "ash/public/cpp/metrics_util.h" #include "ash/public/cpp/metrics_util.h"
#include "ash/wm/desks/desk_animation_base.h" #include "ash/wm/desks/desk_animation_base.h"
#include "ash/wm/desks/desks_histogram_enums.h" #include "ash/wm/desks/desks_histogram_enums.h"
...@@ -15,7 +14,7 @@ namespace ash { ...@@ -15,7 +14,7 @@ namespace ash {
class DesksController; class DesksController;
class PresentationTimeRecorder; class PresentationTimeRecorder;
class ASH_EXPORT DeskActivationAnimation : public DeskAnimationBase { class DeskActivationAnimation : public DeskAnimationBase {
public: public:
DeskActivationAnimation(DesksController* controller, DeskActivationAnimation(DesksController* controller,
int starting_desk_index, int starting_desk_index,
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "ash/wm/desks/desk_animation_impl.h"
#include "ash/public/cpp/ash_features.h"
#include "ash/test/ash_test_base.h"
#include "ash/wm/desks/desks_controller.h"
#include "ash/wm/desks/desks_histogram_enums.h"
#include "base/test/scoped_feature_list.h"
namespace ash {
using DeskActivationAnimationTest = AshTestBase;
// Tests that there is no crash when ending a swipe animation before the
// starting screenshot has been taken. Regression test for
// https://crbug.com/1148607.
TEST_F(DeskActivationAnimationTest, EndSwipeBeforeStartingScreenshot) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kEnhancedDeskAnimations);
auto* desks_controller = DesksController::Get();
desks_controller->NewDesk(DesksCreationRemovalSource::kButton);
DeskActivationAnimation animation(desks_controller, 0, 1,
DesksSwitchSource::kDeskSwitchTouchpad,
/*update_window_activation=*/false);
animation.set_skip_notify_controller_on_animation_finished_for_testing(true);
animation.Launch();
animation.UpdateSwipeAnimation(10);
animation.EndSwipeAnimation();
}
} // namespace ash
...@@ -75,8 +75,6 @@ void TakeScreenshot( ...@@ -75,8 +75,6 @@ void TakeScreenshot(
viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE, viz::CopyOutputRequest::ResultFormat::RGBA_TEXTURE,
std::move(on_screenshot_taken)); std::move(on_screenshot_taken));
screenshot_request->set_area(request_bounds); screenshot_request->set_area(request_bounds);
screenshot_request->set_result_task_runner(
base::SequencedTaskRunnerHandle::Get());
screenshot_layer->RequestCopyOfOutput(std::move(screenshot_request)); screenshot_layer->RequestCopyOfOutput(std::move(screenshot_request));
} }
...@@ -327,7 +325,7 @@ bool RootWindowDeskSwitchAnimator::UpdateSwipeAnimation(float scroll_delta_x) { ...@@ -327,7 +325,7 @@ bool RootWindowDeskSwitchAnimator::UpdateSwipeAnimation(float scroll_delta_x) {
return true; return true;
} }
int RootWindowDeskSwitchAnimator::EndSwipeAnimation() { void RootWindowDeskSwitchAnimator::EndSwipeAnimation() {
// If the starting screenshot has not finished, just let our delegate know // 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 // that the desk animation is finished (and |this| will soon be deleted), and
// go back to the starting desk. // go back to the starting desk.
...@@ -335,7 +333,7 @@ int RootWindowDeskSwitchAnimator::EndSwipeAnimation() { ...@@ -335,7 +333,7 @@ int RootWindowDeskSwitchAnimator::EndSwipeAnimation() {
animation_finished_ = true; animation_finished_ = true;
ending_desk_index_ = starting_desk_index_; ending_desk_index_ = starting_desk_index_;
delegate_->OnDeskSwitchAnimationFinished(); delegate_->OnDeskSwitchAnimationFinished();
return ending_desk_index_; return;
} }
// If the ending desk screenshot has not finished, |visible_desk_index_| will // If the ending desk screenshot has not finished, |visible_desk_index_| will
...@@ -346,7 +344,6 @@ int RootWindowDeskSwitchAnimator::EndSwipeAnimation() { ...@@ -346,7 +344,6 @@ int RootWindowDeskSwitchAnimator::EndSwipeAnimation() {
ending_desk_index_ = visible_desk_index_; ending_desk_index_ = visible_desk_index_;
StartAnimation(); StartAnimation();
return ending_desk_index_;
} }
void RootWindowDeskSwitchAnimator::OnImplicitAnimationsCompleted() { void RootWindowDeskSwitchAnimator::OnImplicitAnimationsCompleted() {
......
...@@ -266,8 +266,8 @@ class ASH_EXPORT RootWindowDeskSwitchAnimator ...@@ -266,8 +266,8 @@ class ASH_EXPORT RootWindowDeskSwitchAnimator
bool UpdateSwipeAnimation(float scroll_delta_x); bool UpdateSwipeAnimation(float scroll_delta_x);
// Called when a user ends a touchpad swipe. This will animate to the most // Called when a user ends a touchpad swipe. This will animate to the most
// visible desk, whose index is also returned. // visible desk.
int EndSwipeAnimation(); void EndSwipeAnimation();
// ui::ImplicitAnimationObserver: // ui::ImplicitAnimationObserver:
void OnImplicitAnimationsCompleted() override; void OnImplicitAnimationsCompleted() override;
......
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