Bug 1456394 - Move KeyframeEffect attribute setters to KeyframeEffectReadOnly; r?hiro,bz
It might seem a bit odd to move the setters to the read-only class that we are
ultimately planning to drop but the reason for doing this is that
KeyframeEffectReadOnly.cpp has a *lot* more code than KeyframeEffect.cpp.
In order to simplify code archaeology we take the following approach:
1. Move code from KeyframeEffect.{h,cpp} to KeyframeEffectReadOnly.{h,cpp}.
2. Delete KeyframeEffect.{h,cpp}.
3. Rename KeyframeEffectReadOnly.{h,cpp} to KeyframeEffect.{h,cpp}.
Note that at least steps 2 and 3 must be performed in separate patches as
mercurial does not successfully track renames when the target name already
exists.
MozReview-Commit-ID: LwJoxGJitKR
--- a/dom/animation/KeyframeEffect.cpp
+++ b/dom/animation/KeyframeEffect.cpp
@@ -80,95 +80,10 @@ KeyframeEffect::NotifySpecifiedTimingUpd
if (mAnimation->IsRelevant()) {
nsNodeUtils::AnimationChanged(mAnimation);
}
RequestRestyle(EffectCompositor::RestyleType::Layer);
}
}
-void
-KeyframeEffect::SetTarget(const Nullable<ElementOrCSSPseudoElement>& aTarget)
-{
- Maybe<OwningAnimationTarget> newTarget = ConvertTarget(aTarget);
- if (mTarget == newTarget) {
- // Assign the same target, skip it.
- return;
- }
-
- if (mTarget) {
- UnregisterTarget();
- ResetIsRunningOnCompositor();
-
- RequestRestyle(EffectCompositor::RestyleType::Layer);
-
- nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
- if (mAnimation) {
- nsNodeUtils::AnimationRemoved(mAnimation);
- }
- }
-
- mTarget = newTarget;
-
- if (mTarget) {
- UpdateTargetRegistration();
- RefPtr<ComputedStyle> computedStyle = GetTargetComputedStyle();
- if (computedStyle) {
- UpdateProperties(computedStyle);
- }
-
- MaybeUpdateFrameForCompositor();
-
- RequestRestyle(EffectCompositor::RestyleType::Layer);
-
- nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
- if (mAnimation) {
- nsNodeUtils::AnimationAdded(mAnimation);
- }
- }
-}
-
-void
-KeyframeEffect::SetIterationComposite(
- const IterationCompositeOperation& aIterationComposite,
- CallerType aCallerType)
-{
- // Ignore iterationComposite if the Web Animations API is not enabled,
- // then the default value 'Replace' will be used.
- if (!nsDocument::IsWebAnimationsEnabled(aCallerType)) {
- return;
- }
-
- if (mEffectOptions.mIterationComposite == aIterationComposite) {
- return;
- }
-
- if (mAnimation && mAnimation->IsRelevant()) {
- nsNodeUtils::AnimationChanged(mAnimation);
- }
-
- mEffectOptions.mIterationComposite = aIterationComposite;
- RequestRestyle(EffectCompositor::RestyleType::Layer);
-}
-
-void
-KeyframeEffect::SetComposite(const CompositeOperation& aComposite)
-{
- if (mEffectOptions.mComposite == aComposite) {
- return;
- }
-
- mEffectOptions.mComposite = aComposite;
-
- if (mAnimation && mAnimation->IsRelevant()) {
- nsNodeUtils::AnimationChanged(mAnimation);
- }
-
- if (mTarget) {
- RefPtr<ComputedStyle> computedStyle = GetTargetComputedStyle();
- if (computedStyle) {
- UpdateProperties(computedStyle);
- }
- }
-}
-
} // namespace dom
} // namespace mozilla
--- a/dom/animation/KeyframeEffect.h
+++ b/dom/animation/KeyframeEffect.h
@@ -59,30 +59,14 @@ public:
static already_AddRefed<KeyframeEffect>
Constructor(const GlobalObject& aGlobal,
const Nullable<ElementOrCSSPseudoElement>& aTarget,
JS::Handle<JSObject*> aKeyframes,
const UnrestrictedDoubleOrKeyframeAnimationOptions& aOptions,
ErrorResult& aRv);
void NotifySpecifiedTimingUpdated();
-
- // This method calls GetTargetComputedStyle which is not safe to use when
- // we are in the middle of updating style. If we need to use this when
- // updating style, we should pass the ComputedStyle into this method and use
- // that to update the properties rather than calling
- // GetComputedStyle.
- void SetTarget(const Nullable<ElementOrCSSPseudoElement>& aTarget);
-
- IterationCompositeOperation IterationComposite(CallerType aCallerType)
- {
- return KeyframeEffectReadOnly::IterationComposite();
- }
- void SetIterationComposite(
- const IterationCompositeOperation& aIterationComposite,
- CallerType aCallerType);
- void SetComposite(const CompositeOperation& aComposite);
};
} // namespace dom
} // namespace mozilla
#endif // mozilla_dom_KeyframeEffect_h
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -95,29 +95,73 @@ KeyframeEffectReadOnly::KeyframeEffectRe
JSObject*
KeyframeEffectReadOnly::WrapObject(JSContext* aCx,
JS::Handle<JSObject*> aGivenProto)
{
return KeyframeEffectReadOnlyBinding::Wrap(aCx, this, aGivenProto);
}
-IterationCompositeOperation
-KeyframeEffectReadOnly::IterationComposite() const
+IterationCompositeOperation KeyframeEffectReadOnly::IterationComposite(
+ CallerType /*aCallerType*/) const
{
return mEffectOptions.mIterationComposite;
}
+void
+KeyframeEffectReadOnly::SetIterationComposite(
+ const IterationCompositeOperation& aIterationComposite,
+ CallerType aCallerType)
+{
+ // Ignore iterationComposite if the Web Animations API is not enabled,
+ // then the default value 'Replace' will be used.
+ if (!nsDocument::IsWebAnimationsEnabled(aCallerType)) {
+ return;
+ }
+
+ if (mEffectOptions.mIterationComposite == aIterationComposite) {
+ return;
+ }
+
+ if (mAnimation && mAnimation->IsRelevant()) {
+ nsNodeUtils::AnimationChanged(mAnimation);
+ }
+
+ mEffectOptions.mIterationComposite = aIterationComposite;
+ RequestRestyle(EffectCompositor::RestyleType::Layer);
+}
+
CompositeOperation
KeyframeEffectReadOnly::Composite() const
{
return mEffectOptions.mComposite;
}
void
+KeyframeEffectReadOnly::SetComposite(const CompositeOperation& aComposite)
+{
+ if (mEffectOptions.mComposite == aComposite) {
+ return;
+ }
+
+ mEffectOptions.mComposite = aComposite;
+
+ if (mAnimation && mAnimation->IsRelevant()) {
+ nsNodeUtils::AnimationChanged(mAnimation);
+ }
+
+ if (mTarget) {
+ RefPtr<ComputedStyle> computedStyle = GetTargetComputedStyle();
+ if (computedStyle) {
+ UpdateProperties(computedStyle);
+ }
+ }
+}
+
+void
KeyframeEffectReadOnly::NotifyAnimationTimingUpdated()
{
UpdateTargetRegistration();
// If the effect is not relevant it will be removed from the target
// element's effect set. However, effects not in the effect set
// will not be included in the set of candidate effects for running on
// the compositor and hence they won't have their compositor status
@@ -856,16 +900,58 @@ KeyframeEffectReadOnly::GetTarget(
break;
default:
NS_NOTREACHED("Animation of unsupported pseudo-type");
aRv.SetNull();
}
}
+void
+KeyframeEffectReadOnly::SetTarget(
+ const Nullable<ElementOrCSSPseudoElement>& aTarget)
+{
+ Maybe<OwningAnimationTarget> newTarget = ConvertTarget(aTarget);
+ if (mTarget == newTarget) {
+ // Assign the same target, skip it.
+ return;
+ }
+
+ if (mTarget) {
+ UnregisterTarget();
+ ResetIsRunningOnCompositor();
+
+ RequestRestyle(EffectCompositor::RestyleType::Layer);
+
+ nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
+ if (mAnimation) {
+ nsNodeUtils::AnimationRemoved(mAnimation);
+ }
+ }
+
+ mTarget = newTarget;
+
+ if (mTarget) {
+ UpdateTargetRegistration();
+ RefPtr<ComputedStyle> computedStyle = GetTargetComputedStyle();
+ if (computedStyle) {
+ UpdateProperties(computedStyle);
+ }
+
+ MaybeUpdateFrameForCompositor();
+
+ RequestRestyle(EffectCompositor::RestyleType::Layer);
+
+ nsAutoAnimationMutationBatch mb(mTarget->mElement->OwnerDoc());
+ if (mAnimation) {
+ nsNodeUtils::AnimationAdded(mAnimation);
+ }
+ }
+}
+
static void
CreatePropertyValue(nsCSSPropertyID aProperty,
float aOffset,
const Maybe<ComputedTimingFunction>& aTimingFunction,
const AnimationValue& aValue,
dom::CompositeOperation aComposite,
AnimationPropertyValueDetails& aResult)
{
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -146,24 +146,40 @@ public:
Maybe<NonOwningAnimationTarget> GetTarget() const
{
Maybe<NonOwningAnimationTarget> result;
if (mTarget) {
result.emplace(*mTarget);
}
return result;
}
+ // This method calls GetTargetComputedStyle which is not safe to use when
+ // we are in the middle of updating style. If we need to use this when
+ // updating style, we should pass the ComputedStyle into this method and use
+ // that to update the properties rather than calling
+ // GetComputedStyle.
+ void SetTarget(const Nullable<ElementOrCSSPseudoElement>& aTarget);
+
void GetKeyframes(JSContext*& aCx,
nsTArray<JSObject*>& aResult,
ErrorResult& aRv);
void GetProperties(nsTArray<AnimationPropertyDetails>& aProperties,
ErrorResult& aRv) const;
- IterationCompositeOperation IterationComposite() const;
+ // aCallerType is not used in the getter so we supply a default value so that
+ // internal users don't need to specify this value.
+ IterationCompositeOperation IterationComposite(
+ CallerType aCallerType = CallerType::System) const;
+ void SetIterationComposite(
+ const IterationCompositeOperation& aIterationComposite,
+ CallerType aCallerType);
+
CompositeOperation Composite() const;
+ void SetComposite(const CompositeOperation& aComposite);
+
void NotifyAnimationTimingUpdated();
void RequestRestyle(EffectCompositor::RestyleType aRestyleType);
void SetAnimation(Animation* aAnimation) override;
void SetKeyframes(JSContext* aContext, JS::Handle<JSObject*> aKeyframes,
ErrorResult& aRv);
void SetKeyframes(nsTArray<Keyframe>&& aKeyframes,
const ComputedStyle* aStyle);
--- a/dom/webidl/KeyframeEffect.webidl
+++ b/dom/webidl/KeyframeEffect.webidl
@@ -24,19 +24,20 @@ dictionary KeyframeEffectOptions : Anima
// processing on the `keyframes` object.
[Func="nsDocument::IsWebAnimationsEnabled",
RunConstructorInCallerCompartment,
Constructor ((Element or CSSPseudoElement)? target,
object? keyframes,
optional (unrestricted double or KeyframeEffectOptions) options),
Constructor (KeyframeEffectReadOnly source)]
interface KeyframeEffectReadOnly : AnimationEffectReadOnly {
- readonly attribute (Element or CSSPseudoElement)? target;
- readonly attribute IterationCompositeOperation iterationComposite;
- readonly attribute CompositeOperation composite;
+ attribute (Element or CSSPseudoElement)? target;
+ [NeedsCallerType]
+ attribute IterationCompositeOperation iterationComposite;
+ attribute CompositeOperation composite;
// We use object instead of ComputedKeyframe so that we can put the
// property-value pairs on the object.
[Throws] sequence<object> getKeyframes();
};
// Non-standard extensions
dictionary AnimationPropertyValueDetails {
@@ -61,15 +62,11 @@ partial interface KeyframeEffectReadOnly
// processing on the `keyframes` object.
[Func="nsDocument::IsWebAnimationsEnabled",
RunConstructorInCallerCompartment,
Constructor ((Element or CSSPseudoElement)? target,
object? keyframes,
optional (unrestricted double or KeyframeEffectOptions) options),
Constructor (KeyframeEffectReadOnly source)]
interface KeyframeEffect : KeyframeEffectReadOnly {
- inherit attribute (Element or CSSPseudoElement)? target;
- [NeedsCallerType]
- inherit attribute IterationCompositeOperation iterationComposite;
- inherit attribute CompositeOperation composite;
[Throws]
void setKeyframes (object? keyframes);
};
--- a/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/idlharness.html.ini
+++ b/testing/web-platform/meta/web-animations/interfaces/KeyframeEffect/idlharness.html.ini
@@ -3,8 +3,16 @@
expected: FAIL
[KeyframeEffect interface: existence and properties of interface prototype object]
expected: FAIL
[KeyframeEffect interface: operation getKeyframes()]
expected: FAIL
+ [KeyframeEffect interface: attribute target]
+ expected: FAIL
+
+ [KeyframeEffect interface: attribute iterationComposite]
+ expected: FAIL
+
+ [KeyframeEffect interface: attribute composite]
+ expected: FAIL