Revert "Introduce AccessibilityAnimationOneShot"

This reverts commit 1d08391.

Reason for revert: Suspicious about test failures of SelectToSpeakTest.FocusRingMovesWithMouse
https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/24407/test-results

Original change's description:
> Introduce AccessibilityAnimationOneShot
>
> This change cleans up animation paths under ash/accessibility/ui. It
> does this by encapsulating proper usage of the Compositor's
> CompositorAnimationObserver.
>
> In particular, the observer is supposed to only be added during the
> duration of the animation and removed thereafter. If this is not done,
> it can lead to lock ups for accessibility (see bug).
>
> Furthermore, coupling with AccessibilityLayer can lead to instances
> where AccessibilityLayer::OnAnimationStep (called as a
> CompositorAnimationObserver), destroys the AccessibilityLayer, leading
> to hard-to-debug use-after-free bugs.
>
> Bug: 1222698
> Test: existing ash_unittests
> Change-Id: Ia20d08c3c6335bb6fd8edeca6afbaa5992055e28
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3056713
> Commit-Queue: David Tseng <dtseng@chromium.org>
> Reviewed-by: Katie Dektar <katie@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#906392}

Bug: 1222698
Change-Id: If5b67efd69b5cd17ca2c485f37a2352c312b524e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3056323
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Owners-Override: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#906527}

ash/BUILD.gn

