Skip to content

Commit

Permalink
Cherry-pick 452ca17. rdar://119191813
Browse files Browse the repository at this point in the history
    [web-animations] effect targeting an element with `display: none` should not schedule immediate animation resolution (affects reddit.com)
    https://bugs.webkit.org/show_bug.cgi?id=265934
    rdar://119191813

    Reviewed by Simon Fraser and Antti Koivisto.

    Typically, effects that are in their active phase (ie. their current time is changing from frame
    to frame (see https://drafts.csswg.org/web-animations-1/#animation-effect-active-phase for details)
    will schedule immediate animation resolution. However there are exceptions, for instance we don't
    schedule immediate animation resolution if the effect does not affect styles or if the effect is
    running accelerated.

    We now also handle the case where an effect's target does not have a renderer, as would be the case
    if an element has a `display: none` style, making sure to also handle the `display: contents` case
    where a renderer is not created for the target element, but will for its children.

    This helps power usage on reddit.com which has JS-originated animations running infinitely that are
    targeting elements in shadow roots that eventually lose their renderer.

    * LayoutTests/webanimations/scheduling-of-animation-with-display-contents-expected.txt: Added.
    * LayoutTests/webanimations/scheduling-of-animation-with-display-contents.html: Added.
    * LayoutTests/webanimations/scheduling-of-animation-without-renderer-expected.txt: Added.
    * LayoutTests/webanimations/scheduling-of-animation-without-renderer.html: Added.
    * Source/WebCore/animation/KeyframeEffect.cpp:
    (WebCore::KeyframeEffect::ticksContinouslyWhileActive const):

    Canonical link: https://commits.webkit.org/271614@main

Identifier: 267815.620@safari-7617-branch
  • Loading branch information
Dan Robson committed Dec 7, 2023
1 parent 2ea6d77 commit 0386269
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

PASS Computing the time until the next tick for an effect targeting an element with 'display: contents'.

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<div style="display: contents"></div>
<script>

promise_test(async () => {
const animation = document.querySelector("div").animate({ color: "blue" }, 1000);
await animation.ready;

animation.currentTime = 100;
assert_equals(internals.timeToNextAnimationTick(animation), 0);
}, "Computing the time until the next tick for an effect targeting an element with 'display: contents'.");

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

PASS Computing the time until the next tick for an effect targeting an element losing its renderer.

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<div></div>
<script>

promise_test(async () => {
const target = document.querySelector("div");
const animation = document.querySelector("div").animate({ marginLeft: "100px" }, 1000);
await animation.ready;

animation.currentTime = 100;
assert_equals(internals.timeToNextAnimationTick(animation), 0, "Animation needs immediate resolution when running with a renderer.");

target.style.display = "none";
assert_equals(getComputedStyle(target).display, "none");
assert_equals(internals.timeToNextAnimationTick(animation), 900, "Animation will not need to be resolved until complete when running without a renderer.");
}, "Computing the time until the next tick for an effect targeting an element losing its renderer.");

</script>
4 changes: 4 additions & 0 deletions Source/WebCore/animation/KeyframeEffect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2414,6 +2414,10 @@ bool KeyframeEffect::ticksContinouslyWhileActive() const
if (doesNotAffectStyles)
return false;

auto targetHasDisplayContents = [&]() { return m_target && m_pseudoId == PseudoId::None && m_target->hasDisplayContents(); };
if (!renderer() && !targetHasDisplayContents())
return false;

if (isCompletelyAccelerated() && isRunningAccelerated())
return false;

Expand Down

0 comments on commit 0386269

Please sign in to comment.