Commit 8fd68fd6 authored by Xiyuan Xia's avatar Xiyuan Xia Committed by Commit Bot

Revert "Replace AnimationMetricsReporter in ShelfView"

This reverts commit 197fd0a5.

Reason for revert:
Crash in OnFadeOutAnimationEnded when it runs with no | fade_out_animation_tracker_| instance.

Original change's description:
> Replace AnimationMetricsReporter in ShelfView
>
> ui::AnimationMetricsReporter is deprecating. Replace it with
> ui::AnimationThroughputReporter for simple layer animations and
> ui::ThroughputTracker for complicated ones.
>
> Bug: 11434673
> Change-Id: I96425e7626929f9d6381828c81dbe08dd58168e9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508793
> Reviewed-by: Andrew Xu <andrewxu@chromium.org>
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#822425}

TBR=xiyuan@chromium.org,andrewxu@chromium.org

Change-Id: Ib675aac6a9a57a7603529827069fd316ae7166be
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 11434673
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508958Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822499}
parent 561e62bd
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "ash/metrics/user_metrics_recorder.h" #include "ash/metrics/user_metrics_recorder.h"
#include "ash/public/cpp/ash_constants.h" #include "ash/public/cpp/ash_constants.h"
#include "ash/public/cpp/ash_features.h" #include "ash/public/cpp/ash_features.h"
#include "ash/public/cpp/metrics_util.h"
#include "ash/public/cpp/shelf_model.h" #include "ash/public/cpp/shelf_model.h"
#include "ash/public/cpp/shelf_types.h" #include "ash/public/cpp/shelf_types.h"
#include "ash/public/cpp/window_properties.h" #include "ash/public/cpp/window_properties.h"
...@@ -63,7 +62,6 @@ ...@@ -63,7 +62,6 @@
#include "ui/base/models/simple_menu_model.h" #include "ui/base/models/simple_menu_model.h"
#include "ui/base/ui_base_features.h" #include "ui/base/ui_base_features.h"
#include "ui/compositor/animation_metrics_reporter.h" #include "ui/compositor/animation_metrics_reporter.h"
#include "ui/compositor/animation_throughput_reporter.h"
#include "ui/compositor/layer.h" #include "ui/compositor/layer.h"
#include "ui/compositor/layer_animation_observer.h" #include "ui/compositor/layer_animation_observer.h"
#include "ui/compositor/layer_animator.h" #include "ui/compositor/layer_animator.h"
...@@ -115,6 +113,12 @@ constexpr char kShelfIconFadeInAnimationHistogram[] = ...@@ -115,6 +113,12 @@ constexpr char kShelfIconFadeInAnimationHistogram[] =
constexpr char kShelfIconFadeOutAnimationHistogram[] = constexpr char kShelfIconFadeOutAnimationHistogram[] =
"Ash.ShelfIcon.AnimationSmoothness.FadeOut"; "Ash.ShelfIcon.AnimationSmoothness.FadeOut";
enum class IconAnimationType {
kMoveAnimation,
kFadeInAnimation,
kFadeOutAnimation
};
// Helper to check if tablet mode is enabled. // Helper to check if tablet mode is enabled.
bool IsTabletModeEnabled() { bool IsTabletModeEnabled() {
return Shell::Get()->tablet_mode_controller() && return Shell::Get()->tablet_mode_controller() &&
...@@ -193,20 +197,34 @@ class ShelfFocusSearch : public views::FocusSearch { ...@@ -193,20 +197,34 @@ class ShelfFocusSearch : public views::FocusSearch {
DISALLOW_COPY_AND_ASSIGN(ShelfFocusSearch); DISALLOW_COPY_AND_ASSIGN(ShelfFocusSearch);
}; };
void ReportMoveAnimationSmoothness(int smoothness) { class IconAnimationMetricsReporter : public ui::AnimationMetricsReporter {
base::UmaHistogramPercentageObsoleteDoNotUse(kShelfIconMoveAnimationHistogram, public:
smoothness); explicit IconAnimationMetricsReporter(IconAnimationType type) : type_(type) {}
} IconAnimationMetricsReporter(const IconAnimationMetricsReporter&) = delete;
IconAnimationMetricsReporter& operator=(const IconAnimationMetricsReporter&) =
delete;
~IconAnimationMetricsReporter() override = default;
void ReportFadeInAnimationSmoothness(int smoothness) { private:
base::UmaHistogramPercentageObsoleteDoNotUse( void Report(int value) override {
kShelfIconFadeInAnimationHistogram, smoothness); switch (type_) {
} case IconAnimationType::kMoveAnimation:
base::UmaHistogramPercentageObsoleteDoNotUse(
kShelfIconMoveAnimationHistogram, value);
break;
case IconAnimationType::kFadeInAnimation:
base::UmaHistogramPercentageObsoleteDoNotUse(
kShelfIconFadeInAnimationHistogram, value);
break;
case IconAnimationType::kFadeOutAnimation:
base::UmaHistogramPercentageObsoleteDoNotUse(
kShelfIconFadeOutAnimationHistogram, value);
break;
}
}
void ReportFadeOutAnimationSmoothness(int smoothness) { IconAnimationType type_ = IconAnimationType::kMoveAnimation;
base::UmaHistogramPercentageObsoleteDoNotUse( };
kShelfIconFadeOutAnimationHistogram, smoothness);
}
// Returns the id of the display on which |view| is shown. // Returns the id of the display on which |view| is shown.
int64_t GetDisplayIdForView(const View* view) { int64_t GetDisplayIdForView(const View* view) {
...@@ -372,6 +390,13 @@ int ShelfView::GetSizeOfAppButtons(int count, int button_size) { ...@@ -372,6 +390,13 @@ int ShelfView::GetSizeOfAppButtons(int count, int button_size) {
} }
void ShelfView::Init() { void ShelfView::Init() {
move_animation_reporter_ = std::make_unique<IconAnimationMetricsReporter>(
IconAnimationType::kMoveAnimation);
fade_in_animation_reporter_ = std::make_unique<IconAnimationMetricsReporter>(
IconAnimationType::kFadeInAnimation);
fade_out_animation_reporter_ = std::make_unique<IconAnimationMetricsReporter>(
IconAnimationType::kFadeOutAnimation);
separator_ = new views::Separator(); separator_ = new views::Separator();
separator_->SetColor(AshColorProvider::Get()->GetContentLayerColor( separator_->SetColor(AshColorProvider::Get()->GetContentLayerColor(
AshColorProvider::ContentLayerType::kSeparatorColor)); AshColorProvider::ContentLayerType::kSeparatorColor));
...@@ -1356,11 +1381,7 @@ void ShelfView::OnTabletModeChanged() { ...@@ -1356,11 +1381,7 @@ void ShelfView::OnTabletModeChanged() {
void ShelfView::AnimateToIdealBounds() { void ShelfView::AnimateToIdealBounds() {
CalculateIdealBounds(); CalculateIdealBounds();
bounds_animator_->SetAnimationMetricsReporter(move_animation_reporter_.get());
move_animation_tracker_.emplace(
GetWidget()->GetCompositor()->RequestNewThroughputTracker());
move_animation_tracker_->Start(metrics_util::ForSmoothness(
base::BindRepeating(&ReportMoveAnimationSmoothness)));
for (int i = 0; i < view_model_->view_size(); ++i) { for (int i = 0; i < view_model_->view_size(); ++i) {
View* view = view_model_->view_at(i); View* view = view_model_->view_at(i);
...@@ -1383,12 +1404,8 @@ void ShelfView::FadeIn(views::View* view) { ...@@ -1383,12 +1404,8 @@ void ShelfView::FadeIn(views::View* view) {
fade_in_animation_settings.SetPreemptionStrategy( fade_in_animation_settings.SetPreemptionStrategy(
ui::LayerAnimator::IMMEDIATELY_SET_NEW_TARGET); ui::LayerAnimator::IMMEDIATELY_SET_NEW_TARGET);
fade_in_animation_settings.AddObserver(fade_in_animation_delegate_.get()); fade_in_animation_settings.AddObserver(fade_in_animation_delegate_.get());
fade_in_animation_settings.SetAnimationMetricsReporter(
ui::AnimationThroughputReporter reporter( fade_in_animation_reporter_.get());
fade_in_animation_settings.GetAnimator(),
metrics_util::ForSmoothness(
base::BindRepeating(&ReportFadeInAnimationSmoothness)));
view->layer()->SetOpacity(1.f); view->layer()->SetOpacity(1.f);
} }
...@@ -1791,9 +1808,6 @@ void ShelfView::OnFadeInAnimationEnded() { ...@@ -1791,9 +1808,6 @@ void ShelfView::OnFadeInAnimationEnded() {
} }
void ShelfView::OnFadeOutAnimationEnded() { void ShelfView::OnFadeOutAnimationEnded() {
fade_out_animation_tracker_->Stop();
fade_out_animation_tracker_.reset();
// Call PreferredSizeChanged() to notify container to re-layout at the end // Call PreferredSizeChanged() to notify container to re-layout at the end
// of removal animation. // of removal animation.
PreferredSizeChanged(); PreferredSizeChanged();
...@@ -2009,13 +2023,10 @@ void ShelfView::ShelfItemRemoved(int model_index, const ShelfItem& old_item) { ...@@ -2009,13 +2023,10 @@ void ShelfView::ShelfItemRemoved(int model_index, const ShelfItem& old_item) {
if (view->GetVisible() && view->layer()->opacity() > 0.0f) { if (view->GetVisible() && view->layer()->opacity() > 0.0f) {
UpdateShelfItemViewsVisibility(); UpdateShelfItemViewsVisibility();
fade_out_animation_tracker_.emplace(
GetWidget()->GetCompositor()->RequestNewThroughputTracker());
fade_out_animation_tracker_->Start(metrics_util::ForSmoothness(
base::BindRepeating(&ReportFadeOutAnimationSmoothness)));
// The first animation fades out the view. When done we'll animate the rest // The first animation fades out the view. When done we'll animate the rest
// of the views to their target location. // of the views to their target location.
bounds_animator_->SetAnimationMetricsReporter(
fade_out_animation_reporter_.get());
bounds_animator_->AnimateViewTo(view.get(), view->bounds()); bounds_animator_->AnimateViewTo(view.get(), view->bounds());
bounds_animator_->SetAnimationDelegate( bounds_animator_->SetAnimationDelegate(
view.get(), std::unique_ptr<gfx::AnimationDelegate>( view.get(), std::unique_ptr<gfx::AnimationDelegate>(
...@@ -2328,11 +2339,6 @@ void ShelfView::OnBoundsAnimatorProgressed(views::BoundsAnimator* animator) { ...@@ -2328,11 +2339,6 @@ void ShelfView::OnBoundsAnimatorProgressed(views::BoundsAnimator* animator) {
void ShelfView::OnBoundsAnimatorDone(views::BoundsAnimator* animator) { void ShelfView::OnBoundsAnimatorDone(views::BoundsAnimator* animator) {
shelf_->set_is_tablet_mode_animation_running(false); shelf_->set_is_tablet_mode_animation_running(false);
if (move_animation_tracker_) {
move_animation_tracker_->Stop();
move_animation_tracker_.reset();
}
if (snap_back_from_rip_off_view_ && animator == bounds_animator_.get()) { if (snap_back_from_rip_off_view_ && animator == bounds_animator_.get()) {
if (!animator->IsAnimating(snap_back_from_rip_off_view_)) { if (!animator->IsAnimating(snap_back_from_rip_off_view_)) {
// Coming here the animation of the ShelfAppButton is finished and the // Coming here the animation of the ShelfAppButton is finished and the
......
...@@ -24,10 +24,8 @@ ...@@ -24,10 +24,8 @@
#include "ash/shell_observer.h" #include "ash/shell_observer.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "third_party/skia/include/core/SkColor.h" #include "third_party/skia/include/core/SkColor.h"
#include "ui/compositor/throughput_tracker.h"
#include "ui/views/accessibility/view_accessibility.h" #include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/accessible_pane_view.h" #include "ui/views/accessible_pane_view.h"
#include "ui/views/animation/bounds_animator_observer.h" #include "ui/views/animation/bounds_animator_observer.h"
...@@ -696,11 +694,14 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView, ...@@ -696,11 +694,14 @@ class ASH_EXPORT ShelfView : public views::AccessiblePaneView,
std::unique_ptr<FadeInAnimationDelegate> fade_in_animation_delegate_; std::unique_ptr<FadeInAnimationDelegate> fade_in_animation_delegate_;
// Tracks the icon move animation. // The animation metrics reporter for icon move animation.
base::Optional<ui::ThroughputTracker> move_animation_tracker_; std::unique_ptr<ui::AnimationMetricsReporter> move_animation_reporter_;
// Tracks the icon fade-out animation. // The animation metrics reporter for icon fade-in animation.
base::Optional<ui::ThroughputTracker> fade_out_animation_tracker_; std::unique_ptr<ui::AnimationMetricsReporter> fade_in_animation_reporter_;
// The animation metrics reporter for icon fade-out animation.
std::unique_ptr<ui::AnimationMetricsReporter> fade_out_animation_reporter_;
// Called when showing shelf context menu. // Called when showing shelf context menu.
base::RepeatingClosure context_menu_shown_callback_; base::RepeatingClosure context_menu_shown_callback_;
......
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