Clear cached data when we change the composite mode

Previously we would change the composite member, but wouldn't actually invalidate any of the (many) layers of caching we have. This meant that the changed composite mode wouldn't actually take effect. Bug: 1005915 Change-Id: I684ccb5011736edbc8b38cba5d2174095ccff9a6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815608 Reviewed-by: Kevin Ellis <kevers@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#699848}
FairyWorld · Sep 25, 2019 · c004564 · c004564
1 parent 2f4b496
commit c004564
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 6 deletions.
diff --git a/third_party/blink/renderer/core/animation/keyframe_effect.cc b/third_party/blink/renderer/core/animation/keyframe_effect.cc
@@ -140,6 +140,9 @@ String KeyframeEffect::composite() const {
 void KeyframeEffect::setComposite(String composite_string) {
   Model()->SetComposite(
       EffectModel::StringToCompositeOperation(composite_string).value());
+
+  ClearEffects();
+  InvalidateAndNotifyOwner();
 }
 
 HeapVector<ScriptValue> KeyframeEffect::getKeyframes(

diff --git a/third_party/blink/renderer/core/animation/keyframe_effect_model.cc b/third_party/blink/renderer/core/animation/keyframe_effect_model.cc
@@ -62,18 +62,20 @@ template <class K>
 void KeyframeEffectModelBase::SetFrames(HeapVector<K>& keyframes) {
   // TODO(samli): Should also notify/invalidate the animation
   keyframes_.clear();
-  keyframe_groups_ = nullptr;
-  interpolation_effect_->Clear();
-  last_fraction_ = std::numeric_limits<double>::quiet_NaN();
   keyframes_.AppendVector(keyframes);
-  needs_compositor_keyframes_snapshot_ = true;
+  ClearCachedData();
 }
 
 template CORE_EXPORT void KeyframeEffectModelBase::SetFrames(
     HeapVector<Member<Keyframe>>& keyframes);
 template CORE_EXPORT void KeyframeEffectModelBase::SetFrames(
     HeapVector<Member<StringKeyframe>>& keyframes);
 
+void KeyframeEffectModelBase::SetComposite(CompositeOperation composite) {
+  composite_ = composite;
+  ClearCachedData();
+}
+
 bool KeyframeEffectModelBase::Sample(
     int iteration,
     double fraction,
@@ -391,6 +393,13 @@ void KeyframeEffectModelBase::EnsureInterpolationEffectPopulated() const {
   interpolation_effect_->SetPopulated();
 }
 
+void KeyframeEffectModelBase::ClearCachedData() {
+  keyframe_groups_ = nullptr;
+  interpolation_effect_->Clear();
+  last_fraction_ = std::numeric_limits<double>::quiet_NaN();
+  needs_compositor_keyframes_snapshot_ = true;
+}
+
 bool KeyframeEffectModelBase::IsReplaceOnly() const {
   EnsureKeyframeGroups();
   for (const auto& entry : *keyframe_groups_) {

diff --git a/third_party/blink/renderer/core/animation/keyframe_effect_model.h b/third_party/blink/renderer/core/animation/keyframe_effect_model.h
@@ -91,7 +91,7 @@ class CORE_EXPORT KeyframeEffectModelBase : public EffectModel {
   void SetFrames(HeapVector<K>& keyframes);
 
   CompositeOperation Composite() const { return composite_; }
-  void SetComposite(CompositeOperation composite) { composite_ = composite; }
+  void SetComposite(CompositeOperation composite);
 
   const PropertySpecificKeyframeVector* GetPropertySpecificKeyframes(
       const PropertyHandle& property) const {
@@ -170,6 +170,9 @@ class CORE_EXPORT KeyframeEffectModelBase : public EffectModel {
   void EnsureKeyframeGroups() const;
   void EnsureInterpolationEffectPopulated() const;
 
+  // Clears the various bits of cached data that this class has.
+  void ClearCachedData();
+
   using ShouldSnapshotPropertyCallback =
       std::function<bool(const PropertyHandle&)>;
   using ShouldSnapshotKeyframeCallback =

diff --git a/...web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt b/...web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt
@@ -4,7 +4,7 @@ PASS Filling effect values reflect changes to font-size on parent element
 PASS Filling effect values reflect changes to variables on element
 PASS Filling effect values reflect changes to variables on parent element
 PASS Filling effect values reflect changes to the the animation's keyframes
-FAIL Filling effect values reflect changes to the the animation's composite mode assert_equals: Effect value after updating the composite mode expected "300px" but got "200px"
+PASS Filling effect values reflect changes to the the animation's composite mode
 FAIL Filling effect values reflect changes to the the animation's iteration composite mode assert_equals: Effect value after updating the iteration composite mode expected "200px" but got "100px"
 PASS Filling effect values reflect changes to the base value when using additive animation
 PASS Filling effect values reflect changes to the base value when using additive animation on a single keyframe