Implement KeyframeEffect::setTarget

This CL implements the writable part of KeyframeEffect's target member.
This brings us into line with the spec, except that Chromium still don't
support CSSPseudoElement and so we only allow setting Elements.

Bug: 813902
Test: external/wpt/web-animations/interfaces/KeyframeEffect/target.html
Test: external/wpt/web-animations/interfaces/KeyframeEffect/idlharness.window.js

Change-Id: I91faf22473e7776a8cad6165ff60c5d9ee47a9a6
Reviewed-on: https://chromium-review.googlesource.com/1157002
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579840}

third_party/WebKit/LayoutTests/external/wpt/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt

-This is a testharness.js-based test.
-PASS Effect values reflect changes to font-size on element
-PASS Effect values reflect changes to font-size on parent element
-PASS Effect values reflect changes to font-size when computed style is not immediately flushed
-PASS Effect values reflect changes to font-size from reparenting
-FAIL Effect values reflect changes to target element Cannot assign to read only property 'target' of object '#<KeyframeEffect>'
-Harness: the test ran to completion.
-

third_party/WebKit/LayoutTests/external/wpt/web-animations/interfaces/KeyframeEffect/idlharness.window-expected.txt

@@ -6,7 +6,7 @@ PASS KeyframeEffect interface object name
 PASS KeyframeEffect interface: existence and properties of interface prototype object
 PASS KeyframeEffect interface: existence and properties of interface prototype object's "constructor" property
 PASS KeyframeEffect interface: existence and properties of interface prototype object's @@unscopables property
-FAIL KeyframeEffect interface: attribute target assert_equals: setter must be function for PutForwards, Replaceable, or non-readonly attributes expected "function" but got "undefined"
+PASS KeyframeEffect interface: attribute target
 PASS Unscopable handled correctly for target property on KeyframeEffect
 FAIL KeyframeEffect interface: attribute iterationComposite assert_true: The prototype object must have a property "iterationComposite" expected true got false
 PASS Unscopable handled correctly for iterationComposite property on KeyframeEffect

third_party/WebKit/LayoutTests/external/wpt/web-animations/interfaces/KeyframeEffect/target-expected.txt

-This is a testharness.js-based test.
-FAIL Test setting target before constructing the associated animation Cannot assign to read only property 'target' of object '#<KeyframeEffect>'
-FAIL Test setting target from null to a valid target Cannot assign to read only property 'target' of object '#<KeyframeEffect>'
-FAIL Test setting target from a valid target to null Cannot assign to read only property 'target' of object '#<KeyframeEffect>'
-FAIL Test setting target from a valid target to another target Cannot assign to read only property 'target' of object '#<KeyframeEffect>'
-Harness: the test ran to completion.
-

third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt

@@ -4116,6 +4116,7 @@ interface KeyframeEffect : AnimationEffect
     method getKeyframes
     method setKeyframes
     setter composite
+    setter target
 interface LinearAccelerationSensor : Accelerometer
     attribute @@toStringTag
     method constructor

third_party/blink/renderer/core/animation/keyframe_effect.cc

