Update TODOs assigned to myself

Most TODOs are assigned to the linked bugs. TODOs related to GroupEffect
are outdated so they are removed.

Bug: None
Change-Id: I7b413773904efd93c6b6959545c5d74f7d4760ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2227299
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775276}

cc/animation/animation_delegate.h

@@ -13,12 +13,6 @@ namespace cc {
 
 class CC_ANIMATION_EXPORT AnimationDelegate {
  public:
-  // TODO(yigu): The Notify* methods will be called multiple times per
-  // animation (once for effect/property pairing).
-  // Ideally, we would only notify start once (e.g., wait on all effects to
-  // start before notifying delegate) this way effect becomes an internal
-  // details of the animation. Perhaps we can do that at some point maybe as
-  // part of https://bugs.chromium.org/p/chromium/issues/detail?id=810003
   virtual void NotifyAnimationStarted(base::TimeTicks monotonic_time,
                                       int target_property,
                                       int group) = 0;

cc/animation/keyframe_model.cc

@@ -156,7 +156,7 @@ void KeyframeModel::SetRunState(RunState run_state,
 
 void KeyframeModel::Pause(base::TimeDelta pause_offset) {
   // Convert pause offset which is in local time to monotonic time.
-  // TODO(yigu): This should be scaled by playbackrate. http://crbug.com/912407
+  // TODO(crbug.com/912407): This should be scaled by playbackrate.
   base::TimeTicks monotonic_time =
       pause_offset + start_time_ + total_paused_duration_;
   SetRunState(PAUSED, monotonic_time);
@@ -246,8 +246,7 @@ bool KeyframeModel::InEffect(base::TimeTicks monotonic_time) const {
   return CalculateActiveTime(monotonic_time).has_value();
 }
 
-// TODO(yigu): Local time should be scaled by playback rate by spec.
-// https://crbug.com/912407.
+// TODO(crbug.com/912407): Local time should be scaled by playback rate by spec.
 base::TimeDelta KeyframeModel::ConvertMonotonicTimeToLocalTime(
     base::TimeTicks monotonic_time) const {
   // When waiting on receiving a start time, then our global clock is 'stuck' at

cc/animation/worklet_animation.cc

@@ -163,10 +163,6 @@ void WorkletAnimation::UpdateInputState(MutatorInputState* input_state,
 
   switch (state_) {
     case State::PENDING:
-      // TODO(yigu): cc side WorkletAnimation is only capable of handling single
-      // keyframe effect at the moment. We should pass in the number of effects
-      // once Worklet Group Effect is fully implemented in cc.
-      // https://crbug.com/767043.
       input_state->Add({worklet_animation_id(), name(),
                         current_time->InMillisecondsF(), CloneOptions(),
                         CloneEffectTimings()});
@@ -187,8 +183,6 @@ void WorkletAnimation::UpdateInputState(MutatorInputState* input_state,
 
 void WorkletAnimation::SetOutputState(
     const MutatorOutputState::AnimationState& state) {
-  // TODO(yigu): cc side WorkletAnimation is only capable of handling single
-  // keyframe effect at the moment. https://crbug.com/767043.
   DCHECK_EQ(state.local_times.size(), 1u);
   local_time_ = state.local_times[0];
 }

third_party/blink/renderer/core/animation/worklet_animation_controller.cc

@@ -110,9 +110,9 @@ WorkletAnimationController::EnsureMainThreadMutatorDispatcher(
   return mutator_dispatcher;
 }
 
-// TODO(yigu): Currently one animator name is synced back per registration.
-// Eventually all registered names should be synced in batch once a module
-// completes its loading in the worklet scope. https://crbug.com/920722.
+// TODO(crbug.com/920722): Currently one animator name is synced back per
+// registration. Eventually all registered names should be synced in batch once
+// a module completes its loading in the worklet scope.
 void WorkletAnimationController::SynchronizeAnimatorName(
     const String& animator_name) {
   animator_names_.insert(animator_name);

third_party/blink/renderer/core/animation/worklet_animation_controller.h

@@ -76,8 +76,8 @@ class CORE_EXPORT WorkletAnimationController
 
   WTF::HashSet<String> animator_names_;
 
-  // TODO(yigu): The following proxy is needed for platform/ to access this
-  // class. We should bypass it eventually.
+  // TODO(crbug.com/1090515): The following proxy is needed for platform/ to
+  // access this class. We should bypass it eventually.
   std::unique_ptr<MainThreadMutatorClient> main_thread_mutator_client_;
   scoped_refptr<base::SingleThreadTaskRunner> mutator_task_runner_;
 

third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope.cc

@@ -218,9 +218,9 @@ void AnimationWorkletGlobalScope::registerAnimator(
   // TODO(https://crbug.com/923063): Ensure worklet definitions are compatible
   // across global scopes.
   animator_definitions_.Set(name, definition);
-  // TODO(yigu): Currently one animator name is synced back per registration.
-  // Eventually all registered names should be synced in batch once a module
-  // completes its loading in the worklet scope. https://crbug.com/920722.
+  // TODO(crbug.com/920722): Currently one animator name is synced back per
+  // registration. Eventually all registered names should be synced in batch
+  // once a module completes its loading in the worklet scope.
   if (AnimationWorkletProxyClient* proxy_client =
           AnimationWorkletProxyClient::From(Clients())) {
     proxy_client->SynchronizeAnimatorName(name);
@@ -295,8 +295,8 @@ void AnimationWorkletGlobalScope::MigrateAnimatorsTo(
                                      "Animator", "state");
       // If an animator state function throws or the state is not
       // serializable, the animator will be removed from the global scope.
-      // TODO(yigu): We should post an error message to console in case of
-      // exceptions.
+      // TODO(crbug.com/1090522): We should post an error message to console in
+      // case of exceptions.
       v8::Local<v8::Value> state = animator->State(isolate, exception_state);
       if (exception_state.HadException()) {
         exception_state.ClearException();

third_party/blink/renderer/modules/animationworklet/worklet_animation.cc

@@ -327,11 +327,11 @@ void WorkletAnimation::play(ExceptionState& exception_state) {
     if (!target)
       continue;
 
-    // TODO(yigu): Currently we have to keep a set of worklet animations in
-    // ElementAnimations so that the compositor knows that there are active
-    // worklet animations running. Ideally, this should be done via the regular
-    // Animation path, i.e., unify the logic between the two Animations.
-    // https://crbug.com/896249.
+    // TODO(crbug.com/896249): Currently we have to keep a set of worklet
+    // animations in ElementAnimations so that the compositor knows that there
+    // are active worklet animations running. Ideally, this should be done via
+    // the regular Animation path, i.e., unify the logic between the two
+    // Animations.
     target->EnsureElementAnimations().GetWorkletAnimations().insert(this);
     target->SetNeedsAnimationStyleRecalc();
   }
@@ -386,10 +386,10 @@ void WorkletAnimation::cancel() {
   has_started_ = false;
   local_times_.Fill(base::nullopt);
   running_on_main_thread_ = false;
-  // TODO(yigu): Because this animation has been detached and will not receive
-  // updates anymore, we have to update its value upon cancel. Similar to
-  // regular animations, we should not detach them immediately and update the
-  // value in the next frame. See https://crbug.com/883312.
+  // TODO(crbug.com/883312): Because this animation has been detached and will
+  // not receive updates anymore, we have to update its value upon cancel.
+  // Similar to regular animations, we should not detach them immediately and
+  // update the value in the next frame.
   if (IsActive(play_state_)) {
     for (auto& effect : effects_) {
       effect->UpdateInheritedTime(base::nullopt, base::nullopt,
@@ -403,11 +403,11 @@ void WorkletAnimation::cancel() {
     Element* target = effect->EffectTarget();
     if (!target)
       continue;
-    // TODO(yigu): Currently we have to keep a set of worklet animations in
-    // ElementAnimations so that the compositor knows that there are active
-    // worklet animations running. Ideally, this should be done via the regular
-    // Animation path, i.e., unify the logic between the two Animations.
-    // https://crbug.com/896249.
+    // TODO(crbug.com/896249): Currently we have to keep a set of worklet
+    // animations in ElementAnimations so that the compositor knows that there
+    // are active worklet animations running. Ideally, this should be done via
+    // the regular Animation path, i.e., unify the logic between the two
+    // Animations.
     target->EnsureElementAnimations().GetWorkletAnimations().erase(this);
     target->SetNeedsAnimationStyleRecalc();
   }
@@ -621,8 +621,8 @@ bool WorkletAnimation::StartOnCompositor() {
     // update the compositor to have the correct orientation and start/end
     // offset information.
     compositor_animation_ = CompositorAnimation::CreateWorkletAnimation(
-        id_, animator_name_, playback_rate_,
-        std::move(options_), std::move(effect_timings_));
+        id_, animator_name_, playback_rate_, std::move(options_),
+        std::move(effect_timings_));
     compositor_animation_->SetAnimationDelegate(this);
   }
 

third_party/blink/renderer/platform/animation/compositor_animation_delegate.h

@@ -16,8 +16,6 @@ class PLATFORM_EXPORT CompositorAnimationDelegate {
  public:
   virtual ~CompositorAnimationDelegate() = default;
 
-  // TODO(yigu): The Notify* methods should be called from cc once per
-  // animation.
   virtual void NotifyAnimationStarted(double monotonic_time, int group) = 0;
   virtual void NotifyAnimationFinished(double monotonic_time, int group) = 0;
   virtual void NotifyAnimationAborted(double monotonic_time, int group) = 0;