Make pure virtual interface for ProgressIndicatorAnimationRegistry.

The existing HoldingSpaceAnimationRegistry becomes an instance and a
new registry will be added in a follow up CL for the dictation tray.

Bug: 1280377
Change-Id: Ia20ddcf015c99047cd298f2238dccd8c35eebbea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3402679
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/main@{#961170}

WATCHLISTS

@@ -1126,6 +1126,7 @@
     'holding_space': {
       'filepath': 'ash/public/cpp/holding_space|'\
                   'ash/system/holding_space|'\
+                  'ash/system/progress_indicator|'\
                   'chrome/browser/lacros/.*holding_space|'\
                   'chrome/browser/ui/ash/holding_space|'\
                   'chrome/browser/ui/ash/thumbnail_loader|'\

ash/BUILD.gn

@@ -1419,6 +1419,7 @@ component("ash") {
     "system/privacy_screen/privacy_screen_toast_controller.h",
     "system/privacy_screen/privacy_screen_toast_view.cc",
     "system/privacy_screen/privacy_screen_toast_view.h",
+    "system/progress_indicator/progress_indicator_animation_registry.h",
     "system/rotation/rotation_lock_feature_pod_controller.cc",
     "system/rotation/rotation_lock_feature_pod_controller.h",
     "system/scheduled_feature/scheduled_feature.cc",

ash/system/accessibility/dictation_button_tray.cc

@@ -51,7 +51,9 @@ gfx::ImageSkia GetIconImage(bool enabled) {
 
 DictationProgressIndicator::DictationProgressIndicator(
     const DictationButtonTray* tray)
-    : HoldingSpaceProgressIndicator(/*animation_key=*/tray), tray_(tray) {}
+    : HoldingSpaceProgressIndicator(/*animation_registry=*/nullptr,
+                                    /*animation_key=*/tray),
+      tray_(tray) {}
 
 bool DictationProgressIndicator::IsVisible() {
   absl::optional<float> progress = CalculateProgress();

ash/system/holding_space/holding_space_animation_registry.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "ash/system/holding_space/holding_space_animation_registry.h"
+
 #include <map>
 #include <memory>
 #include <set>
@@ -11,7 +13,6 @@
 #include "ash/public/cpp/holding_space/holding_space_controller_observer.h"
 #include "ash/public/cpp/holding_space/holding_space_model.h"
 #include "ash/public/cpp/holding_space/holding_space_model_observer.h"
-#include "ash/system/holding_space/holding_space_animation_registry.h"
 #include "ash/system/holding_space/holding_space_progress_icon_animation.h"
 #include "ash/system/holding_space/holding_space_progress_ring_animation.h"
 #include "base/containers/contains.h"

ash/system/holding_space/holding_space_animation_registry.h

@@ -8,6 +8,7 @@
 #include "ash/ash_export.h"
 #include "ash/shell.h"
 #include "ash/shell_observer.h"
+#include "ash/system/progress_indicator/progress_indicator_animation_registry.h"
 #include "base/callback.h"
 #include "base/callback_list.h"
 #include "base/scoped_observation.h"
@@ -17,12 +18,20 @@ namespace ash {
 class HoldingSpaceProgressIconAnimation;
 class HoldingSpaceProgressRingAnimation;
 
-// A lazily initialized singleton registry for holding space animations. Since
-// registered animations are owned by the singleton, they can be shared across
-// different UI components as well have a lifetime which is decoupled from UI
-// component lifetime. Note that the singleton may only exist while `Shell` is
-// alive and will automatically delete itself when `Shell` is being destroyed.
-class ASH_EXPORT HoldingSpaceAnimationRegistry : public ShellObserver {
+// A lazily initialized singleton registry for holding space animations.
+//
+// Since registered animations are owned by the singleton, they can be shared
+// across different UI components as well have a lifetime which is decoupled
+// from UI component lifetime. Note that the singleton may only exist while
+// `Shell` is alive and will automatically delete itself when `Shell` is being
+// destroyed.
+//
+// Supported animation types:
+//   * Progress icon animation - see `ProgressIndicatorAnimationRegistry`.
+//   * Progress ring animation - see `ProgressIndicatorAnimationRegistry`.
+class ASH_EXPORT HoldingSpaceAnimationRegistry
+    : public ProgressIndicatorAnimationRegistry,
+      public ShellObserver {
  public:
   HoldingSpaceAnimationRegistry(const HoldingSpaceAnimationRegistry&) = delete;
   HoldingSpaceAnimationRegistry& operator=(
@@ -34,55 +43,17 @@ class ASH_EXPORT HoldingSpaceAnimationRegistry : public ShellObserver {
   // when `Shell` is being destroyed.
   static HoldingSpaceAnimationRegistry* GetInstance();
 
-  using ProgressIconAnimationChangedCallbackList =
-      base::RepeatingCallbackList<void(HoldingSpaceProgressIconAnimation*)>;
-
-  // Adds the specified `callback` to be notified of changes to the progress
-  // icon animation associated with the specified `key`. Progress icon
-  // animations independently drive the animation of properties for a progress
-  // indicator's inner icon, as opposed to progress ring animations which
-  // independently drive the animation of properties for a progress indicator's
-  // outer ring. The `callback` will continue to receive events so long as both
-  // `this` and the returned subscription exist.
+  // ProgressIndicatorAnimationRegistry:
   base::CallbackListSubscription AddProgressIconAnimationChangedCallbackForKey(
       const void* key,
-      ProgressIconAnimationChangedCallbackList::CallbackType callback);
-
-  using ProgressRingAnimationChangedCallbackList =
-      base::RepeatingCallbackList<void(HoldingSpaceProgressRingAnimation*)>;
-
-  // Adds the specified `callback` to be notified of changes to the progress
-  // ring animation associated with the specified `key`. Progress ring
-  // animations independently drive the animation of properties for a progress
-  // indicator's outer ring, as opposed to progress icon animations which
-  // independently drive the animation of properties for a progress indicator's
-  // inner icon. The `callback` will continue to receive events so long as both
-  // `this` and the returned subscription exist.
+      ProgressIconAnimationChangedCallbackList::CallbackType callback) override;
   base::CallbackListSubscription AddProgressRingAnimationChangedCallbackForKey(
       const void* key,
-      ProgressRingAnimationChangedCallbackList::CallbackType callback);
-
-  // Returns the progress icon animation registered for the specified `key`.
-  // Progress icon animations independently drive the animation of properties
-  // for a progress indicator's inner icon, as opposed to progress ring
-  // animations which independently drive the animation of properties for a
-  // progress indicator's outer ring. For cumulative progress, the animation is
-  // keyed on a pointer to the holding space controller. For individual item
-  // progress, the animation is keyed on a pointer to the holding space item
-  // itself. NOTE: This may return `nullptr` if no such animation is registered.
+      ProgressRingAnimationChangedCallbackList::CallbackType callback) override;
   HoldingSpaceProgressIconAnimation* GetProgressIconAnimationForKey(
-      const void* key);
-
-  // Returns the progress ring animation registered for the specified `key`.
-  // Progress ring animations independently drive the animation of properties
-  // for a progress indicator's outer ring, as opposed to progress icon
-  // animations which independently drive the animation of properties for a
-  // progress indicator's inner icon. For cumulative progress, the animation is
-  // keyed on a pointer to the holding space controller. For individual item
-  // progress, the animation is keyed on a pointer to the holding space item
-  // itself. NOTE: This may return `nullptr` if no such animation is registered.
+      const void* key) override;
   HoldingSpaceProgressRingAnimation* GetProgressRingAnimationForKey(
-      const void* key);
+      const void* key) override;
 
  private:
   HoldingSpaceAnimationRegistry();

ash/system/holding_space/holding_space_progress_indicator.cc

@@ -13,8 +13,10 @@
 #include "ash/public/cpp/holding_space/holding_space_progress.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/style/ash_color_provider.h"
+#include "ash/system/holding_space/holding_space_animation_registry.h"
 #include "ash/system/holding_space/holding_space_progress_icon_animation.h"
 #include "ash/system/holding_space/holding_space_progress_ring_animation.h"
+#include "ash/system/progress_indicator/progress_indicator_animation_registry.h"
 #include "base/scoped_observation.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "third_party/skia/include/core/SkPathBuilder.h"
@@ -156,7 +158,9 @@ class HoldingSpaceControllerProgressIndicator
  public:
   explicit HoldingSpaceControllerProgressIndicator(
       HoldingSpaceController* controller)
-      : HoldingSpaceProgressIndicator(/*animation_key=*/controller),
+      : HoldingSpaceProgressIndicator(
+            /*animation_registry=*/HoldingSpaceAnimationRegistry::GetInstance(),
+            /*animation_key=*/controller),
         controller_(controller) {
     controller_observation_.Observe(controller_);
     if (controller_->model())
@@ -252,7 +256,10 @@ class HoldingSpaceItemProgressIndicator : public HoldingSpaceProgressIndicator,
                                           public HoldingSpaceModelObserver {
  public:
   explicit HoldingSpaceItemProgressIndicator(const HoldingSpaceItem* item)
-      : HoldingSpaceProgressIndicator(/*animation_key=*/item), item_(item) {
+      : HoldingSpaceProgressIndicator(
+            /*animation_registry=*/HoldingSpaceAnimationRegistry::GetInstance(),
+            /*animation_key=*/item),
+        item_(item) {
     model_observation_.Observe(HoldingSpaceController::Get()->model());
   }
 
@@ -300,16 +307,17 @@ constexpr char HoldingSpaceProgressIndicator::kClassName[];
 constexpr float HoldingSpaceProgressIndicator::kProgressComplete;
 
 HoldingSpaceProgressIndicator::HoldingSpaceProgressIndicator(
+    ProgressIndicatorAnimationRegistry* animation_registry,
     const void* animation_key)
-    : animation_key_(animation_key) {
-  HoldingSpaceAnimationRegistry* animation_registry =
-      HoldingSpaceAnimationRegistry::GetInstance();
+    : animation_registry_(animation_registry), animation_key_(animation_key) {
+  if (!animation_registry_)
+    return;
 
   // Register to be notified of changes to the icon animation associated with
   // this progress indicator's `animation_key_`. Note that it is safe to use a
   // raw pointer here since `this` owns the subscription.
   icon_animation_changed_subscription_ =
-      animation_registry->AddProgressIconAnimationChangedCallbackForKey(
+      animation_registry_->AddProgressIconAnimationChangedCallbackForKey(
           animation_key_,
           base::BindRepeating(
               &HoldingSpaceProgressIndicator::OnProgressIconAnimationChanged,
@@ -318,7 +326,7 @@ HoldingSpaceProgressIndicator::HoldingSpaceProgressIndicator(
   // If an `icon_animation` is already registered, perform additional
   // initialization.
   HoldingSpaceProgressIconAnimation* icon_animation =
-      animation_registry->GetProgressIconAnimationForKey(animation_key_);
+      animation_registry_->GetProgressIconAnimationForKey(animation_key_);
   if (icon_animation)
     OnProgressIconAnimationChanged(icon_animation);
 
@@ -326,7 +334,7 @@ HoldingSpaceProgressIndicator::HoldingSpaceProgressIndicator(
   // this progress indicator's `animation_key_`. Note that it is safe to use a
   // raw pointer here since `this` owns the subscription.
   ring_animation_changed_subscription_ =
-      animation_registry->AddProgressRingAnimationChangedCallbackForKey(
+      animation_registry_->AddProgressRingAnimationChangedCallbackForKey(
           animation_key_,
           base::BindRepeating(
               &HoldingSpaceProgressIndicator::OnProgressRingAnimationChanged,
@@ -335,7 +343,7 @@ HoldingSpaceProgressIndicator::HoldingSpaceProgressIndicator(
   // If `ring_animation` is already registered, perform additional
   // initialization.
   HoldingSpaceProgressRingAnimation* ring_animation =
-      animation_registry->GetProgressRingAnimationForKey(animation_key_);
+      animation_registry_->GetProgressRingAnimationForKey(animation_key_);
   if (ring_animation)
     OnProgressRingAnimationChanged(ring_animation);
 }
@@ -405,8 +413,9 @@ void HoldingSpaceProgressIndicator::OnPaintLayer(
     const ui::PaintContext& context) {
   // Look up the associated `ring_animation` (if one exists).
   HoldingSpaceProgressRingAnimation* ring_animation =
-      HoldingSpaceAnimationRegistry::GetInstance()
-          ->GetProgressRingAnimationForKey(animation_key_);
+      animation_registry_
+          ? animation_registry_->GetProgressRingAnimationForKey(animation_key_)
+          : nullptr;
 
   // Unless `this` is animating, nothing will paint if `progress_` is complete.
   if (progress_ == kProgressComplete && !ring_animation)
@@ -484,8 +493,9 @@ void HoldingSpaceProgressIndicator::OnPaintLayer(
 
   // Look up the associated `icon_animation` (if one exists).
   HoldingSpaceProgressIconAnimation* icon_animation =
-      HoldingSpaceAnimationRegistry::GetInstance()
-          ->GetProgressIconAnimationForKey(animation_key_);
+      animation_registry_
+          ? animation_registry_->GetProgressIconAnimationForKey(animation_key_)
+          : nullptr;
 
   float inner_ring_stroke_width = GetInnerRingStrokeWidth(layer());
 

ash/system/holding_space/holding_space_progress_indicator.h

@@ -9,7 +9,6 @@
 #include <vector>
 
 #include "ash/ash_export.h"
-#include "ash/system/holding_space/holding_space_animation_registry.h"
 #include "base/callback_list.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/compositor/layer_delegate.h"
@@ -21,6 +20,7 @@ class HoldingSpaceController;
 class HoldingSpaceItem;
 class HoldingSpaceProgressIconAnimation;
 class HoldingSpaceProgressRingAnimation;
+class ProgressIndicatorAnimationRegistry;
 
 // A class owning a `ui::Layer` which paints indication of progress.
 // NOTE: The owned `layer()` is not painted if progress == `1.f`.
@@ -75,10 +75,13 @@ class ASH_EXPORT HoldingSpaceProgressIndicator : public ui::LayerOwner,
 
  protected:
   // Each progress indicator is associated with an `animation_key_` which is
-  // used to look up animations in the `HoldingSpaceAnimationRegistry`. When an
+  // used to look up animations in the provided `animation_registry`. When an
   // animation exists, it will be painted in lieu of the determinate progress
   // indication that would otherwise be painted for the cached `progress_`.
-  explicit HoldingSpaceProgressIndicator(const void* animation_key);
+  // NOTE: `animation_registry` may be `nullptr` if animations are not needed.
+  HoldingSpaceProgressIndicator(
+      ProgressIndicatorAnimationRegistry* animation_registry,
+      const void* animation_key);
 
   // Returns the calculated progress to paint to the owned `layer()`. This is
   // invoked during `UpdateVisualState()` just prior to painting.
@@ -94,30 +97,36 @@ class ASH_EXPORT HoldingSpaceProgressIndicator : public ui::LayerOwner,
   void UpdateVisualState() override;
 
   // Invoked when the icon `animation` associated with this progress indicator's
-  // `animation_key_` has changed in the `HoldingSpaceAnimationRegistry`.
+  // `animation_key_` has changed in the `animation_registry_`.
   // NOTE: The specified `animation` may be `nullptr`.
   void OnProgressIconAnimationChanged(
       HoldingSpaceProgressIconAnimation* animation);
 
   // Invoked when the ring `animation` associated with this progress indicator's
-  // `animation_key_` has changed in the `HoldingSpaceAnimationRegistry`.
+  // `animation_key_` has changed in the `animation_registry_`.
   // NOTE: The specified `animation` may be `nullptr`.
   void OnProgressRingAnimationChanged(
       HoldingSpaceProgressRingAnimation* animation);
 
-  // The key for which to look up animations in the
-  // `HoldingSpaceAnimationRegistry`. When an animation exists, it will be
-  // painted in lieu of the determinate progress indication that would otherwise
-  // be painted for the cached `progress_`.
+  // The animation registry in which to look up animations for the associated
+  // `animation_key_`. When an animation exists, it will be painted in lieu of
+  // the determinate progress indication that would otherwise be painted for the
+  // cached `progress_`.
+  ProgressIndicatorAnimationRegistry* const animation_registry_;
+
+  // The key for which to look up animations in the `animation_registry_`.
+  // When an animation exists, it will be painted in lieu of the determinate
+  // progress indication that would otherwise be painted for the cached
+  // `progress_`.
   const void* const animation_key_;
 
   // A subscription to receive events when the icon animation associated with
   // this progress indicator's `animation_key_` has changed in the
-  // `HoldingSpaceAnimationRegistry`.
+  // `animation_registry_`.
   base::CallbackListSubscription icon_animation_changed_subscription_;
 
   // A subscription to receive events on updates to the icon animation owned by
-  // the `HoldingSpaceAnimationRegistry` which is associated with this progress
+  // the `animation_registry_` which is associated with this progress
   // indicator's `animation_key_`. On icon animation update, the progress
   // indicator will `InvalidateLayer()` to trigger paint of the next animation
   // frame.
@@ -125,11 +134,11 @@ class ASH_EXPORT HoldingSpaceProgressIndicator : public ui::LayerOwner,
 
   // A subscription to receive events when the ring animation associated with
   // this progress indicator's `animation_key_` has changed in the
-  // `HoldingSpaceAnimationRegistry`.
+  // `animation_registry_`.
   base::CallbackListSubscription ring_animation_changed_subscription_;
 
   // A subscription to receive events on updates to the ring animation owned by
-  // the `HoldingSpaceAnimationRegistry` which is associated with this progress
+  // the `animation_registry_` which is associated with this progress
   // indicator's `animation_key_`. On ring animation update, the progress
   // indicator will `InvalidateLayer()` to trigger paint of the next animation
   // frame.

ash/system/holding_space/holding_space_progress_indicator_unittest.cc

@@ -15,7 +15,8 @@ namespace {
 class TestHoldingSpaceProgressIndicator : public HoldingSpaceProgressIndicator {
  public:
   TestHoldingSpaceProgressIndicator()
-      : HoldingSpaceProgressIndicator(/*animation_key=*/this) {}
+      : HoldingSpaceProgressIndicator(/*animation_registry=*/nullptr,
+                                      /*animation_key=*/this) {}
 
   void SetProgress(const absl::optional<float>& progress) {
     progress_ = progress;

ash/system/holding_space/holding_space_tray_icon_preview.cc

@@ -15,6 +15,7 @@
 #include "ash/public/cpp/shelf_config.h"
 #include "ash/shelf/shelf.h"
 #include "ash/style/ash_color_provider.h"
+#include "ash/system/holding_space/holding_space_animation_registry.h"
 #include "ash/system/holding_space/holding_space_progress_indicator.h"
 #include "ash/system/holding_space/holding_space_tray_icon.h"
 #include "ash/system/tray/tray_constants.h"

ash/system/progress_indicator/OWNERS

@@ -0,0 +1 @@
+file://ash/public/cpp/holding_space/OWNERS

ash/system/progress_indicator/progress_indicator_animation_registry.h

@@ -0,0 +1,69 @@
+// Copyright 2022 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_SYSTEM_PROGRESS_INDICATOR_PROGRESS_INDICATOR_ANIMATION_REGISTRY_H_
+#define ASH_SYSTEM_PROGRESS_INDICATOR_PROGRESS_INDICATOR_ANIMATION_REGISTRY_H_
+
+#include "base/callback_forward.h"
+
+namespace ash {
+
+class HoldingSpaceProgressIconAnimation;
+class HoldingSpaceProgressRingAnimation;
+
+// A registry for progress indicator animations.
+//
+// Since animations are owned by the registry, they can be shared across
+// different UI components as well have a lifetime which is decoupled from UI
+// component lifetime.
+//
+// Supported animation types:
+//   * Progress icon animation - independently drive the animation of properties
+//     for a progress indicator's inner icon, as opposed to progress ring
+//     animations which independently drive the animation of properties for a
+//     progress indicator's outer ring.
+//   * Progress ring animation - independently drive the animation of properties
+//     for a progress indicator's outer ring, as opposed to progress icon
+//     animations which independently drive the animation of properties for a
+//     progress indicator's inner icon.
+class ProgressIndicatorAnimationRegistry {
+ public:
+  using ProgressIconAnimationChangedCallbackList =
+      base::RepeatingCallbackList<void(HoldingSpaceProgressIconAnimation*)>;
+
+  // Adds the specified `callback` to be notified of changes to the progress
+  // icon animation associated with the specified `key`. The `callback` will
+  // continue to receive events so long as both `this` and the returned
+  // subscription exist.
+  virtual base::CallbackListSubscription
+  AddProgressIconAnimationChangedCallbackForKey(
+      const void* key,
+      ProgressIconAnimationChangedCallbackList::CallbackType callback) = 0;
+
+  using ProgressRingAnimationChangedCallbackList =
+      base::RepeatingCallbackList<void(HoldingSpaceProgressRingAnimation*)>;
+
+  // Adds the specified `callback` to be notified of changes to the progress
+  // ring animation associated with the specified `key`. The `callback` will
+  // continue to receive events so long as both `this` and the returned
+  // subscription exist.
+  virtual base::CallbackListSubscription
+  AddProgressRingAnimationChangedCallbackForKey(
+      const void* key,
+      ProgressRingAnimationChangedCallbackList::CallbackType callback) = 0;
+
+  // Returns the progress icon animation registered for the specified `key`.
+  // NOTE: This may return `nullptr` if no such animation is registered.
+  virtual HoldingSpaceProgressIconAnimation* GetProgressIconAnimationForKey(
+      const void* key) = 0;
+
+  // Returns the progress ring animation registered for the specified `key`.
+  // NOTE: This may return `nullptr` if no such animation is registered.
+  virtual HoldingSpaceProgressRingAnimation* GetProgressRingAnimationForKey(
+      const void* key) = 0;
+};
+
+}  // namespace ash
+
+#endif  // ASH_SYSTEM_PROGRESS_INDICATOR_PROGRESS_INDICATOR_ANIMATION_REGISTRY_H_