From fd1486852e72128d90159d0516af1b63db4f3335 Mon Sep 17 00:00:00 2001 From: Yuki Shiino Date: Thu, 29 Jul 2021 02:25:13 +0000 Subject: [PATCH] Revert "Introduce AccessibilityAnimationOneShot" This reverts commit 1d08391ee4505464beb169bb0f906bd064ec0d31. 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 > Reviewed-by: Katie Dektar > 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 Commit-Queue: Yuki Shiino Owners-Override: Yuki Shiino Cr-Commit-Position: refs/heads/master@{#906527} --- ash/BUILD.gn | 2 - .../switch_access/point_scan_controller.cc | 13 ++--- .../switch_access/point_scan_controller.h | 5 +- .../switch_access/point_scan_layer.cc | 3 ++ .../switch_access/point_scan_layer.h | 1 + .../ui/accessibility_animation_one_shot.cc | 38 -------------- .../ui/accessibility_animation_one_shot.h | 49 ------------------- ...ccessibility_focus_ring_controller_impl.cc | 46 ++++++++++------- ...accessibility_focus_ring_controller_impl.h | 8 ++- .../ui/accessibility_focus_ring_group.cc | 18 ++++--- .../ui/accessibility_focus_ring_group.h | 6 ++- .../ui/accessibility_highlight_layer.cc | 4 ++ .../ui/accessibility_highlight_layer.h | 1 + ash/accessibility/ui/accessibility_layer.cc | 32 ++++++++++++ ash/accessibility/ui/accessibility_layer.h | 31 +++++++++++- ash/accessibility/ui/focus_ring_controller.cc | 2 + ash/accessibility/ui/focus_ring_controller.h | 1 + ash/accessibility/ui/focus_ring_layer.cc | 4 ++ ash/accessibility/ui/focus_ring_layer.h | 1 + 19 files changed, 132 insertions(+), 133 deletions(-) delete mode 100644 ash/accessibility/ui/accessibility_animation_one_shot.cc delete mode 100644 ash/accessibility/ui/accessibility_animation_one_shot.h diff --git a/ash/BUILD.gn b/ash/BUILD.gn index 45d7e4480b7a7..877752279f961 100644 --- a/ash/BUILD.gn +++ b/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", diff --git a/ash/accessibility/switch_access/point_scan_controller.cc b/ash/accessibility/switch_access/point_scan_controller.cc index aeef0643a12af..1b7ed91bbd4de 100644 --- a/ash/accessibility/switch_access/point_scan_controller.cc +++ b/ash/accessibility/switch_access/point_scan_controller.cc @@ -31,10 +31,6 @@ void PointScanController::Start() { HideAll(); ResetAnimation(); StartHorizontalRangeScan(); - point_scan_animation_ = std::make_unique( - 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,6 +176,10 @@ 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) { @@ -188,7 +187,7 @@ void PointScanController::UpdateTimeInfo( 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 diff --git a/ash/accessibility/switch_access/point_scan_controller.h b/ash/accessibility/switch_access/point_scan_controller.h index ef2dd934ad063..56526b98a81d2 100644 --- a/ash/accessibility/switch_access/point_scan_controller.h +++ b/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 horizontal_range_layer_; @@ -70,7 +70,6 @@ class ASH_EXPORT PointScanController : public AccessibilityLayerDelegate { std::unique_ptr vertical_range_layer_; PointScanLayerAnimationInfo vertical_line_layer_info_; std::unique_ptr vertical_line_layer_; - std::unique_ptr point_scan_animation_; PointScanState state_ = PointScanState::kOff; }; diff --git a/ash/accessibility/switch_access/point_scan_layer.cc b/ash/accessibility/switch_access/point_scan_layer.cc index dd79fa4bfe030..7160fc2643264 100644 --- a/ash/accessibility/switch_access/point_scan_layer.cc +++ b/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; } diff --git a/ash/accessibility/switch_access/point_scan_layer.h b/ash/accessibility/switch_access/point_scan_layer.h index a1b3b4c02a9fd..5487b37a59523 100644 --- a/ash/accessibility/switch_access/point_scan_layer.h +++ b/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: diff --git a/ash/accessibility/ui/accessibility_animation_one_shot.cc b/ash/accessibility/ui/accessibility_animation_one_shot.cc deleted file mode 100644 index 18481d19cfeec..0000000000000 --- a/ash/accessibility/ui/accessibility_animation_one_shot.cc +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2021 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/accessibility/ui/accessibility_animation_one_shot.h" - -#include "ash/shell.h" -#include "ui/compositor/layer.h" -#include "ui/display/display.h" -#include "ui/display/screen.h" - -namespace ash { - -AccessibilityAnimationOneShot::AccessibilityAnimationOneShot( - const gfx::Rect& bounds_in_dip, - base::RepeatingCallback callback) - : callback_(callback) { - display::Display display = - display::Screen::GetScreen()->GetDisplayMatching(bounds_in_dip); - aura::Window* root_window = Shell::GetRootWindowForDisplayId(display.id()); - ui::Compositor* compositor = root_window->layer()->GetCompositor(); - animation_observation_.Observe(compositor); -} - -AccessibilityAnimationOneShot::~AccessibilityAnimationOneShot() = default; - -void AccessibilityAnimationOneShot::OnAnimationStep(base::TimeTicks timestamp) { - if (callback_.Run(timestamp)) - animation_observation_.Reset(); -} - -void AccessibilityAnimationOneShot::OnCompositingShuttingDown( - ui::Compositor* compositor) { - if (compositor && animation_observation_.IsObservingSource(compositor)) - animation_observation_.Reset(); -} - -} // namespace ash diff --git a/ash/accessibility/ui/accessibility_animation_one_shot.h b/ash/accessibility/ui/accessibility_animation_one_shot.h deleted file mode 100644 index c407060911ee6..0000000000000 --- a/ash/accessibility/ui/accessibility_animation_one_shot.h +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2021 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. - -#ifndef ASH_ACCESSIBILITY_UI_ACCESSIBILITY_ANIMATION_ONE_SHOT_H_ -#define ASH_ACCESSIBILITY_UI_ACCESSIBILITY_ANIMATION_ONE_SHOT_H_ - -#include "base/callback.h" -#include "base/scoped_observation.h" -#include "ui/compositor/compositor.h" -#include "ui/compositor/compositor_animation_observer.h" -#include "ui/gfx/geometry/rect.h" - -namespace ash { - -// Adapts the observer-like interface in |CompositorAnimationObserver| for -// simpler, safe usage. -// -// The |Compositor| expects all |CompositorAnimationObserver|s to be added while -// animating, but removed soon after. This class ensures this occurs by -// requiring the user pass a callback rather than dealing with the observer. -class AccessibilityAnimationOneShot : public ui::CompositorAnimationObserver { - public: - // |callback| below returns true if animation has finished; false to - // continue animating. - AccessibilityAnimationOneShot( - const gfx::Rect& bounds_in_dip, - base::RepeatingCallback callback); - ~AccessibilityAnimationOneShot() override; - AccessibilityAnimationOneShot(const AccessibilityAnimationOneShot&) = delete; - AccessibilityAnimationOneShot& operator=( - const AccessibilityAnimationOneShot&) = delete; - - private: - // CompositorAnimationObserver: - void OnAnimationStep(base::TimeTicks timestamp) override; - void OnCompositingShuttingDown(ui::Compositor* compositor) override; - - base::RepeatingCallback callback_; - base::ScopedObservation - animation_observation_{this}; -}; - -} // namespace ash - -#endif // ASH_ACCESSIBILITY_UI_ACCESSIBILITY_ANIMATION_ONE_SHOT_H_ diff --git a/ash/accessibility/ui/accessibility_focus_ring_controller_impl.cc b/ash/accessibility/ui/accessibility_focus_ring_controller_impl.cc index a3b20ad77f76a..6cb88c052f86c 100644 --- a/ash/accessibility/ui/accessibility_focus_ring_controller_impl.cc +++ b/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( - 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( - 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* diff --git a/ash/accessibility/ui/accessibility_focus_ring_controller_impl.h b/ash/accessibility/ui/accessibility_focus_ring_controller_impl.h index 1f7cbb2f3098c..20953349b12d0 100644 --- a/ash/accessibility/ui/accessibility_focus_ring_controller_impl.h +++ b/ash/accessibility/ui/accessibility_focus_ring_controller_impl.h @@ -8,7 +8,6 @@ #include #include -#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 cursor_layer_; - std::unique_ptr cursor_animation_; LayerAnimationInfo caret_animation_info_; gfx::Point caret_location_; std::unique_ptr caret_layer_; - std::unique_ptr caret_animation_; std::vector highlight_rects_; std::unique_ptr highlight_layer_; diff --git a/ash/accessibility/ui/accessibility_focus_ring_group.cc b/ash/accessibility/ui/accessibility_focus_ring_group.cc index 7223b267e2cd4..04b5da781b1ef 100644 --- a/ash/accessibility/ui/accessibility_focus_ring_group.cc +++ b/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( - 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& rects) const { if (rects.size() == 1) diff --git a/ash/accessibility/ui/accessibility_focus_ring_group.h b/ash/accessibility/ui/accessibility_focus_ring_group.h index 4b810234af382..b6bd3bdfa4dea 100644 --- a/ash/accessibility/ui/accessibility_focus_ring_group.h +++ b/ash/accessibility/ui/accessibility_focus_ring_group.h @@ -8,7 +8,6 @@ #include #include -#include "ash/accessibility/ui/accessibility_animation_one_shot.h" #include "ash/accessibility/ui/accessibility_focus_ring.h" #include "ash/accessibility/ui/accessibility_focus_ring_layer.h" #include "ash/accessibility/ui/accessibility_layer.h" @@ -30,6 +29,7 @@ class ASH_EXPORT AccessibilityFocusRingGroup { virtual ~AccessibilityFocusRingGroup(); void UpdateFocusRingsFromInfo(AccessibilityLayerDelegate* delegate); + bool CanAnimate() const; bool AnimateFocusRings(base::TimeTicks timestamp); // Returns true if the focus ring has changed, false if there were no changes. @@ -41,6 +41,9 @@ class ASH_EXPORT AccessibilityFocusRingGroup { static void ComputeOpacity(LayerAnimationInfo* animation_info, base::TimeTicks timestamp); + // Clears the underlying layer's animation observation. + void ClearAnimationObservation(); + LayerAnimationInfo* focus_animation_info() { return &focus_animation_info_; } void set_no_fade_for_testing() { no_fade_for_testing_ = true; } @@ -73,7 +76,6 @@ class ASH_EXPORT AccessibilityFocusRingGroup { std::unique_ptr focus_ring_info_; std::vector previous_focus_rings_; std::vector> focus_layers_; - std::unique_ptr focus_animation_; std::vector focus_rings_; LayerAnimationInfo focus_animation_info_; bool no_fade_for_testing_ = false; diff --git a/ash/accessibility/ui/accessibility_highlight_layer.cc b/ash/accessibility/ui/accessibility_highlight_layer.cc index ce79c769fb6ec..a70dce6dfe79b 100644 --- a/ash/accessibility/ui/accessibility_highlight_layer.cc +++ b/ash/accessibility/ui/accessibility_highlight_layer.cc @@ -57,6 +57,10 @@ void AccessibilityHighlightLayer::Set(const std::vector& rects, /*stack_at_top=*/false); } +bool AccessibilityHighlightLayer::NeedToAnimate() const { + return false; +} + int AccessibilityHighlightLayer::GetInset() const { return kLayerMargin; } diff --git a/ash/accessibility/ui/accessibility_highlight_layer.h b/ash/accessibility/ui/accessibility_highlight_layer.h index db51b87d665bb..96b97a0528054 100644 --- a/ash/accessibility/ui/accessibility_highlight_layer.h +++ b/ash/accessibility/ui/accessibility_highlight_layer.h @@ -25,6 +25,7 @@ class ASH_EXPORT AccessibilityHighlightLayer : public AccessibilityLayer { void Set(const std::vector& rects, SkColor color); // AccessibilityLayer overrides: + bool NeedToAnimate() const override; int GetInset() const override; private: diff --git a/ash/accessibility/ui/accessibility_layer.cc b/ash/accessibility/ui/accessibility_layer.cc index d1e0595f996e0..8ccdd07969a64 100644 --- a/ash/accessibility/ui/accessibility_layer.cc +++ b/ash/accessibility/ui/accessibility_layer.cc @@ -5,8 +5,11 @@ #include "ash/accessibility/ui/accessibility_layer.h" #include "ui/aura/window.h" +#include "ui/compositor/compositor_animation_observer.h" #include "ui/compositor/layer.h" #include "ui/compositor/paint_recorder.h" +#include "ui/display/display.h" +#include "ui/display/screen.h" #include "ui/gfx/canvas.h" namespace ui { @@ -45,6 +48,10 @@ void AccessibilityLayer::SetSubpixelPositionOffset( layer_->SetSubpixelPositionOffset(offset); } +bool AccessibilityLayer::CanAnimate() const { + return animation_observation_.IsObserving(); +} + void AccessibilityLayer::CreateOrUpdateLayer(aura::Window* root_window, const char* layer_name, const gfx::Rect& bounds, @@ -73,6 +80,17 @@ void AccessibilityLayer::CreateOrUpdateLayer(aura::Window* root_window, layer_->SetBounds(bounds); gfx::Rect layer_bounds(0, 0, bounds.width(), bounds.height()); layer_->SchedulePaint(layer_bounds); + + if (NeedToAnimate()) { + // Update the animation observer. + display::Display display = + display::Screen::GetScreen()->GetDisplayMatching(bounds); + ui::Compositor* compositor = root_window->layer()->GetCompositor(); + if (compositor && !animation_observation_.IsObservingSource(compositor)) { + ClearAnimationObservation(); + animation_observation_.Observe(compositor); + } + } } void AccessibilityLayer::OnDeviceScaleFactorChanged( @@ -82,4 +100,18 @@ void AccessibilityLayer::OnDeviceScaleFactorChanged( delegate_->OnDeviceScaleFactorChanged(); } +void AccessibilityLayer::OnAnimationStep(base::TimeTicks timestamp) { + delegate_->OnAnimationStep(timestamp); + // |this| may have been deleted by |delegate_| at this point. +} + +void AccessibilityLayer::OnCompositingShuttingDown(ui::Compositor* compositor) { + if (compositor && animation_observation_.IsObservingSource(compositor)) + ClearAnimationObservation(); +} + +void AccessibilityLayer::ClearAnimationObservation() { + animation_observation_.Reset(); +} + } // namespace ash diff --git a/ash/accessibility/ui/accessibility_layer.h b/ash/accessibility/ui/accessibility_layer.h index e33f73ebb1ad9..fd98cead06c0f 100644 --- a/ash/accessibility/ui/accessibility_layer.h +++ b/ash/accessibility/ui/accessibility_layer.h @@ -8,7 +8,10 @@ #include #include "base/macros.h" +#include "base/scoped_observation.h" #include "base/time/time.h" +#include "ui/compositor/compositor.h" +#include "ui/compositor/compositor_animation_observer.h" #include "ui/compositor/layer_delegate.h" #include "ui/gfx/geometry/rect.h" @@ -17,6 +20,7 @@ class Window; } namespace ui { +class Compositor; class Layer; } // namespace ui @@ -28,13 +32,18 @@ class AccessibilityLayerDelegate { public: virtual void OnDeviceScaleFactorChanged() = 0; + // Called by a layer during animation observation on its compositor. Returns + // true when animation has finished. + virtual void OnAnimationStep(base::TimeTicks timestamp) = 0; + protected: virtual ~AccessibilityLayerDelegate() {} }; // AccessibilityLayer manages a global always-on-top layer used to // highlight or annotate UI elements for accessibility. -class AccessibilityLayer : public ui::LayerDelegate { +class AccessibilityLayer : public ui::LayerDelegate, + public ui::CompositorAnimationObserver { public: explicit AccessibilityLayer(AccessibilityLayerDelegate* delegate); ~AccessibilityLayer() override; @@ -51,6 +60,16 @@ class AccessibilityLayer : public ui::LayerDelegate { // Set the layer's offset from parent layer. void SetSubpixelPositionOffset(const gfx::Vector2dF& offset); + // Returns true if this layer is in a composited window with an + // animation observer. + bool CanAnimate() const; + + // Clears this layer's animation observation. + void ClearAnimationObservation(); + + // Returns true if a layer needs to animate. + virtual bool NeedToAnimate() const = 0; + // Gets the inset for this layer in DIPs. This is used to increase // the bounding box to provide space for any margins or padding. virtual int GetInset() const = 0; @@ -82,9 +101,19 @@ class AccessibilityLayer : public ui::LayerDelegate { void OnDeviceScaleFactorChanged(float old_device_scale_factor, float new_device_scale_factor) override; + // CompositorAnimationObserver overrides: + void OnAnimationStep(base::TimeTicks timestamp) override; + void OnCompositingShuttingDown(ui::Compositor* compositor) override; + // The object that owns this layer. AccessibilityLayerDelegate* delegate_; + base::ScopedObservation + animation_observation_{this}; + DISALLOW_COPY_AND_ASSIGN(AccessibilityLayer); }; diff --git a/ash/accessibility/ui/focus_ring_controller.cc b/ash/accessibility/ui/focus_ring_controller.cc index bf4bffadd3a15..33ec394d49468 100644 --- a/ash/accessibility/ui/focus_ring_controller.cc +++ b/ash/accessibility/ui/focus_ring_controller.cc @@ -96,6 +96,8 @@ void FocusRingController::OnDeviceScaleFactorChanged() { UpdateFocusRing(); } +void FocusRingController::OnAnimationStep(base::TimeTicks timestamp) {} + void FocusRingController::SetWidget(views::Widget* widget) { if (widget_) { widget_->RemoveObserver(this); diff --git a/ash/accessibility/ui/focus_ring_controller.h b/ash/accessibility/ui/focus_ring_controller.h index ac1dd0a21b0c9..37c1d07fc81f0 100644 --- a/ash/accessibility/ui/focus_ring_controller.h +++ b/ash/accessibility/ui/focus_ring_controller.h @@ -38,6 +38,7 @@ class ASH_EXPORT FocusRingController : public AccessibilityLayerDelegate, private: // AccessibilityLayerDelegate. void OnDeviceScaleFactorChanged() override; + void OnAnimationStep(base::TimeTicks timestamp) override; // Sets the focused |widget|. void SetWidget(views::Widget* widget); diff --git a/ash/accessibility/ui/focus_ring_layer.cc b/ash/accessibility/ui/focus_ring_layer.cc index c4a732fca19fc..64c43a0469f04 100644 --- a/ash/accessibility/ui/focus_ring_layer.cc +++ b/ash/accessibility/ui/focus_ring_layer.cc @@ -40,6 +40,10 @@ void FocusRingLayer::ResetColor() { custom_color_.reset(); } +bool FocusRingLayer::NeedToAnimate() const { + return true; +} + int FocusRingLayer::GetInset() const { return kShadowRadius + 2; } diff --git a/ash/accessibility/ui/focus_ring_layer.h b/ash/accessibility/ui/focus_ring_layer.h index 1ee89a3c7ebcf..9f38e1ef01c82 100644 --- a/ash/accessibility/ui/focus_ring_layer.h +++ b/ash/accessibility/ui/focus_ring_layer.h @@ -22,6 +22,7 @@ class FocusRingLayer : public AccessibilityLayer { ~FocusRingLayer() override; // AccessibilityLayer overrides: + bool NeedToAnimate() const override; int GetInset() const override; // Set a custom color, or reset to the default.