Revert "wm: Reland improve cross fade animation."

This reverts commit 46da36e1.

Reason for revert: Still breaking 
HomeToOverviewNudgeControllerTest.NudgeHiddenDuringShowAnimation

Original change's description:
> wm: Reland improve cross fade animation.
> 
> Reland crrev.com/c/2224990, including a fix for failing builders and
> a fix with tablet screenshot related.
> 
> Speculative fix crrev.com/c/2218557 was not enough. I was able to repro
> locally and confirm this fix this time. It seems with certain build args
> we can get into a state where the CrossFadeObserver which observes window
> can be not animating, which is strange as the observer observes window
> and deletes itself on animation end.
> 
> It seems in that test and bot, it can happen perhaps OnWindowDestroying
> is called before the window starts animating, or some other edge case.
> In this case, make sure the window is deleted if not animating.
> 
> Test: none
> Bug: 1088169, 1088039
> Change-Id: Ibe035209e83400921f16cff2bb272a8e186934dc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2224023
> Commit-Queue: Sammie Quon <sammiequon@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#777446}

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

Change-Id: Ib273d1ba96dc26a68cd765917f649230a2bbc0d0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1088169, 1088039
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2242218Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777522}

ash/wm/window_animations.cc

@@ -19,6 +19,7 @@
 #include "ash/wm/workspace_controller.h"
 #include "base/check.h"
 #include "base/i18n/rtl.h"
+#include "base/no_destructor.h"
 #include "base/notreached.h"
 #include "base/stl_util.h"
 #include "base/time/time.h"
@@ -73,6 +74,9 @@ constexpr base::TimeDelta kZeroAnimationMs =
 constexpr char kCrossFadeSmoothness[] =
     "Ash.Window.AnimationSmoothness.CrossFade";
 