@@ -107,8 +107,6 @@ component("ash") {
     "accessibility/switch_access/point_scan_layer.h",
     "accessibility/switch_access/point_scan_layer_animation_info.cc",
     "accessibility/switch_access/point_scan_layer_animation_info.h",
-    "accessibility/ui/accessibility_animation_one_shot.cc",
-    "accessibility/ui/accessibility_animation_one_shot.h",
     "accessibility/ui/accessibility_confirmation_dialog.cc",
     "accessibility/ui/accessibility_confirmation_dialog.h",
     "accessibility/ui/accessibility_cursor_ring_layer.cc",

ash/accessibility/switch_access/point_scan_controller.cc

@@ -31,10 +31,6 @@ void PointScanController::Start() {
   HideAll();
   ResetAnimation();
   StartHorizontalRangeScan();
-  point_scan_animation_ = std::make_unique<AccessibilityAnimationOneShot>(
-      gfx::Rect(0, 0, 0, 0),
-      base::BindRepeating(&PointScanController::AnimateLine,
-                          base::Unretained(this)));
 }
 
 void PointScanController::StartHorizontalRangeScan() {
@@ -94,7 +90,6 @@ void PointScanController::Stop() {
   state_ = PointScanState::kOff;
   vertical_line_layer_->Pause();
   vertical_range_layer_->SetOpacity(0);
-  point_scan_animation_.reset();
 }
 
 void PointScanController::HideAll() {
@@ -181,14 +176,18 @@ void PointScanController::SetSpeedDipsPerSecond(int speed_dips_per_second) {
 
 void PointScanController::OnDeviceScaleFactorChanged() {}
 
+void PointScanController::OnAnimationStep(base::TimeTicks timestamp) {
+  AnimateLine(timestamp);
+}
+
 void PointScanController::UpdateTimeInfo(
     PointScanLayerAnimationInfo* animation_info,
     base::TimeTicks timestamp) {
   animation_info->start_time = animation_info->change_time;
   animation_info->change_time = timestamp;
 }
 
-bool PointScanController::AnimateLine(base::TimeTicks timestamp) {
+void PointScanController::AnimateLine(base::TimeTicks timestamp) {
   if (horizontal_range_layer_->IsMoving()) {
     ComputeOffset(&horizontal_range_layer_info_, timestamp);
     horizontal_range_layer_->SetSubpixelPositionOffset(
@@ -210,8 +209,6 @@ bool PointScanController::AnimateLine(base::TimeTicks timestamp) {
         gfx::Vector2dF(0.0, vertical_line_layer_info_.offset));
     UpdateTimeInfo(&vertical_line_layer_info_, timestamp);
   }
-
-  return false;
 }
 
 }  // namespace ash

ash/accessibility/switch_access/point_scan_controller.h

@@ -6,7 +6,6 @@
 #define ASH_ACCESSIBILITY_SWITCH_ACCESS_POINT_SCAN_CONTROLLER_H_
 
 #include "ash/accessibility/switch_access/point_scan_layer_animation_info.h"
-#include "ash/accessibility/ui/accessibility_animation_one_shot.h"
 #include "ash/accessibility/ui/accessibility_layer.h"
 #include "ash/ash_export.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -57,10 +56,11 @@ class ASH_EXPORT PointScanController : public AccessibilityLayerDelegate {
  private:
   // AccessibilityLayerDelegate implementation:
   void OnDeviceScaleFactorChanged() override;
+  void OnAnimationStep(base::TimeTicks timestamp) override;
 
   void UpdateTimeInfo(PointScanLayerAnimationInfo* animation_info,
                       base::TimeTicks timestamp);
-  bool AnimateLine(base::TimeTicks timestamp);
+  void AnimateLine(base::TimeTicks timestamp);
 
   PointScanLayerAnimationInfo horizontal_range_layer_info_;
   std::unique_ptr<PointScanLayer> horizontal_range_layer_;
@@ -70,7 +70,6 @@ class ASH_EXPORT PointScanController : public AccessibilityLayerDelegate {
   std::unique_ptr<PointScanLayer> vertical_range_layer_;
   PointScanLayerAnimationInfo vertical_line_layer_info_;
   std::unique_ptr<PointScanLayer> vertical_line_layer_;
-  std::unique_ptr<AccessibilityAnimationOneShot> point_scan_animation_;
 
   PointScanState state_ = PointScanState::kOff;
 };

ash/accessibility/switch_access/point_scan_layer.cc

@@ -78,6 +78,9 @@ bool PointScanLayer::IsMoving() const {
   return is_moving_;
 }
 
+bool PointScanLayer::NeedToAnimate() const {
+  return true;
+}
 int PointScanLayer::GetInset() const {
   return 0;
 }

ash/accessibility/switch_access/point_scan_layer.h

@@ -43,6 +43,7 @@ class PointScanLayer : public AccessibilityLayer {
   gfx::Rect bounds() { return layer()->bounds(); }
 
   // AccessibilityLayer overrides:
+  bool NeedToAnimate() const override;
   int GetInset() const override;
 
  private:

ash/accessibility/ui/accessibility_focus_ring_controller_impl.cc

@@ -114,17 +114,11 @@ void AccessibilityFocusRingControllerImpl::SetCursorRing(
         this, kCursorRingColorRed, kCursorRingColorGreen, kCursorRingColorBlue);
   }
   cursor_layer_->Set(location);
-  cursor_animation_ = std::make_unique<AccessibilityAnimationOneShot>(
-      gfx::Rect(location.x(), location.y(), 0, 0),
-      base::BindRepeating(
-          &AccessibilityFocusRingControllerImpl::AnimateCursorRing,
-          base::Unretained(this)));
   OnLayerChange(&cursor_animation_info_);
 }
 
 void AccessibilityFocusRingControllerImpl::HideCursorRing() {
   cursor_layer_.reset();
-  cursor_animation_.reset();
 }
 
 void AccessibilityFocusRingControllerImpl::SetCaretRing(
@@ -137,17 +131,11 @@ void AccessibilityFocusRingControllerImpl::SetCaretRing(
   }
 
   caret_layer_->Set(location);
-  caret_animation_ = std::make_unique<AccessibilityAnimationOneShot>(
-      gfx::Rect(location.x(), location.y(), 0, 0),
-      base::BindRepeating(
-          &AccessibilityFocusRingControllerImpl::AnimateCaretRing,
-          base::Unretained(this)));
   OnLayerChange(&caret_animation_info_);
 }
 
 void AccessibilityFocusRingControllerImpl::HideCaretRing() {
   caret_layer_.reset();
-  caret_animation_.reset();
 }
 
 void AccessibilityFocusRingControllerImpl::SetNoFadeForTesting() {
@@ -191,30 +179,52 @@ void AccessibilityFocusRingControllerImpl::OnDeviceScaleFactorChanged() {
     iter->second->UpdateFocusRingsFromInfo(this);
 }
 
-bool AccessibilityFocusRingControllerImpl::AnimateCursorRing(
+void AccessibilityFocusRingControllerImpl::OnAnimationStep(
+    base::TimeTicks timestamp) {
+  bool has_animation_finished = true;
+  for (auto iter = focus_ring_groups_.begin(); iter != focus_ring_groups_.end();
+       ++iter) {
+    if (iter->second->CanAnimate())
+      has_animation_finished &= iter->second->AnimateFocusRings(timestamp);
+  }
+
+  // Release all resources, including observers if needed.
+  if (has_animation_finished) {
+    for (auto iter = focus_ring_groups_.begin();
+         iter != focus_ring_groups_.end(); ++iter) {
+      iter->second->ClearAnimationObservation();
+    }
+  }
+
+  if (cursor_layer_ && cursor_layer_->CanAnimate())
+    AnimateCursorRing(timestamp);
+
+  if (caret_layer_ && caret_layer_->CanAnimate())
+    AnimateCaretRing(timestamp);
+}
+
+void AccessibilityFocusRingControllerImpl::AnimateCursorRing(
     base::TimeTicks timestamp) {
   CHECK(cursor_layer_);
 
   ComputeOpacity(&cursor_animation_info_, timestamp);
   if (cursor_animation_info_.opacity == 0.0) {
     cursor_layer_.reset();
-    return true;
+    return;
   }
   cursor_layer_->SetOpacity(cursor_animation_info_.opacity);
-  return false;
 }
 
-bool AccessibilityFocusRingControllerImpl::AnimateCaretRing(
+void AccessibilityFocusRingControllerImpl::AnimateCaretRing(
     base::TimeTicks timestamp) {
   CHECK(caret_layer_);
 
   ComputeOpacity(&caret_animation_info_, timestamp);
   if (caret_animation_info_.opacity == 0.0) {
     caret_layer_.reset();
-    return true;
+    return;
   }
   caret_layer_->SetOpacity(caret_animation_info_.opacity);
-  return false;
 }
 
 AccessibilityFocusRingGroup*

ash/accessibility/ui/accessibility_focus_ring_controller_impl.h

@@ -8,7 +8,6 @@
 #include <memory>
 #include <vector>
 
-#include "ash/accessibility/ui/accessibility_animation_one_shot.h"
 #include "ash/accessibility/ui/accessibility_focus_ring.h"
 #include "ash/accessibility/ui/accessibility_focus_ring_group.h"
 #include "ash/accessibility/ui/accessibility_layer.h"
@@ -75,13 +74,14 @@ class ASH_EXPORT AccessibilityFocusRingControllerImpl
  private:
   // AccessibilityLayerDelegate overrides.
   void OnDeviceScaleFactorChanged() override;
+  void OnAnimationStep(base::TimeTicks timestamp) override;
 
   void UpdateHighlightFromHighlightRects();
 
   bool AnimateFocusRings(base::TimeTicks timestamp,
                          AccessibilityFocusRingGroup* focus_ring);
-  bool AnimateCursorRing(base::TimeTicks timestamp);
-  bool AnimateCaretRing(base::TimeTicks timestamp);
+  void AnimateCursorRing(base::TimeTicks timestamp);
+  void AnimateCaretRing(base::TimeTicks timestamp);
 
   void OnLayerChange(LayerAnimationInfo* animation_info);
 
@@ -94,12 +94,10 @@ class ASH_EXPORT AccessibilityFocusRingControllerImpl
   LayerAnimationInfo cursor_animation_info_;
   gfx::Point cursor_location_;
   std::unique_ptr<AccessibilityCursorRingLayer> cursor_layer_;
-  std::unique_ptr<AccessibilityAnimationOneShot> cursor_animation_;
 
   LayerAnimationInfo caret_animation_info_;
   gfx::Point caret_location_;
   std::unique_ptr<AccessibilityCursorRingLayer> caret_layer_;
-  std::unique_ptr<AccessibilityAnimationOneShot> caret_animation_;
 
   std::vector<gfx::Rect> highlight_rects_;
   std::unique_ptr<AccessibilityHighlightLayer> highlight_layer_;

ash/accessibility/ui/accessibility_focus_ring_group.cc

@@ -59,7 +59,6 @@ void AccessibilityFocusRingGroup::UpdateFocusRingsFromInfo(
     AccessibilityLayerDelegate* delegate) {
   previous_focus_rings_.swap(focus_rings_);
   focus_rings_.clear();
-  focus_animation_.reset();
   RectsToRings(focus_ring_info_->rects_in_screen, &(focus_rings_));
   focus_layers_.resize(focus_rings_.size());
   if (focus_rings_.empty())
@@ -72,7 +71,7 @@ void AccessibilityFocusRingGroup::UpdateFocusRingsFromInfo(
   }
 
   if (focus_ring_info_->behavior == FocusRingBehavior::PERSIST &&
-      !no_fade_for_testing_) {
+      focus_layers_[0]->CanAnimate() && !no_fade_for_testing_) {
     // In PERSIST mode, animate the first ring to its destination
     // location, then set the rest of the rings directly.
     // If no_fade_for_testing_ is set, don't wait for animation.
@@ -90,12 +89,12 @@ void AccessibilityFocusRingGroup::UpdateFocusRingsFromInfo(
         focus_ring_info_->color, focus_ring_info_->secondary_color,
         focus_ring_info_->background_color);
   }
+}
 
-  // Start watching for animations.
-  focus_animation_ = std::make_unique<AccessibilityAnimationOneShot>(
-      focus_rings_[0].GetBounds(),
-      base::BindRepeating(&AccessibilityFocusRingGroup::AnimateFocusRings,
-                          base::Unretained(this)));
+bool AccessibilityFocusRingGroup::CanAnimate() const {
+  if (no_fade_for_testing_)
+    return false;
+  return !focus_rings_.empty() && focus_layers_[0]->CanAnimate();
 }
 
 bool AccessibilityFocusRingGroup::AnimateFocusRings(base::TimeTicks timestamp) {
@@ -329,6 +328,11 @@ void AccessibilityFocusRingGroup::SplitIntoParagraphShape(
   *bottom = bottom_rect;
 }
 
+void AccessibilityFocusRingGroup::ClearAnimationObservation() {
+  for (auto& focus_layer : focus_layers_)
+    focus_layer->ClearAnimationObservation();
+}
+
 AccessibilityFocusRing AccessibilityFocusRingGroup::RingFromSortedRects(
     const std::vector<gfx::Rect>& rects) const {
   if (rects.size() == 1)