@@ -128,6 +128,17 @@ KeyframeEffect::KeyframeEffect(Element* target,
 
 KeyframeEffect::~KeyframeEffect() = default;
 
+void KeyframeEffect::setTarget(Element* target) {
+  if (target_ == target)
+    return;
+
+  DetachTarget(GetAnimation());
+  target_ = target;
+  AttachTarget(GetAnimation());
+
+  InvalidateAndNotifyOwner();
+}
+
 String KeyframeEffect::composite() const {
   return EffectModel::CompositeOperationToString(CompositeInternal());
 }
@@ -191,8 +202,7 @@ void KeyframeEffect::SetKeyframes(StringKeyframeVector keyframes) {
 
   // Changing the keyframes will invalidate any sampled effect, as well as
   // potentially affect the effect owner.
-  if (sampled_effect_)
-    ClearEffects();
+  ClearEffects();
   InvalidateAndNotifyOwner();
 }
 
@@ -369,8 +379,8 @@ void KeyframeEffect::ApplyEffects() {
 }
 
 void KeyframeEffect::ClearEffects() {
-  DCHECK(sampled_effect_);
-
+  if (!sampled_effect_)
+    return;
   sampled_effect_->Clear();
   sampled_effect_ = nullptr;
   if (GetAnimation())
@@ -388,30 +398,40 @@ void KeyframeEffect::UpdateChildrenAndEffects() const {
   DCHECK(owner_);
   if (IsInEffect() && !owner_->EffectSuppressed())
     const_cast<KeyframeEffect*>(this)->ApplyEffects();
-  else if (sampled_effect_)
+  else
     const_cast<KeyframeEffect*>(this)->ClearEffects();
 }
 
 void KeyframeEffect::Attach(AnimationEffectOwner* owner) {
-  if (target_ && owner->GetAnimation()) {
-    target_->EnsureElementAnimations().Animations().insert(
-        owner->GetAnimation());
-    target_->SetNeedsAnimationStyleRecalc();
-    if (RuntimeEnabledFeatures::WebAnimationsSVGEnabled() &&
-        target_->IsSVGElement())
-      ToSVGElement(target_)->SetWebAnimationsPending();
-  }
+  AttachTarget(owner->GetAnimation());
   AnimationEffect::Attach(owner);
 }
 
 void KeyframeEffect::Detach() {
-  if (target_ && GetAnimation())
-    target_->GetElementAnimations()->Animations().erase(GetAnimation());
-  if (sampled_effect_)
-    ClearEffects();
+  DetachTarget(GetAnimation());
   AnimationEffect::Detach();
 }
 
+void KeyframeEffect::AttachTarget(Animation* animation) {
+  if (!target_ || !animation)
+    return;
+  target_->EnsureElementAnimations().Animations().insert(animation);
+  target_->SetNeedsAnimationStyleRecalc();
+  if (RuntimeEnabledFeatures::WebAnimationsSVGEnabled() &&
+      target_->IsSVGElement())
+    ToSVGElement(target_)->SetWebAnimationsPending();
+}
+
+void KeyframeEffect::DetachTarget(Animation* animation) {
+  if (target_ && animation)
+    target_->GetElementAnimations()->Animations().erase(animation);
+  // If we have sampled this effect previously, we need to purge that state.
+  // ClearEffects takes care of clearing the cached sampled effect, informing
+  // the target that it needs to refresh its style, and doing any necessary
+  // update on the compositor.
+  ClearEffects();
+}
+
 double KeyframeEffect::CalculateTimeToEffectChange(
     bool forwards,
     double local_time,

third_party/blink/renderer/core/animation/keyframe_effect.h

@@ -77,6 +77,7 @@ class CORE_EXPORT KeyframeEffect final : public AnimationEffect {
 
   // IDL implementation.
   Element* target() const { return target_; }
+  void setTarget(Element*);
   String composite() const;
   void setComposite(String);
   Vector<ScriptValue> getKeyframes(ScriptState*);
@@ -139,6 +140,8 @@ class CORE_EXPORT KeyframeEffect final : public AnimationEffect {
   void UpdateChildrenAndEffects() const override;
   void Attach(AnimationEffectOwner*) override;
   void Detach() override;
+  void AttachTarget(Animation*);
+  void DetachTarget(Animation*);
   double CalculateTimeToEffectChange(
       bool forwards,
       double inherited_time,

third_party/blink/renderer/core/animation/keyframe_effect.idl

@@ -39,7 +39,7 @@ enum CompositeOperation { "replace", "add", "accumulate" };
     RaisesException=Constructor,
     RuntimeEnabled=WebAnimationsAPI
 ] interface KeyframeEffect : AnimationEffect {
-  readonly attribute Element? target;
+  attribute Element? target;
   attribute CompositeOperation composite;
   [CallWith=ScriptState] sequence<object> getKeyframes();
   [CallWith=ScriptState, RaisesException] void setKeyframes(object? keyframes);