Bug 1253476 - Implement Animation.commitStyles; r=boris,emilio,bzbarsky,smaug

Differential Revision: https://phabricator.services.mozilla.com/D30327

--HG--
extra : moz-landing-system : lando

dom/animation/Animation.cpp

@@ -5,7 +5,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "Animation.h"
+
 #include "AnimationUtils.h"
+#include "mozAutoDocUpdate.h"
 #include "mozilla/dom/AnimationBinding.h"
 #include "mozilla/dom/AnimationPlaybackEvent.h"
 #include "mozilla/dom/Document.h"
@@ -14,10 +16,13 @@
 #include "mozilla/AnimationEventDispatcher.h"
 #include "mozilla/AnimationTarget.h"
 #include "mozilla/AutoRestore.h"
-#include "mozilla/Maybe.h"          // For Maybe
-#include "mozilla/TypeTraits.h"     // For std::forward<>
-#include "nsAnimationManager.h"     // For CSSAnimation
-#include "nsDOMMutationObserver.h"  // For nsAutoAnimationMutationBatch
+#include "mozilla/DeclarationBlock.h"
+#include "mozilla/Maybe.h"       // For Maybe
+#include "mozilla/TypeTraits.h"  // For std::forward<>
+#include "nsAnimationManager.h"  // For CSSAnimation
+#include "nsComputedDOMStyle.h"
+#include "nsDOMMutationObserver.h"    // For nsAutoAnimationMutationBatch
+#include "nsDOMCSSAttrDeclaration.h"  // For nsDOMCSSAttributeDeclaration
 #include "nsThreadUtils.h"  // For nsRunnableMethod and nsRevocableEventPtr
 #include "nsTransitionManager.h"      // For CSSTransition
 #include "PendingAnimationTracker.h"  // For PendingAnimationTracker
@@ -613,6 +618,103 @@ void Animation::Persist() {
   }
 }
 
+// https://drafts.csswg.org/web-animations/#dom-animation-commitstyles
+void Animation::CommitStyles(ErrorResult& aRv) {
+  if (!mEffect) {
+    return;
+  }
+
+  // Take an owning reference to the keyframe effect. This will ensure that
+  // this Animation and the target element remain alive after flushing style.
+  RefPtr<KeyframeEffect> keyframeEffect = mEffect->AsKeyframeEffect();
+  if (!keyframeEffect) {
+    return;
+  }
+
+  Maybe<NonOwningAnimationTarget> target = keyframeEffect->GetTarget();
+  if (!target) {
+    return;
+  }
+
+  if (target->mPseudoType != PseudoStyleType::NotPseudo) {
+    aRv.Throw(NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR);
+    return;
+  }
+
+  // Check it is an element with a style attribute
+  nsCOMPtr<nsStyledElement> styledElement = do_QueryInterface(target->mElement);
+  if (!styledElement) {
+    aRv.Throw(NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR);
+    return;
+  }
+
+  // Flush style before checking if the target element is rendered since the
+  // result could depend on pending style changes.
+  if (Document* doc = target->mElement->GetComposedDoc()) {
+    doc->FlushPendingNotifications(FlushType::Style);
+  }
+  if (!target->mElement->IsRendered()) {
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return;
+  }
+
+  nsPresContext* presContext =
+      nsContentUtils::GetContextForContent(target->mElement);
+  if (!presContext) {
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return;
+  }
+
+  // Get the computed animation values
+  UniquePtr<RawServoAnimationValueMap> animationValues =
+      Servo_AnimationValueMap_Create().Consume();
+  if (!presContext->EffectCompositor()->ComposeServoAnimationRuleForEffect(
+          *keyframeEffect, CascadeLevel(), animationValues.get())) {
+    NS_WARNING("Failed to compose animation style to commit");
+    return;
+  }
+
+  // Calling SetCSSDeclaration will trigger attribute setting code.
+  // Start the update now so that the old rule doesn't get used
+  // between when we mutate the declaration and when we set the new
+  // rule.
+  mozAutoDocUpdate autoUpdate(target->mElement->OwnerDoc(), true);
+
+  // Get the inline style to append to
+  RefPtr<DeclarationBlock> declarationBlock;
+  if (auto* existing = target->mElement->GetInlineStyleDeclaration()) {
+    declarationBlock = existing->EnsureMutable();
+  } else {
+    declarationBlock = new DeclarationBlock();
+    declarationBlock->SetDirty();
+  }
+
+  // Set the animated styles
+  bool changed = false;
+  nsCSSPropertyIDSet properties = keyframeEffect->GetPropertySet();
+  for (nsCSSPropertyID property : properties) {
+    RefPtr<RawServoAnimationValue> computedValue =
+        Servo_AnimationValueMap_GetValue(animationValues.get(), property)
+            .Consume();
+    if (computedValue) {
+      changed |= Servo_DeclarationBlock_SetPropertyToAnimationValue(
+          declarationBlock->Raw(), computedValue);
+    }
+  }
+
+  if (!changed) {
+    return;
+  }
+
+  // Update inline style declaration
+  MutationClosureData closureData;
+  closureData.mClosure = nsDOMCSSAttributeDeclaration::MutationClosureFunction;
+  closureData.mElement = target->mElement;
+
+  target->mElement->InlineStyleDeclarationWillChange(closureData);
+  target->mElement->SetInlineStyleDeclaration(*declarationBlock, closureData);
+}
+
 // ---------------------------------------------------------------------------
 //
 // JS wrappers for Animation interface:

