Bug 1260976 - Remove some references to properties within nsTransitionManager; r?heycam
Although we know that the animation properties will always be filled in for
a transition in the cases where we need to query them (going forward we will
have a situation where an animation may only have frames, not properties, but
that will only happen when the animation isn't attached to an element or the
element is not attached to a document, but we don't run animations in that case
and cancel existing ones when we enter that state so although they *can* enter
that state, we'll never run these methods on them when they do), we still want
to move towards making frames the primary unit for interacting with animation
values since frames always exist and represent the public interface.
Ultimately it would be good to make the properties array on
a KeyframeEffect(ReadOnly) an encapsulated detail so that we can freely change
their structure (e.g. segments might not be the best setup, it might be better
to just have arrays of free-standing values to avoid the duplication of
values when segments are continuous).
This patch removes or encapsulates a few references to properties and
simplifies the code at the same time.
MozReview-Commit-ID: 3II36SYVoRE
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -170,16 +170,27 @@ CSSTransition::TransitionProperty() cons
// we'll need to store the original transition property so we keep
// returning the same value in that case.
dom::KeyframeEffectReadOnly* effect = GetEffect();
MOZ_ASSERT(effect && effect->AsTransition(),
"Transition should have a transition effect");
return effect->AsTransition()->TransitionProperty();
}
+StyleAnimationValue
+CSSTransition::ToValue() const
+{
+ // FIXME: Once we support replacing/removing the effect (bug 1049975)
+ // the following assertion will no longer hold.
+ dom::KeyframeEffectReadOnly* effect = GetEffect();
+ MOZ_ASSERT(effect && effect->AsTransition(),
+ "Transition should have a transition effect");
+ return effect->AsTransition()->ToValue();
+}
+
bool
CSSTransition::HasLowerCompositeOrderThan(const CSSTransition& aOther) const
{
MOZ_ASSERT(IsTiedToMarkup() && aOther.IsTiedToMarkup(),
"Should only be called for CSS transitions that are sorted "
"as CSS transitions (i.e. tied to CSS markup)");
// 0. Object-equality case
@@ -418,34 +429,28 @@ nsTransitionManager::StyleContextChanged
OwningCSSTransitionPtrArray& animations = collection->mAnimations;
size_t i = animations.Length();
MOZ_ASSERT(i != 0, "empty transitions list?");
StyleAnimationValue currentValue;
do {
--i;
CSSTransition* anim = animations[i];
- dom::KeyframeEffectReadOnly* effect = anim->GetEffect();
- MOZ_ASSERT(effect && effect->Properties().Length() == 1,
- "Should have one animation property for a transition");
- MOZ_ASSERT(effect && effect->Properties()[0].mSegments.Length() == 1,
- "Animation property should have one segment for a transition");
- const AnimationProperty& prop = effect->Properties()[0];
- const AnimationPropertySegment& segment = prop.mSegments[0];
// properties no longer in 'transition-property'
if ((checkProperties &&
- !allTransitionProperties.HasProperty(prop.mProperty)) ||
+ !allTransitionProperties.HasProperty(anim->TransitionProperty())) ||
// properties whose computed values changed but for which we
// did not start a new transition (because delay and
// duration are both zero, or because the new value is not
- // interpolable); a new transition would have segment.mToValue
+ // interpolable); a new transition would have anim->ToValue()
// matching currentValue
- !ExtractComputedValueForTransition(prop.mProperty, afterChangeStyle,
+ !ExtractComputedValueForTransition(anim->TransitionProperty(),
+ afterChangeStyle,
currentValue) ||
- currentValue != segment.mToValue) {
+ currentValue != anim->ToValue()) {
// stop the transition
if (anim->HasCurrentEffect()) {
EffectSet* effectSet = EffectSet::GetEffectSet(aElement, pseudoType);
if (effectSet) {
effectSet->UpdateAnimationGeneration(mPresContext);
}
}
anim->CancelFromStyle();
@@ -798,30 +803,23 @@ nsTransitionManager::PruneCompletedTrans
do {
--i;
CSSTransition* anim = animations[i];
if (anim->HasCurrentEffect()) {
continue;
}
- dom::KeyframeEffectReadOnly* effect = anim->GetEffect();
- MOZ_ASSERT(effect->Properties().Length() == 1,
- "Should have one animation property for a transition");
- MOZ_ASSERT(effect->Properties()[0].mSegments.Length() == 1,
- "Animation property should have one segment for a transition");
- const AnimationProperty& prop = effect->Properties()[0];
- const AnimationPropertySegment& segment = prop.mSegments[0];
-
// Since effect is a finished transition, we know it didn't
// influence style.
StyleAnimationValue currentValue;
- if (!ExtractComputedValueForTransition(prop.mProperty, aNewStyleContext,
+ if (!ExtractComputedValueForTransition(anim->TransitionProperty(),
+ aNewStyleContext,
currentValue) ||
- currentValue != segment.mToValue) {
+ currentValue != anim->ToValue()) {
anim->CancelFromStyle();
animations.RemoveElementAt(i);
}
} while (i != 0);
if (collection->mAnimations.IsEmpty()) {
collection->Destroy();
// |collection| is now a dangling pointer!
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -143,16 +143,17 @@ public:
// However, once we clear the owning element, CascadeLevel() will begin
// returning CascadeLevel::Animations.
mOwningElement = OwningElementRef();
}
void Tick() override;
nsCSSProperty TransitionProperty() const;
+ StyleAnimationValue ToValue() const;
bool HasLowerCompositeOrderThan(const CSSTransition& aOther) const;
EffectCompositor::CascadeLevel CascadeLevel() const override
{
return IsTiedToMarkup() ?
EffectCompositor::CascadeLevel::Transitions :
EffectCompositor::CascadeLevel::Animations;
}