REGRESSION (262875@main / iOS 17): fill: 'both' not respected with an…

…imation https://bugs.webkit.org/show_bug.cgi?id=257861 rdar://111407099 Reviewed by Simon Fraser. While we fixed a similar issue with 265498@main, the fix was not complete. Indeed, to correctly deal with accelerated replaced effects in an effect stack, we must ensure two things: 1. in `KeyframeEffect::applyPendingAcceleratedActionsOrUpdateTimingProperties()`, where we ensure the accelerated effect is present on the backing GraphicsLayerCA in the right sort order, we not only need to add an `UpdateProperties` accelerated action but also ensure that we mark it as running by setting the `m_lastRecordedAcceleratedAction` member. 2. in `KeyframeEffectStack::applyPendingAcceleratedActions()`, where we update all effects in the stack to ensure any pending accelerated actions as applied, we must *not* force the running of replaced effects if all the accelerated effects in the stack are *not* actively running. In that case we must only call `KeyframeEffect::applyPendingAcceleratedActions` to ensure any previously-running accelerated effect has a chance to be stopped. * LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root.html: Added. * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::applyPendingAcceleratedActionsOrUpdateTimingProperties): * Source/WebCore/animation/KeyframeEffectStack.cpp: (WebCore::KeyframeEffectStack::applyPendingAcceleratedActions const): Canonical link: https://commits.webkit.org/265909@main
WebKit · Jul 10, 2023 · 6f928cc · 6f928cc
1 parent 4805993
commit 6f928cc
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 2 deletions.
diff --git a/...-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root-expected.html b/...-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root-expected.html
@@ -0,0 +1,9 @@
+<!doctype html>
+<style>
+div {
+  width: 100px;
+  height: 100px;
+  background: green;
+}
+</style>
+<div></div>
diff --git a/...ation-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root-ref.html b/...ation-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root-ref.html
@@ -0,0 +1,9 @@
+<!doctype html>
+<style>
+div {
+  width: 100px;
+  height: 100px;
+  background: green;
+}
+</style>
+<div></div>
diff --git a/...animation-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root.html b/...animation-model/keyframe-effects/effect-value-opacity-replaced-effect-in-shadow-root.html
@@ -0,0 +1,26 @@
+<!doctype html>
+<html class="reftest-wait">
+<head>
+<meta charset=utf-8>
+<title>The effect value of consecutive animations targeting 'opacity' in a shadow root</title>
+<link rel="help" href="https://drafts.csswg.org/web-animations/">
+<link rel="match" href="effect-value-opacity-replaced-effect-in-shadow-root-ref.html">
+</head>
+<body>
+<custom-element></custom-element>
+<script>
+'use strict';
+
+(async function () {
+  const customElement = document.querySelector('custom-element');
+  const shadowRoot = customElement.attachShadow({ mode: 'open' });
+  shadowRoot.innerHTML = `<div style="width: 100px; height: 100px; background-color: green"></div>`;
+  const target = shadowRoot.firstElementChild;
+
+  await target.animate({ opacity: [1, 0] }, { duration: 10, fill: 'both' }).finished;
+  await target.animate({ opacity: [0, 1] }, { duration: 10, fill: 'both' }).finished;
+  document.documentElement.classList.remove("reftest-wait");
+})();
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp
@@ -2013,6 +2013,7 @@ void KeyframeEffect::applyPendingAcceleratedActionsOrUpdateTimingProperties()
 
     if (m_pendingAcceleratedActions.isEmpty()) {
         m_pendingAcceleratedActions.append(AcceleratedAction::UpdateProperties);
+        m_lastRecordedAcceleratedAction = AcceleratedAction::Play;
         applyPendingAcceleratedActions();
         m_pendingAcceleratedActions.clear();
     } else

diff --git a/Source/WebCore/animation/KeyframeEffectStack.cpp b/Source/WebCore/animation/KeyframeEffectStack.cpp
@@ -277,8 +277,16 @@ void KeyframeEffectStack::cascadeDidOverrideProperties(const HashSet<AnimatableP
 
 void KeyframeEffectStack::applyPendingAcceleratedActions() const
 {
-    for (auto& effect : m_effects)
-        effect->applyPendingAcceleratedActionsOrUpdateTimingProperties();
+    bool hasActiveAcceleratedEffect = m_effects.containsIf([](const auto& effect) {
+        return effect->canBeAccelerated() && effect->animation()->playState() == WebAnimation::PlayState::Running;
+    });
+
+    for (auto& effect : m_effects) {
+        if (hasActiveAcceleratedEffect)
+            effect->applyPendingAcceleratedActionsOrUpdateTimingProperties();
+        else
+            effect->applyPendingAcceleratedActions();
+    }
 }
 
 } // namespace WebCore