+base::NoDestructor<ui::HistogramPercentageMetricsReporter<kCrossFadeSmoothness>>
+    g_reporter_cross_fade;
+
 base::TimeDelta GetCrossFadeDuration(aura::Window* window,
                                      const gfx::RectF& old_bounds,
                                      const gfx::Rect& new_bounds) {
@@ -100,8 +104,7 @@ base::TimeDelta GetCrossFadeDuration(aura::Window* window,
 // the layer's animation completes, it deletes the layer and removes itself as
 // an observer.
 class CrossFadeObserver : public aura::WindowObserver,
-                          public ui::ImplicitAnimationObserver,
-                          public ui::AnimationMetricsReporter {
+                          public ui::ImplicitAnimationObserver {
  public:
   // Observes |window| for destruction, but does not take ownership.
   // Takes ownership of |layer_owner| and its child layers.
@@ -125,34 +128,11 @@ class CrossFadeObserver : public aura::WindowObserver,
   }
 
   // ui::ImplicitAnimationObserver:
-  void OnImplicitAnimationsCompleted() override {
-    // Stop the old animator to trigger aborts or ends on any observers it may
-    // have.
-    layer_owner_->root()->GetAnimator()->StopAnimating();
-    delete this;
-  }
-
-  // ui::AnimationMetricsReporter:
-  void Report(int value) override {
-    if (reported_)
-      return;
-
-    UMA_HISTOGRAM_PERCENTAGE(kCrossFadeSmoothness, value);
-    reported_ = true;
-  }
+  void OnImplicitAnimationsCompleted() override { delete this; }
 
  protected:
-  void StopAnimating() {
-    // Trigger OnImplicitAnimationsCompleted() to be called and deletes us. If
-    // no animation is running then do the deletion ourselves.
-    DCHECK(window_);
-    window_->layer()->GetAnimator()->StopAnimating();
-  }
-
-  // Report() gets called for each LayerAnimationElement. For cross fade, its
-  // fairly common to animate both transform and opacity, so only report the
-  // metric once.
-  bool reported_ = false;
+  // Triggers OnImplicitAnimationsCompleted() to be called and deletes us.
+  void StopAnimating() { layer_owner_->root()->GetAnimator()->StopAnimating(); }
 
   aura::Window* window_;  // not owned
   std::unique_ptr<ui::LayerTreeOwner> layer_owner_;
@@ -184,7 +164,6 @@ class CrossFadeUpdateTransformObserver
   void OnLayerAnimationStarted(ui::LayerAnimationSequence* sequence) override {
     DCHECK(!layer_owner_->root()->GetAnimator()->IsAnimatingProperty(
         ui::LayerAnimationElement::TRANSFORM));
-    CrossFadeObserver::OnLayerAnimationStarted(sequence);
   }
 
   // ui::CompositorAnimationObserver:
@@ -266,10 +245,18 @@ void CrossFadeAnimationInternal(
     old_layer->GetAnimator()->StopAnimating();
     old_layer->SetTransform(old_transform);
     ui::ScopedLayerAnimationSettings settings(old_layer->GetAnimator());
+    // Animation observer owns the old layer and deletes itself.
+    CrossFadeObserver* observer =
+        animate_old_layer_transform
+            ? new CrossFadeObserver(window, std::move(old_layer_owner))
+            : new CrossFadeUpdateTransformObserver(window,
+                                                   std::move(old_layer_owner));
+    settings.AddObserver(observer);
     settings.SetTransitionDuration(animation_duration);
     settings.SetTweenType(animation_tween_type);
+    // Only add reporter to |old_layer|.
+    settings.SetAnimationMetricsReporter(g_reporter_cross_fade.get());
     settings.DeferPaint();
-
     if (old_on_top) {
       // Only caching render surface when there is an opacity animation and
       // multiple layers.
@@ -320,20 +307,9 @@ void CrossFadeAnimationInternal(
     new_layer->SetOpacity(kWindowAnimation_HideOpacity);
   }
   {
-    // Animation observer owns the old layer and deletes itself. It should be
-    // attached to the new layer so that if the new layer animation gets
-    // aborted, we can delete the old layer.
-    CrossFadeObserver* observer =
-        animate_old_layer_transform
-            ? new CrossFadeObserver(window, std::move(old_layer_owner))
-            : new CrossFadeUpdateTransformObserver(window,
-                                                   std::move(old_layer_owner));
-
     // Animate the new layer to the identity transform, so the window goes to
     // its newly set bounds.
     ui::ScopedLayerAnimationSettings settings(new_layer->GetAnimator());
-    settings.AddObserver(observer);
-    settings.SetAnimationMetricsReporter(observer);
     settings.SetTransitionDuration(animation_duration);
     settings.SetTweenType(animation_tween_type);
     settings.DeferPaint();

ash/wm/window_animations_unittest.cc

@@ -14,7 +14,6 @@
 #include "ash/wm/wm_event.h"
 #include "ash/wm/workspace_controller.h"
 #include "base/command_line.h"
-#include "base/test/metrics/histogram_tester.h"
 #include "base/time/time.h"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window.h"
@@ -173,37 +172,6 @@ TEST_F(WindowAnimationsTest, CrossFadeToBounds) {
                                        base::TimeDelta::FromSeconds(1));
 }
 
-TEST_F(WindowAnimationsTest, CrossFadeHistograms) {
-  constexpr char kHistogram[] = "Ash.Window.AnimationSmoothness.CrossFade";
-  ui::ScopedAnimationDurationScaleMode test_duration_mode(
-      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
-
-  base::HistogramTester histogram_tester;
-
-  std::unique_ptr<Window> window(CreateTestWindowInShellWithId(0));
-  window->SetBounds(gfx::Rect(5, 10, 320, 240));
-  window->Show();
-
-  Layer* old_layer = window->layer();
-  old_layer->GetAnimator()->StopAnimating();
-
-  auto* window_state = WindowState::Get(window.get());
-  // Cross fade to a larger size, as in a maximize animation. We should get one
-  // recorded histogram.
-  window_state->SetBoundsDirectCrossFade(gfx::Rect(640, 480));
-  old_layer->GetAnimator()->StopAnimating();
-  window->layer()->GetAnimator()->StopAnimating();
-  histogram_tester.ExpectTotalCount(kHistogram, 1);
-
-  // Cross fade to a smaller size, as in a restore animation. Tests that there
-  // is only one recorded histogram.
-  old_layer = window->layer();
-  window_state->SetBoundsDirectCrossFade(gfx::Rect(5, 10, 320, 240));
-  old_layer->GetAnimator()->StopAnimating();
-  window->layer()->GetAnimator()->StopAnimating();
-  histogram_tester.ExpectTotalCount(kHistogram, 2);
-}
-
 // Tests that when crossfading from a window which has a transform, the cross
 // fading animation should be ignored and the window should set to its desired
 // bounds directly.

ash/wm/window_state.h

@@ -352,7 +352,6 @@ class ASH_EXPORT WindowState : public aura::WindowObserver {
   friend class TabletModeWindowState;
   friend class ScopedBoundsChangeAnimation;
   FRIEND_TEST_ALL_PREFIXES(WindowAnimationsTest, CrossFadeToBounds);
-  FRIEND_TEST_ALL_PREFIXES(WindowAnimationsTest, CrossFadeHistograms);
   FRIEND_TEST_ALL_PREFIXES(WindowAnimationsTest,
                            CrossFadeToBoundsFromTransform);
   FRIEND_TEST_ALL_PREFIXES(WindowStateTest, PipWindowMaskRecreated);