dom/animation/Animation.h

@@ -142,6 +142,7 @@ class Animation : public DOMEventTargetHelper,
   void Reverse(ErrorResult& aRv);
 
   void Persist();
+  void CommitStyles(ErrorResult& aRv);
 
   bool IsRunningOnCompositor() const;
 

dom/animation/EffectCompositor.cpp

@@ -394,6 +394,26 @@ class EffectCompositeOrderComparator {
 };
 }  // namespace
 
+static void ComposeSortedEffects(
+    const nsTArray<KeyframeEffect*>& aSortedEffects,
+    const EffectSet* aEffectSet, EffectCompositor::CascadeLevel aCascadeLevel,
+    RawServoAnimationValueMap* aAnimationValues) {
+  // If multiple animations affect the same property, animations with higher
+  // composite order (priority) override or add to animations with lower
+  // priority.
+  nsCSSPropertyIDSet propertiesToSkip;
+  if (aEffectSet) {
+    propertiesToSkip =
+        aCascadeLevel == EffectCompositor::CascadeLevel::Animations
+            ? aEffectSet->PropertiesForAnimationsLevel().Inverse()
+            : aEffectSet->PropertiesForAnimationsLevel();
+  }
+
+  for (KeyframeEffect* effect : aSortedEffects) {
+    effect->GetAnimation()->ComposeStyle(*aAnimationValues, propertiesToSkip);
+  }
+}
+
 bool EffectCompositor::GetServoAnimationRule(
     const dom::Element* aElement, PseudoStyleType aPseudoType,
     CascadeLevel aCascadeLevel, RawServoAnimationValueMap* aAnimationValues) {
@@ -417,16 +437,8 @@ bool EffectCompositor::GetServoAnimationRule(
   }
   sortedEffectList.Sort(EffectCompositeOrderComparator());
 
-  // If multiple animations affect the same property, animations with higher
-  // composite order (priority) override or add or animations with lower
-  // priority.
-  const nsCSSPropertyIDSet propertiesToSkip =
-      aCascadeLevel == CascadeLevel::Animations
-          ? effectSet->PropertiesForAnimationsLevel().Inverse()
-          : effectSet->PropertiesForAnimationsLevel();
-  for (KeyframeEffect* effect : sortedEffectList) {
-    effect->GetAnimation()->ComposeStyle(*aAnimationValues, propertiesToSkip);
-  }
+  ComposeSortedEffects(sortedEffectList, effectSet, aCascadeLevel,
+                       aAnimationValues);
 
   MOZ_ASSERT(effectSet == EffectSet::GetEffectSet(aElement, aPseudoType),
              "EffectSet should not change while composing style");
@@ -434,6 +446,59 @@ bool EffectCompositor::GetServoAnimationRule(
   return true;
 }
 
+bool EffectCompositor::ComposeServoAnimationRuleForEffect(
+    KeyframeEffect& aEffect, CascadeLevel aCascadeLevel,
+    RawServoAnimationValueMap* aAnimationValues) {
+  MOZ_ASSERT(aAnimationValues);
+  MOZ_ASSERT(mPresContext && mPresContext->IsDynamic(),
+             "Should not be in print preview");
+
+  Maybe<NonOwningAnimationTarget> target = aEffect.GetTarget();
+  if (!target) {
+    return false;
+  }
+
+  // Don't try to compose animations for elements in documents without a pres
+  // shell (e.g. XMLHttpRequest documents).
+  if (!nsContentUtils::GetPresShellForContent(target->mElement)) {
+    return false;
+  }
+
+  // GetServoAnimationRule is called as part of the regular style resolution
+  // where the cascade results are updated in the pre-traversal as needed.
+  // This function, however, is only called when committing styles so we
+  // need to ensure the cascade results are up-to-date manually.
+  EffectCompositor::MaybeUpdateCascadeResults(target->mElement,
+                                              target->mPseudoType);
+
+  EffectSet* effectSet =
+      EffectSet::GetEffectSet(target->mElement, target->mPseudoType);
+
+  // Get a list of effects sorted by composite order up to and including
+  // |aEffect|, even if it is not in the EffectSet.
+  auto comparator = EffectCompositeOrderComparator();
+  nsTArray<KeyframeEffect*> sortedEffectList(effectSet ? effectSet->Count() + 1
+                                                       : 1);
+  if (effectSet) {
+    for (KeyframeEffect* effect : *effectSet) {
+      if (comparator.LessThan(effect, &aEffect)) {
+        sortedEffectList.AppendElement(effect);
+      }
+    }
+    sortedEffectList.Sort(comparator);
+  }
+  sortedEffectList.AppendElement(&aEffect);
+
+  ComposeSortedEffects(sortedEffectList, effectSet, aCascadeLevel,
+                       aAnimationValues);
+
+  MOZ_ASSERT(effectSet ==
+                 EffectSet::GetEffectSet(target->mElement, target->mPseudoType),
+             "EffectSet should not change while composing style");
+
+  return true;
+}
+
 /* static */ dom::Element* EffectCompositor::GetElementToRestyle(
     dom::Element* aElement, PseudoStyleType aPseudoType) {
   if (aPseudoType == PseudoStyleType::NotPseudo) {

dom/animation/EffectCompositor.h

@@ -40,6 +40,7 @@ struct NonOwningAnimationTarget;
 namespace dom {
 class Animation;
 class Element;
+class KeyframeEffect;
 }  // namespace dom
 
 class EffectCompositor {
@@ -118,8 +119,9 @@ class EffectCompositor {
                               dom::Element* aElement,
                               PseudoStyleType aPseudoType);
 
-  // Get animation rule for stylo. This is an equivalent of GetAnimationRule
-  // and will be called from servo side.
+  // Get the animation rule for the appropriate level of the cascade for
+  // a (pseudo-)element. Called from the Servo side.
+  //
   // The animation rule is stored in |RawServoAnimationValueMap|.
   // We need to be careful while doing any modification because it may cause
   // some thread-safe issues.
@@ -128,6 +130,15 @@ class EffectCompositor {
                              CascadeLevel aCascadeLevel,
                              RawServoAnimationValueMap* aAnimationValues);
 
+  // A variant on GetServoAnimationRule that composes all the effects for an
+  // element up to and including |aEffect|.
+  //
+  // Note that |aEffect| might not be in the EffectSet since we can use this for
+  // committing the computed style of a removed Animation.
+  bool ComposeServoAnimationRuleForEffect(
+      dom::KeyframeEffect& aEffect, CascadeLevel aCascadeLevel,
+      RawServoAnimationValueMap* aAnimationValues);
+
   bool HasPendingStyleUpdates() const;
 
   static bool HasAnimationsForCompositor(const nsIFrame* aFrame,

dom/animation/KeyframeEffect.cpp

@@ -579,13 +579,10 @@ void KeyframeEffect::ComposeStyle(RawServoAnimationValueMap& aComposeResult,
   if (HasPropertiesThatMightAffectOverflow()) {
     nsPresContext* presContext =
         nsContentUtils::GetContextForContent(mTarget->mElement);
-    if (presContext) {
+    EffectSet* effectSet =
+        EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType);
+    if (presContext && effectSet) {
       TimeStamp now = presContext->RefreshDriver()->MostRecentRefresh();
-      EffectSet* effectSet =
-          EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType);
-      MOZ_ASSERT(effectSet,
-                 "ComposeStyle should only be called on an effect "
-                 "that is part of an effect set");
       effectSet->UpdateLastOverflowAnimationSyncTime(now);
     }
   }

dom/webidl/Animation.webidl

@@ -54,6 +54,8 @@ interface Animation : EventTarget {
   void reverse ();
   [Pref="dom.animations-api.autoremove.enabled"]
   void persist ();
+  [Pref="dom.animations-api.autoremove.enabled", Throws]
+  void commitStyles ();
 };
 
 // Non-standard extensions

layout/style/ServoBindingTypes.h

@@ -173,8 +173,6 @@ SERVO_ARC_TYPE(ComputedStyle, mozilla::ComputedStyle)
 
 // Other special cases.
 
-// TODO(heycam): Handle these elsewhere.
 struct RawServoAnimationValueTable;
-struct RawServoAnimationValueMap;
 
 #endif  // mozilla_ServoBindingTypes_h

layout/style/ServoBoxedTypeList.h

@@ -24,6 +24,7 @@
 // to just generate the forward declaration.
 
 SERVO_BOXED_TYPE(StyleSet, RawServoStyleSet)
+SERVO_BOXED_TYPE(AnimationValueMap, RawServoAnimationValueMap)
 SERVO_BOXED_TYPE(AuthorStyles, RawServoAuthorStyles)
 SERVO_BOXED_TYPE(SelectorList, RawServoSelectorList)
 SERVO_BOXED_TYPE(SharedMemoryBuilder, RawServoSharedMemoryBuilder)

servo/components/style/gecko/boxed_types.rs

@@ -5,12 +5,22 @@
 //! FFI implementations for types listed in ServoBoxedTypeList.h.
 
 use crate::gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI};
+use crate::properties::animated_properties::AnimationValueMap;
 use to_shmem::SharedMemoryBuilder;
 
 // TODO(heycam): The FFI impls for most of the types in ServoBoxedTypeList.h are spread across
 // various files at the moment, but should probably all move here, and use macros to define
 // them more succinctly, like we do in arc_types.rs.
 
+#[cfg(feature = "gecko")]
+unsafe impl HasFFI for AnimationValueMap {
+    type FFIType = crate::gecko_bindings::bindings::RawServoAnimationValueMap;
+}
+#[cfg(feature = "gecko")]
+unsafe impl HasSimpleFFI for AnimationValueMap {}
+#[cfg(feature = "gecko")]
+unsafe impl HasBoxFFI for AnimationValueMap {}
+
 #[cfg(feature = "gecko")]
 unsafe impl HasFFI for SharedMemoryBuilder {
     type FFIType = crate::gecko_bindings::bindings::RawServoSharedMemoryBuilder;

servo/components/style/properties/helpers/animated_properties.mako.rs

@@ -9,9 +9,7 @@
     from itertools import groupby
 %>
 
-#[cfg(feature = "gecko")] use crate::gecko_bindings::structs::RawServoAnimationValueMap;
 #[cfg(feature = "gecko")] use crate::gecko_bindings::structs::nsCSSPropertyID;
-#[cfg(feature = "gecko")] use crate::gecko_bindings::sugar::ownership::{HasFFI, HasSimpleFFI};
 use itertools::{EitherOrBoth, Itertools};
 use crate::properties::{CSSWideKeyword, PropertyDeclaration};
 use crate::properties::longhands;
@@ -190,13 +188,6 @@ impl AnimatedProperty {
 /// composed for each TransitionProperty.
 pub type AnimationValueMap = FxHashMap<LonghandId, AnimationValue>;
 
-#[cfg(feature = "gecko")]
-unsafe impl HasFFI for AnimationValueMap {
-    type FFIType = RawServoAnimationValueMap;
-}
-#[cfg(feature = "gecko")]
-unsafe impl HasSimpleFFI for AnimationValueMap {}
-
 /// An enum to represent a single computed value belonging to an animated
 /// property in order to be interpolated with another one. When interpolating,
 /// both values need to belong to the same property.

servo/ports/geckolib/glue.rs

@@ -101,7 +101,7 @@ use style::global_style_data::{GlobalStyleData, GLOBAL_STYLE_DATA, STYLE_THREAD_
 use style::invalidation::element::restyle_hints::RestyleHint;
 use style::media_queries::MediaList;
 use style::parser::{self, Parse, ParserContext};
-use style::properties::animated_properties::AnimationValue;
+use style::properties::animated_properties::{AnimationValue, AnimationValueMap};
 use style::properties::{parse_one_declaration_into, parse_style_attribute};
 use style::properties::{ComputedValues, Importance, NonCustomPropertyId};
 use style::properties::{LonghandId, LonghandIdSet, PropertyDeclarationBlock, PropertyId};
@@ -913,6 +913,36 @@ fn resolve_rules_for_element_with_context<'a>(
         .0
 }
 
+#[no_mangle]
+pub extern "C" fn Servo_AnimationValueMap_Create() -> Owned<structs::RawServoAnimationValueMap> {
+    Box::<AnimationValueMap>::default().into_ffi()
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn Servo_AnimationValueMap_Drop(value_map: *mut structs::RawServoAnimationValueMap) {
+    AnimationValueMap::drop_ffi(value_map)
+}
+
+#[no_mangle]
+pub extern "C" fn Servo_AnimationValueMap_GetValue(
+    raw_value_map: &mut structs::RawServoAnimationValueMap,
+    property_id: nsCSSPropertyID,
+) -> Strong<RawServoAnimationValue> {
+    use style::properties::animated_properties::AnimationValueMap;
+
+    let property = match LonghandId::from_nscsspropertyid(property_id) {
+        Ok(longhand) => longhand,
+        Err(()) => return Strong::null(),
+    };
+    let value_map = AnimationValueMap::from_ffi_mut(raw_value_map);
+
+    value_map
+        .get(&property)
+        .map_or(Strong::null(), |value| {
+            Arc::new(value.clone()).into_strong()
+        })
+}
+
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_GetBaseComputedValuesForElement(
     raw_style_set: &RawServoStyleSet,

testing/web-platform/meta/web-animations/interfaces/Animation/commitStyles.html.ini

@@ -0,0 +1,3 @@
+[commitStyles.html]
+  [Does NOT trigger mutation observers when the change to style is redundant]
+    expected: FAIL

testing/web-platform/tests/web-animations/interfaces/Animation/commitStyles.html

@@ -0,0 +1,389 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>Animation.commitStyles</title>
+<link rel="help" href="https://drafts.csswg.org/web-animations/#dom-animation-commitstyles">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../testcommon.js"></script>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+function assert_numeric_style_equals(opacity, expected, description) {
+  return assert_approx_equals(
+    parseFloat(opacity),
+    expected,
+    0.0001,
+    description
+  );
+}
+
+test(t => {
+  const div = createDiv(t);
+  div.style.opacity = '0.1';
+
+  const animation = div.animate(
+    { opacity: 0.2 },
+    { duration: 1, fill: 'forwards' }
+  );
+  animation.finish();
+
+  animation.commitStyles();
+
+  // Cancel the animation so we can inspect the underlying style
+  animation.cancel();
+
+  assert_numeric_style_equals(getComputedStyle(div).opacity, 0.2);
+}, 'Commits styles');
+
+promise_test(async t => {
+  const div = createDiv(t);
+  div.style.opacity = '0.1';
+
+  const animA = div.animate(
+    { opacity: 0.2 },
+    { duration: 1, fill: 'forwards' }
+  );
+  const animB = div.animate(
+    { opacity: 0.3 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  await animA.finished;
+
+  animB.cancel();
+
+  animA.commitStyles();
+
+  assert_numeric_style_equals(getComputedStyle(div).opacity, 0.2);
+}, 'Commits styles for an animation that has been removed');
+
+test(t => {
+  const div = createDiv(t);
+  div.style.margin = '10px';
+
+  const animation = div.animate(
+    { margin: '20px' },
+    { duration: 1, fill: 'forwards' }
+  );
+  animation.finish();
+
+  animation.commitStyles();
+
+  animation.cancel();
+
+  assert_equals(div.style.marginLeft, '20px');
+}, 'Commits shorthand styles');
+
+test(t => {
+  const div = createDiv(t);
+  div.style.marginLeft = '10px';
+
+  const animation = div.animate(
+    { marginInlineStart: '20px' },
+    { duration: 1, fill: 'forwards' }
+  );
+  animation.finish();
+
+  animation.commitStyles();
+
+  animation.cancel();
+
+  assert_equals(div.style.marginLeft, '20px');
+}, 'Commits logical properties');
+
+test(t => {
+  const div = createDiv(t);
+  div.style.marginLeft = '10px';
+
+  const animation = div.animate({ opacity: [0.2, 0.7] }, 1000);
+  animation.currentTime = 500;
+  animation.commitStyles();
+  animation.cancel();
+
+  assert_numeric_style_equals(getComputedStyle(div).opacity, 0.45);
+}, 'Commits values calculated mid-interval');
+
+test(t => {
+  const div = createDiv(t);
+  div.style.setProperty('--target', '0.5');
+
+  const animation = div.animate(
+    { opacity: 'var(--target)' },
+    { duration: 1, fill: 'forwards' }
+  );
+  animation.finish();
+  animation.commitStyles();
+  animation.cancel();
+
+  assert_numeric_style_equals(getComputedStyle(div).opacity, 0.5);
+
+  // Changes to the variable should have no effect
+  div.style.setProperty('--target', '1');
+
+  assert_numeric_style_equals(getComputedStyle(div).opacity, 0.5);
+}, 'Commits variables as their computed values');
+
+test(t => {
+  const div = createDiv(t);
+  div.style.fontSize = '10px';
+
+  const animation = div.animate(
+    { width: '10em' },
+    { duration: 1, fill: 'forwards' }
+  );
+  animation.finish();
+  animation.commitStyles();
+  animation.cancel();
+
+  assert_numeric_style_equals(getComputedStyle(div).width, 100);
+
+  // Changes to the font-size should have no effect
+  div.style.fontSize = '20px';
+
+  assert_numeric_style_equals(getComputedStyle(div).width, 100);
+}, 'Commits em units as pixel values');
+
+promise_test(async t => {
+  const div = createDiv(t);
+  div.style.opacity = '0.1';
+
+  const animA = div.animate(
+    { opacity: '0.2' },
+    { duration: 1, fill: 'forwards' }
+  );
+  const animB = div.animate(
+    { opacity: '0.2', composite: 'add' },
+    { duration: 1, fill: 'forwards' }
+  );
+  const animC = div.animate(
+    { opacity: '0.3', composite: 'add' },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  animA.persist();
+  animB.persist();
+
+  await animB.finished;
+
+  // The values above have been chosen such that various error conditions
+  // produce results that all differ from the desired result:
+  //
+  //  Expected result:
+  //
+  //    animA + animB = 0.4
+  //
+  //  Likely error results:
+  //
+  //    <underlying> = 0.1
+  //    (Commit didn't work at all)
+  //
+  //    animB = 0.2
+  //    (Didn't add at all when resolving)
+  //
+  //    <underlying> + animB = 0.3
+  //    (Added to the underlying value instead of lower-priority animations when
+  //    resolving)
+  //
+  //    <underlying> + animA + animB = 0.5
+  //    (Didn't respect the composite mode of lower-priority animations)
+  //
+  //    animA + animB + animC = 0.7
+  //    (Resolved the whole stack, not just up to the target effect)
+  //
+
+  animB.commitStyles();
+
+  animA.cancel();
+  animB.cancel();
+  animC.cancel();
+
+  assert_numeric_style_equals(getComputedStyle(div).opacity, 0.4);
+}, 'Commits the intermediate value of an animation in the middle of stack');
+
+promise_test(async t => {
+  const div = createDiv(t);
+  div.style.opacity = '0.1';
+
+  // Setup animation
+  const animation = div.animate(
+    { opacity: 0.2 },
+    { duration: 1, fill: 'forwards' }
+  );
+  animation.finish();
+
+  // Setup observer
+  const mutationRecords = [];
+  const observer = new MutationObserver(mutations => {
+    mutationRecords.push(...mutations);
+  });
+  observer.observe(div, { attributes: true, attributeOldValue: true });
+
+  animation.commitStyles();
+
+  // Wait for mutation records to be dispatched
+  await Promise.resolve();
+
+  assert_equals(mutationRecords.length, 1, 'Should have one mutation record');
+
+  const mutation = mutationRecords[0];
+  assert_equals(mutation.type, 'attributes');
+  assert_equals(mutation.oldValue, 'opacity: 0.1;');
+
+  observer.disconnect();
+}, 'Triggers mutation observers when updating style');
+
+promise_test(async t => {
+  const div = createDiv(t);
+  div.style.opacity = '0.2';
+
+  // Setup animation
+  const animation = div.animate(
+    { opacity: 0.2 },
+    { duration: 1, fill: 'forwards' }
+  );
+  animation.finish();
+
+  // Setup observer
+  const mutationRecords = [];
+  const observer = new MutationObserver(mutations => {
+    mutationRecords.push(...mutations);
+  });
+  observer.observe(div, { attributes: true });
+
+  animation.commitStyles();
+
+  // Wait for mutation records to be dispatched
+  await Promise.resolve();
+
+  assert_equals(mutationRecords.length, 0, 'Should have no mutation records');
+
+  observer.disconnect();
+}, 'Does NOT trigger mutation observers when the change to style is redundant');
+
+test(t => {
+  const pseudo = getPseudoElement(t, 'before');
+  const animation = pseudo.animate(
+    { opacity: 0 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  assert_throws('NoModificationAllowedError', () => {
+    animation.commitStyles();
+  });
+}, 'Throws if the target element is a pseudo element');
+
+test(t => {
+  const animation = createDiv(t).animate(
+    { opacity: 0 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  const nonStyleElement
+    = document.createElementNS('http://example.org/test', 'test');
+  document.body.appendChild(nonStyleElement);
+  animation.effect.target = nonStyleElement;
+
+  assert_throws('NoModificationAllowedError', () => {
+    animation.commitStyles();
+  });
+
+  nonStyleElement.remove();
+}, 'Throws if the target element is not something with a style attribute');
+
+test(t => {
+  const div = createDiv(t);
+  const animation = div.animate(
+    { opacity: 0 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  div.style.display = 'none';
+
+  assert_throws('InvalidStateError', () => {
+    animation.commitStyles();
+  });
+}, 'Throws if the target effect is display:none');
+
+test(t => {
+  const container = createDiv(t);
+  const div = createDiv(t);
+  container.append(div);
+
+  const animation = div.animate(
+    { opacity: 0 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  container.style.display = 'none';
+
+  assert_throws('InvalidStateError', () => {
+    animation.commitStyles();
+  });
+}, "Throws if the target effect's ancestor is display:none");
+
+test(t => {
+  const container = createDiv(t);
+  const div = createDiv(t);
+  container.append(div);
+
+  const animation = div.animate(
+    { opacity: 0 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  container.style.display = 'contents';
+
+  // Should NOT throw
+  animation.commitStyles();
+}, 'Treats display:contents as rendered');
+
+test(t => {
+  const container = createDiv(t);
+  const div = createDiv(t);
+  container.append(div);
+
+  const animation = div.animate(
+    { opacity: 0 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  div.style.display = 'contents';
+  container.style.display = 'none';
+
+  assert_throws('InvalidStateError', () => {
+    animation.commitStyles();
+  });
+}, 'Treats display:contents in a display:none subtree as not rendered');
+
+test(t => {
+  const div = createDiv(t);
+  const animation = div.animate(
+    { opacity: 0 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  div.remove();
+
+  assert_throws('InvalidStateError', () => {
+    animation.commitStyles();
+  });
+}, 'Throws if the target effect is disconnected');
+
+test(t => {
+  const pseudo = getPseudoElement(t, 'before');
+  const animation = pseudo.animate(
+    { opacity: 0 },
+    { duration: 1, fill: 'forwards' }
+  );
+
+  pseudo.element.remove();
+
+  assert_throws('NoModificationAllowedError', () => {
+    animation.commitStyles();
+  });
+}, 'Checks the pseudo element condition before the not rendered condition');
+
+</script>
+</body>

testing/web-platform/tests/web-animations/interfaces/Animation/style-change-events.html

@@ -11,8 +11,8 @@
 <script>
 'use strict';
 
-// Test that each property defined in the Animation interface does not produce
-// style change events.
+// Test that each property defined in the Animation interface behaves as
+// expected with regards to whether or not it produces style change events.
 //
 // There are two types of tests:
 //
@@ -29,8 +29,9 @@
 //     (b) An object with the following format:
 //
 //         {
-//            setup: elem => { /* return Animation */ }
-//            test: animation => { /* play |animation| */ }
+//            setup: elem => { /* return Animation */ },
+//            test: animation => { /* play |animation| */ },
+//            shouldFlush: boolean /* optional, defaults to false */
 //         }
 //
 //     If the latter form is used, the setup function should return an Animation
@@ -56,15 +57,17 @@
 //    animation, but simply needs to get/set the property under test.
 
 const PlayAnimationTest = testFuncOrObj => {
-  let test, setup;
+  let test, setup, shouldFlush;
 
   if (typeof testFuncOrObj === 'function') {
     test = testFuncOrObj;
+    shouldFlush = false;
   } else {
     test = testFuncOrObj.test;
     if (typeof testFuncOrObj.setup === 'function') {
       setup = testFuncOrObj.setup;
     }
+    shouldFlush = !!testFuncOrObj.shouldFlush;
   }
 
   if (!setup) {
@@ -74,11 +77,11 @@ const PlayAnimationTest = testFuncOrObj => {
       );
   }
 
-  return { test, setup };
+  return { test, setup, shouldFlush };
 };
 
 const UsePropertyTest = testFuncOrObj => {
-  const { setup, test } = PlayAnimationTest(testFuncOrObj);
+  const { setup, test, shouldFlush } = PlayAnimationTest(testFuncOrObj);
 
   let coveringAnimation;
   return {
@@ -93,6 +96,7 @@ const UsePropertyTest = testFuncOrObj => {
       test(animation);
       coveringAnimation.play();
     },
+    shouldFlush,
   };
 };
 
@@ -253,6 +257,31 @@ const tests = {
       animation.persist();
     },
   }),
+  commitStyles: PlayAnimationTest({
+    setup: async elem => {
+      // Create an animation whose replaceState is 'removed'.
+      const animA = elem.animate(
+        // It's important to use opacity of '1' here otherwise we'll create a
+        // transition due to updating the specified style whereas the transition
+        // we want to detect is the one from flushing due to calling
+        // commitStyles.
+        { opacity: 1 },
+        { duration: 1, fill: 'forwards' }
+      );
+      const animB = elem.animate(
+        { opacity: 1 },
+        { duration: 1, fill: 'forwards' }
+      );
+      await animA.finished;
+      animB.cancel();
+
+      return animA;
+    },
+    test: animation => {
+      animation.commitStyles();
+    },
+    shouldFlush: true,
+  }),
   get ['Animation constructor']() {
     let originalElem;
     return UsePropertyTest({
@@ -294,7 +323,7 @@ test(() => {
 for (const key of properties) {
   promise_test(async t => {
     assert_own_property(tests, key, `Should have a test for '${key}' property`);
-    const { setup, test } = tests[key];
+    const { setup, test, shouldFlush } = tests[key];
 
     // Setup target element
     const div = createDiv(t);
@@ -319,17 +348,24 @@ for (const key of properties) {
     // If the test function produced a style change event it will have triggered
     // a transition.
 
-    // Wait for the animation to start and then for at least one animation
-    // frame to give the transitionrun event a chance to be dispatched.
+    // Wait for the animation to start and then for at least two animation
+    // frames to give the transitionrun event a chance to be dispatched.
     assert_true(
       typeof animation.ready !== 'undefined',
       'Should have a valid animation to wait on'
     );
     await animation.ready;
-    await waitForAnimationFrames(1);
+    await waitForAnimationFrames(2);
 
-    assert_false(gotTransition, 'A transition should NOT have been triggered');
-  }, `Animation.${key} does NOT trigger a style change event`);
+    if (shouldFlush) {
+      assert_true(gotTransition, 'A transition should have been triggered');
+    } else {
+      assert_false(
+        gotTransition,
+        'A transition should NOT have been triggered'
+      );
+    }
+  }, `Animation.${key} produces expected style change events`);
 }
 </script>
 </body>