Separate devtools notification from PlayStateUpdateScope.

This patch pulls the probe notification code out of the destructor for
PlayStateUpdateScope. This is part of a refactoring project to align
the implementation of web-animations with the spec.  Over the course of
the next few patches, the update scope will be phased out and the probe
notification will be directly called whenever there is a potential state
change.

Bug: 960944
Change-Id: Icb7356ed3d2a8eb7dbe18a0e1c1a1ca88016e22a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1822321Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699768}

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

@@ -154,6 +154,7 @@ Animation::Animation(ExecutionContext* execution_context,
                      AnimationEffect* content)
     : ContextLifecycleObserver(execution_context),
       internal_play_state_(kIdle),
+      reported_play_state_(kIdle),
       animation_play_state_(kIdle),
       playback_rate_(1),
       start_time_(),
@@ -1449,31 +1450,8 @@ Animation::PlayStateUpdateScope::PlayStateUpdateScope(
 Animation::PlayStateUpdateScope::~PlayStateUpdateScope() {
   AnimationPlayState old_play_state = initial_play_state_;
   AnimationPlayState new_play_state = animation_->CalculatePlayState();
-
-  // TODO(crbug.com/958433): Phase out internal_play_state_ in favor of the spec
-  // compliant version. At present, both are needed as the web exposed play
-  // state cannot simply be inferred from the internal play state.
   animation_->internal_play_state_ = new_play_state;
   animation_->animation_play_state_ = animation_->CalculateAnimationPlayState();
-  if (old_play_state != new_play_state) {
-    bool was_active = old_play_state == kPending || old_play_state == kRunning;
-    bool is_active = new_play_state == kPending || new_play_state == kRunning;
-    if (!was_active && is_active) {
-      TRACE_EVENT_NESTABLE_ASYNC_BEGIN1(
-          "blink.animations,devtools.timeline,benchmark,rail", "Animation",
-          animation_, "data", inspector_animation_event::Data(*animation_));
-    } else if (was_active && !is_active) {
-      TRACE_EVENT_NESTABLE_ASYNC_END1(
-          "blink.animations,devtools.timeline,benchmark,rail", "Animation",
-          animation_, "endData",
-          inspector_animation_state_event::Data(*animation_));
-    } else {
-      TRACE_EVENT_NESTABLE_ASYNC_INSTANT1(
-          "blink.animations,devtools.timeline,benchmark,rail", "Animation",
-          animation_, "data",
-          inspector_animation_state_event::Data(*animation_));
-    }
-  }
 
   // Ordering is important, the ready promise should resolve/reject before
   // the finished promise.
@@ -1539,11 +1517,7 @@ Animation::PlayStateUpdateScope::~PlayStateUpdateScope() {
       break;
   }
   animation_->EndUpdatingState();
-
-  if (old_play_state != new_play_state) {
-    probe::AnimationPlayStateChanged(animation_->document_, animation_,
-                                     old_play_state, new_play_state);
-  }
+  animation_->NotifyProbe();
 }
 
 void Animation::AddedEventListener(
@@ -1624,6 +1598,35 @@ void Animation::RejectAndResetPromiseMaybeAsync(AnimationPromise* promise) {
   }
 }
 
+void Animation::NotifyProbe() {
+  AnimationPlayState old_play_state = reported_play_state_;
+  AnimationPlayState new_play_state =
+      pending() ? kPending : CalculateAnimationPlayState();
+
+  if (old_play_state != new_play_state) {
+    probe::AnimationPlayStateChanged(document_, this, old_play_state,
+                                     new_play_state);
+    reported_play_state_ = new_play_state;
+
+    bool was_active = old_play_state == kPending || old_play_state == kRunning;
+    bool is_active = new_play_state == kPending || new_play_state == kRunning;
+
+    if (!was_active && is_active) {
+      TRACE_EVENT_NESTABLE_ASYNC_BEGIN1(
+          "blink.animations,devtools.timeline,benchmark,rail", "Animation",
+          this, "data", inspector_animation_event::Data(*this));
+    } else if (was_active && !is_active) {
+      TRACE_EVENT_NESTABLE_ASYNC_END1(
+          "blink.animations,devtools.timeline,benchmark,rail", "Animation",
+          this, "endData", inspector_animation_state_event::Data(*this));
+    } else {
+      TRACE_EVENT_NESTABLE_ASYNC_INSTANT1(
+          "blink.animations,devtools.timeline,benchmark,rail", "Animation",
+          this, "data", inspector_animation_state_event::Data(*this));
+    }
+  }
+}
+
 void Animation::Trace(blink::Visitor* visitor) {
   visitor->Trace(content_);
   visitor->Trace(document_);

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

@@ -322,11 +322,20 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
   double TimelineTime() const;
   DocumentTimeline& TickingTimeline();
 
+  // Tracking the state of animations in dev tools.
+  void NotifyProbe();
+
   String id_;
 
   // Extended play state with additional pending state for managing timing of
   // micro-tasks.
+  // TODO(crbug.com/958433): Phase out this version of the play state. Should
+  // just need the reported play state.
   AnimationPlayState internal_play_state_;
+  // Extended play state reported to dev tools. This play state has an
+  // additional pending state that is not part of the spec by expected by dev
+  // tools.
+  AnimationPlayState reported_play_state_;
   // Web exposed play state, which does not have pending state.
   AnimationPlayState animation_play_state_;
   double playback_rate_;