Fix hotseat bounds animation recorder

Hotseat bounds animation recorder reports the animation smoothness
for each hotseat state. However, the hotseat state used by
animation recorder is set by HotseatTransitionAnimator which may not
run when hotseat state changes. As a result, the animation smoothness
may be included by the wrong histogram.

In order to access the target hotseat state when starting the hotseat
bounds animation, the hotseat state stored in hotseat widget updates
before setting bounds. Although the comment in old code explains
why it is necessary to update the hotseat state after setting bounds,
swapping the order does not bring any visual difference; neither does
it break any test case.

The test case for hotseat bounds animation recorder will be implemented
after https://crbug.com/1044306 is solved which disables the widget
animation waiter.

Bug: 1059603
Change-Id: I0cea5ae33b9b137b786713691d0d1bac232c652b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095933Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749792}

ash/shelf/hotseat_widget.cc

@@ -491,7 +491,7 @@ void HotseatWidget::UpdateLayout(bool animate) {
     animation_setter.SetPreemptionStrategy(
         ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
     animation_setter.SetAnimationMetricsReporter(
-        shelf_->GetHotseatTransitionMetricsReporter());
+        shelf_->GetHotseatTransitionMetricsReporter(state_));
 
     layer->SetOpacity(new_layout_inputs.opacity);
     SetBounds(new_layout_inputs.bounds);

ash/shelf/shelf.cc

@@ -59,18 +59,13 @@ namespace ash {
 
 // Records smoothness of bounds animations for the HotseatWidget.
 class HotseatWidgetAnimationMetricsReporter
-    : public HotseatTransitionAnimator::Observer,
-      public ui::AnimationMetricsReporter {
+    : public ui::AnimationMetricsReporter {
  public:
-  explicit HotseatWidgetAnimationMetricsReporter(HotseatState state,
-                                                 Shelf* shelf)
-      : target_state_(state) {}
+  HotseatWidgetAnimationMetricsReporter() = default;
+  ~HotseatWidgetAnimationMetricsReporter() override = default;
 
-  ~HotseatWidgetAnimationMetricsReporter() override {}
-
-  void OnHotseatTransitionAnimationWillStart(HotseatState from_state,
-                                             HotseatState to_state) override {
-    target_state_ = to_state;
+  void SetTargetHotseatState(HotseatState target_state) {
+    target_state_ = target_state;
   }
 
   // ui::AnimationMetricsReporter:
@@ -102,7 +97,7 @@ class HotseatWidgetAnimationMetricsReporter
 
  private:
   // The state to which the animation is transitioning.
-  HotseatState target_state_;
+  HotseatState target_state_ = HotseatState::kHidden;
 };
 
 // An animation metrics reporter for the shelf navigation widget.
@@ -329,10 +324,7 @@ void Shelf::CreateHotseatWidget(aura::Window* container) {
   hotseat_widget_->Initialize(container, this);
   shelf_widget_->RegisterHotseatWidget(hotseat_widget());
   hotseat_transition_metrics_reporter_ =
-      std::make_unique<HotseatWidgetAnimationMetricsReporter>(
-          hotseat_widget()->state(), this);
-  shelf_widget_->hotseat_transition_animator()->AddObserver(
-      hotseat_transition_metrics_reporter_.get());
+      std::make_unique<HotseatWidgetAnimationMetricsReporter>();
 }
 
 void Shelf::CreateStatusAreaWidget(aura::Window* status_container) {
@@ -370,8 +362,6 @@ void Shelf::CreateShelfWidget(aura::Window* root) {
 }
 
 void Shelf::ShutdownShelfWidget() {
-  shelf_widget_->hotseat_transition_animator()->RemoveObserver(
-      hotseat_transition_metrics_reporter_.get());
   // The contents view of the hotseat widget may rely on the status area widget.
   // So do explicit destruction here.
   hotseat_widget_.reset();
@@ -598,7 +588,9 @@ ShelfView* Shelf::GetShelfViewForTesting() {
   return shelf_widget_->shelf_view_for_testing();
 }
 
-ui::AnimationMetricsReporter* Shelf::GetHotseatTransitionMetricsReporter() {
+ui::AnimationMetricsReporter* Shelf::GetHotseatTransitionMetricsReporter(
+    HotseatState target_state) {
+  hotseat_transition_metrics_reporter_->SetTargetHotseatState(target_state);
   return hotseat_transition_metrics_reporter_.get();
 }
 

ash/shelf/shelf.h

@@ -231,7 +231,10 @@ class ASH_EXPORT Shelf : public ShelfLayoutManagerObserver {
 
   ShelfTooltipManager* tooltip() { return tooltip_.get(); }
 
-  ui::AnimationMetricsReporter* GetHotseatTransitionMetricsReporter();
+  // |target_state| is the hotseat state after hotseat transition animation.
+  ui::AnimationMetricsReporter* GetHotseatTransitionMetricsReporter(
+      HotseatState target_state);
+
   ui::AnimationMetricsReporter* GetNavigationWidgetAnimationMetricsReporter();
 
  protected:

ash/shelf/shelf_layout_manager.cc

@@ -1256,6 +1256,7 @@ void ShelfLayoutManager::SetState(ShelfVisibilityState visibility_state) {
     MaybeUpdateShelfBackground(change_type);
 
   CalculateTargetBoundsAndUpdateWorkArea();
+  shelf_->hotseat_widget()->SetState(new_hotseat_state);
   UpdateBoundsAndOpacity(true /* animate */);
 
   // OnAutoHideStateChanged Should be emitted when:
@@ -1268,9 +1269,6 @@ void ShelfLayoutManager::SetState(ShelfVisibilityState visibility_state) {
       observer.OnAutoHideStateChanged(state_.auto_hide_state);
   }
 
-  // Do not set the hotseat state until after bounds have been set because
-  // observers rely on final bounds.
-  shelf_->hotseat_widget()->SetState(new_hotseat_state);
   if (previous_hotseat_state != hotseat_state()) {
     if (hotseat_state() == HotseatState::kExtended)
       hotseat_event_handler_ = std::make_unique<HotseatEventHandler>(this);