Skip to content

Commit

Permalink
[M97 merge - requested by matthewjoseph@] Code health cleanup: replac…
Browse files Browse the repository at this point in the history
…ing animations.

Animation::Update performed a synchronous processing of the finish
microtask to ensure that finished events where dispatched ahead of
replace events. This step does not align with the spec.  Instead we
should be queuing the replace event.  Microtasks will be processed in
the correct order.

Spec link: https://www.w3.org/TR/web-animations-1/#timelines


Change-Id: Ibe7753e792fb6cf905bbe6815a080a8cc51c2803

(cherry picked from commit d4fb69f)

(cherry picked from commit 5fe0a96)

Bug: 1290858,1296150
Change-Id: Ibe7753e792fb6cf905bbe6815a080a8cc51c2803
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3414765
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/main@{#964223}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3455770
Reviewed-by: Michael Ershov <miersh@google.com>
Owners-Override: Michael Ershov <miersh@google.com>
Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com>
Cr-Original-Commit-Position: refs/branch-heads/4664@{#1477}
Cr-Original-Branched-From: 24dc4ee-refs/heads/main@{#929512}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3475376
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Commit-Queue: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/4692@{#1524}
Cr-Branched-From: 038cd96-refs/heads/main@{#938553}
  • Loading branch information
kevers-google authored and Krishna Govind committed Feb 18, 2022
1 parent 2e2f351 commit ae61862
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
4 changes: 0 additions & 4 deletions third_party/blink/renderer/core/animation/animation.cc
Expand Up @@ -2317,10 +2317,6 @@ bool Animation::Update(TimingUpdateReason reason) {

if (reason == kTimingUpdateForAnimationFrame) {
if (idle || CalculateAnimationPlayState() == kFinished) {
// TODO(crbug.com/1029348): Per spec, we should have a microtask
// checkpoint right after the update cycle. Once this is fixed we should
// no longer need to force a synchronous resolution here.
AsyncFinishMicrotask();
finished_ = true;
}
}
Expand Down
Expand Up @@ -49,6 +49,7 @@
#include "third_party/blink/renderer/core/page/page_animator.h"
#include "third_party/blink/renderer/core/style/computed_style.h"
#include "third_party/blink/renderer/platform/bindings/microtask.h"
#include "third_party/blink/renderer/platform/heap/persistent.h"

namespace blink {

Expand Down Expand Up @@ -260,10 +261,13 @@ void DocumentAnimations::RemoveReplacedAnimations(

// The list of animations for removal is constructed in reverse composite
// ordering for efficiency. Flip the ordering to ensure that events are
// dispatched in composite order.
// dispatched in composite order. Queue as a microtask so that the finished
// event is dispatched ahead of the remove event.
for (auto it = animations_to_remove.rbegin();
it != animations_to_remove.rend(); it++) {
(*it)->RemoveReplacedAnimation();
Animation* animation = *it;
Microtask::EnqueueMicrotask(WTF::Bind(&Animation::RemoveReplacedAnimation,
WrapWeakPersistent(animation)));
}
}

Expand Down

0 comments on commit ae61862

Please sign in to comment.