From 47fe95d62996f45826788b712785e606db81d7f7 Mon Sep 17 00:00:00 2001 From: "Carsten \"Tomcat\" Book" Date: Wed, 18 Mar 2015 16:32:54 +0100 Subject: [PATCH] Backed out 7 changesets (bug 1125455) for test failures in m1 test_animation-player-ready.html on a CLOSED TREE Backed out changeset 8a316064caff (bug 1125455) Backed out changeset ad326dbcbd03 (bug 1125455) Backed out changeset 83dab9578e23 (bug 1125455) Backed out changeset 5bd86c20cd02 (bug 1125455) Backed out changeset 751177025dcb (bug 1125455) Backed out changeset f60c5b4adf84 (bug 1125455) Backed out changeset 326ef9a86c85 (bug 1125455) --- dom/animation/Animation.cpp | 18 +---- dom/animation/Animation.h | 21 +---- layout/base/nsDisplayList.cpp | 87 +++++++++------------ layout/style/AnimationCommon.cpp | 8 -- layout/style/AnimationCommon.h | 22 ------ layout/style/nsAnimationManager.cpp | 8 -- layout/style/nsTransitionManager.cpp | 80 ------------------- layout/style/nsTransitionManager.h | 7 -- layout/style/test/test_animations.html | 21 ----- layout/style/test/test_animations_omta.html | 28 ------- 10 files changed, 44 insertions(+), 256 deletions(-) diff --git a/dom/animation/Animation.cpp b/dom/animation/Animation.cpp index 46e5cd0554fb..483feff47c35 100644 --- a/dom/animation/Animation.cpp +++ b/dom/animation/Animation.cpp @@ -252,16 +252,16 @@ Animation::IsInEffect() const return computedTiming.mTimeFraction != ComputedTiming::kNullTimeFraction; } -const AnimationProperty* -Animation::GetAnimationOfProperty(nsCSSProperty aProperty) const +bool +Animation::HasAnimationOfProperty(nsCSSProperty aProperty) const { for (size_t propIdx = 0, propEnd = mProperties.Length(); propIdx != propEnd; ++propIdx) { if (aProperty == mProperties[propIdx].mProperty) { - return &mProperties[propIdx]; + return true; } } - return nullptr; + return false; } void @@ -297,16 +297,6 @@ Animation::ComposeStyle(nsRefPtr& aStyleRule, continue; } - if (!prop.mWinsInCascade) { - // This isn't the winning declaration, so don't add it to style. - // For transitions, this is important, because it's how we - // implement the rule that CSS transitions don't run when a CSS - // animation is running on the same property and element. For - // animations, this is only skipping things that will otherwise be - // overridden. - continue; - } - aSetProperties.AddProperty(prop.mProperty); MOZ_ASSERT(prop.mSegments.Length() > 0, diff --git a/dom/animation/Animation.h b/dom/animation/Animation.h index 93cb4b2526e8..d0510e331881 100644 --- a/dom/animation/Animation.h +++ b/dom/animation/Animation.h @@ -150,25 +150,10 @@ struct AnimationPropertySegment struct AnimationProperty { nsCSSProperty mProperty; - - // Does this property win in the CSS Cascade? - // - // For CSS transitions, this is true as long as a CSS animation on the - // same property and element is not running, in which case we set this - // to false so that the animation (lower in the cascade) can win. We - // then use this to decide whether to apply the style both in the CSS - // cascade and for OMTA. - // - // FIXME (bug 847287): For CSS Animations, which are overridden by - // !important rules in the cascade, we actually determine this from - // the CSS cascade computations, and then use it for OMTA. - bool mWinsInCascade; - InfallibleTArray mSegments; bool operator==(const AnimationProperty& aOther) const { return mProperty == aOther.mProperty && - mWinsInCascade == aOther.mWinsInCascade && mSegments == aOther.mSegments; } bool operator!=(const AnimationProperty& aOther) const { @@ -305,11 +290,7 @@ public: bool IsCurrent() const; bool IsInEffect() const; - const AnimationProperty* - GetAnimationOfProperty(nsCSSProperty aProperty) const; - bool HasAnimationOfProperty(nsCSSProperty aProperty) const { - return GetAnimationOfProperty(aProperty) != nullptr; - } + bool HasAnimationOfProperty(nsCSSProperty aProperty) const; const InfallibleTArray& Properties() const { return mProperties; } diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index d907312591cc..4afa515f8df6 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -332,7 +332,7 @@ static void AddTransformFunctions(nsCSSValueList* aList, } static TimingFunction -ToTimingFunction(const ComputedTimingFunction& aCTF) +ToTimingFunction(ComputedTimingFunction& aCTF) { if (aCTF.GetType() == nsTimingFunction::Function) { const nsSMILKeySpline* spline = aCTF.GetFunction(); @@ -345,7 +345,7 @@ ToTimingFunction(const ComputedTimingFunction& aCTF) } static void -AddAnimationForProperty(nsIFrame* aFrame, const AnimationProperty& aProperty, +AddAnimationForProperty(nsIFrame* aFrame, nsCSSProperty aProperty, AnimationPlayer* aPlayer, Layer* aLayer, AnimationData& aData, bool aPending) { @@ -373,33 +373,44 @@ AddAnimationForProperty(nsIFrame* aFrame, const AnimationProperty& aProperty, animation->duration() = timing.mIterationDuration; animation->iterationCount() = timing.mIterationCount; animation->direction() = timing.mDirection; - animation->property() = aProperty.mProperty; + animation->property() = aProperty; animation->data() = aData; - for (uint32_t segIdx = 0; segIdx < aProperty.mSegments.Length(); segIdx++) { - const AnimationPropertySegment& segment = aProperty.mSegments[segIdx]; + dom::Animation* anim = aPlayer->GetSource(); + for (size_t propIdx = 0; + propIdx < anim->Properties().Length(); + propIdx++) { + AnimationProperty& property = anim->Properties()[propIdx]; - AnimationSegment* animSegment = animation->segments().AppendElement(); - if (aProperty.mProperty == eCSSProperty_transform) { - animSegment->startState() = InfallibleTArray(); - animSegment->endState() = InfallibleTArray(); - - nsCSSValueSharedList* list = - segment.mFromValue.GetCSSValueSharedListValue(); - AddTransformFunctions(list->mHead, styleContext, presContext, bounds, - animSegment->startState().get_ArrayOfTransformFunction()); - - list = segment.mToValue.GetCSSValueSharedListValue(); - AddTransformFunctions(list->mHead, styleContext, presContext, bounds, - animSegment->endState().get_ArrayOfTransformFunction()); - } else if (aProperty.mProperty == eCSSProperty_opacity) { - animSegment->startState() = segment.mFromValue.GetFloatValue(); - animSegment->endState() = segment.mToValue.GetFloatValue(); + if (aProperty != property.mProperty) { + continue; } - animSegment->startPortion() = segment.mFromKey; - animSegment->endPortion() = segment.mToKey; - animSegment->sampleFn() = ToTimingFunction(segment.mTimingFunction); + for (uint32_t segIdx = 0; segIdx < property.mSegments.Length(); segIdx++) { + AnimationPropertySegment& segment = property.mSegments[segIdx]; + + AnimationSegment* animSegment = animation->segments().AppendElement(); + if (aProperty == eCSSProperty_transform) { + animSegment->startState() = InfallibleTArray(); + animSegment->endState() = InfallibleTArray(); + + nsCSSValueSharedList* list = + segment.mFromValue.GetCSSValueSharedListValue(); + AddTransformFunctions(list->mHead, styleContext, presContext, bounds, + animSegment->startState().get_ArrayOfTransformFunction()); + + list = segment.mToValue.GetCSSValueSharedListValue(); + AddTransformFunctions(list->mHead, styleContext, presContext, bounds, + animSegment->endState().get_ArrayOfTransformFunction()); + } else if (aProperty == eCSSProperty_opacity) { + animSegment->startState() = segment.mFromValue.GetFloatValue(); + animSegment->endState() = segment.mToValue.GetFloatValue(); + } + + animSegment->startPortion() = segment.mFromKey; + animSegment->endPortion() = segment.mToKey; + animSegment->sampleFn() = ToTimingFunction(segment.mTimingFunction); + } } } @@ -407,32 +418,12 @@ static void AddAnimationsForProperty(nsIFrame* aFrame, nsCSSProperty aProperty, AnimationPlayerPtrArray& aPlayers, Layer* aLayer, AnimationData& aData, - bool aPending) -{ + bool aPending) { for (size_t playerIdx = 0; playerIdx < aPlayers.Length(); playerIdx++) { AnimationPlayer* player = aPlayers[playerIdx]; - if (!player->IsRunning()) { - continue; - } dom::Animation* anim = player->GetSource(); - if (!anim) { - continue; - } - const AnimationProperty* property = - anim->GetAnimationOfProperty(aProperty); - if (!property) { - continue; - } - - if (!property->mWinsInCascade) { - // We have an animation or transition, but it isn't actually - // winning in the CSS cascade, so we don't want to send it to the - // compositor. - // I believe that anything that changes mWinsInCascade should - // trigger this code again, either because of a restyle that - // changes the properties in question, or because of the - // main-thread style update that results when an animation stops - // filling. + if (!(anim && anim->HasAnimationOfProperty(aProperty) && + player->IsRunning())) { continue; } @@ -452,7 +443,7 @@ AddAnimationsForProperty(nsIFrame* aFrame, nsCSSProperty aProperty, } } - AddAnimationForProperty(aFrame, *property, player, aLayer, aData, aPending); + AddAnimationForProperty(aFrame, aProperty, player, aLayer, aData, aPending); player->SetIsRunningOnCompositor(); } } diff --git a/layout/style/AnimationCommon.cpp b/layout/style/AnimationCommon.cpp index fe9cf87c681d..9a1dab1575a9 100644 --- a/layout/style/AnimationCommon.cpp +++ b/layout/style/AnimationCommon.cpp @@ -741,14 +741,6 @@ AnimationPlayerCollection::EnsureStyleRuleFor(TimeStamp aRefreshTime, } mManager->CheckNeedsRefresh(); - - // If one of our animations just started or stopped filling, we need - // to notify the transition manager. This does the notification a bit - // more than necessary, but it's easier than doing it exactly. - if (mManager->IsAnimationManager()) { - mManager->mPresContext->TransitionManager()-> - UpdateCascadeResultsWithAnimations(this); - } } bool diff --git a/layout/style/AnimationCommon.h b/layout/style/AnimationCommon.h index 5cd1e9076e04..04ad10f5ee88 100644 --- a/layout/style/AnimationCommon.h +++ b/layout/style/AnimationCommon.h @@ -24,7 +24,6 @@ #include "mozilla/FloatingPoint.h" #include "nsCSSPseudoElements.h" #include "nsCycleCollectionParticipant.h" -#include "nsCSSPropertySet.h" class nsIFrame; class nsPresContext; @@ -192,14 +191,6 @@ public: mozilla::StyleAnimationValue mValue; }; - void AddPropertiesToSet(nsCSSPropertySet& aSet) const - { - for (size_t i = 0, i_end = mPropertyValuePairs.Length(); i < i_end; ++i) { - const PropertyValuePair &cv = mPropertyValuePairs[i]; - aSet.AddProperty(cv.mProperty); - } - } - private: ~AnimValuesStyleRule() {} @@ -333,19 +324,6 @@ struct AnimationPlayerCollection : public PRCList return NS_LITERAL_STRING("::after"); } - nsCSSPseudoElements::Type PseudoElementType() const - { - if (IsForElement()) { - return nsCSSPseudoElements::ePseudo_NotPseudoElement; - } - if (IsForBeforePseudo()) { - return nsCSSPseudoElements::ePseudo_before; - } - MOZ_ASSERT(IsForAfterPseudo(), - "::before & ::after should be the only pseudo-elements here"); - return nsCSSPseudoElements::ePseudo_after; - } - mozilla::dom::Element* GetElementToRestyle() const; void PostRestyleForAnimation(nsPresContext *aPresContext) { diff --git a/layout/style/nsAnimationManager.cpp b/layout/style/nsAnimationManager.cpp index 424b6759b39f..e98239fe28c0 100644 --- a/layout/style/nsAnimationManager.cpp +++ b/layout/style/nsAnimationManager.cpp @@ -272,13 +272,6 @@ nsAnimationManager::CheckAnimationRule(nsStyleContext* aStyleContext, if (newPlayers.IsEmpty()) { if (collection) { - // There might be transitions that run now that animations don't - // override them. - collection->mPlayers.Clear(); - collection->mStyleRule = nullptr; - mPresContext->TransitionManager()-> - UpdateCascadeResultsWithAnimations(collection); - collection->Destroy(); } return nullptr; @@ -601,7 +594,6 @@ nsAnimationManager::BuildAnimations(nsStyleContext* aStyleContext, AnimationProperty &propData = *destAnim->Properties().AppendElement(); propData.mProperty = prop; - propData.mWinsInCascade = true; KeyframeData *fromKeyframe = nullptr; nsRefPtr fromContext; diff --git a/layout/style/nsTransitionManager.cpp b/layout/style/nsTransitionManager.cpp index bf4174135268..54fa80f28f62 100644 --- a/layout/style/nsTransitionManager.cpp +++ b/layout/style/nsTransitionManager.cpp @@ -347,8 +347,6 @@ nsTransitionManager::StyleContextChanged(dom::Element *aElement, "must have element transitions if we started any transitions"); if (collection) { - UpdateCascadeResultsWithTransitions(collection); - // Set the style rule refresh time to null so that EnsureStyleRuleFor // creates a new style rule if we started *or* stopped transitions. collection->mStyleRuleRefreshTime = TimeStamp(); @@ -547,7 +545,6 @@ nsTransitionManager::ConsiderStartingTransition( AnimationProperty& prop = *pt->Properties().AppendElement(); prop.mProperty = aProperty; - prop.mWinsInCascade = true; AnimationPropertySegment& segment = *prop.mSegments.AppendElement(); segment.mFromValue = startValue; @@ -602,83 +599,6 @@ nsTransitionManager::ConsiderStartingTransition( aWhichStarted->AddProperty(aProperty); } -void -nsTransitionManager::UpdateCascadeResultsWithTransitions( - AnimationPlayerCollection* aTransitions) -{ - AnimationPlayerCollection* animations = - mPresContext->AnimationManager()-> - GetAnimationPlayers(aTransitions->mElement, - aTransitions->PseudoElementType(), false); - UpdateCascadeResults(aTransitions, animations); -} - -void -nsTransitionManager::UpdateCascadeResultsWithAnimations( - const AnimationPlayerCollection* aAnimations) -{ - AnimationPlayerCollection* transitions = - mPresContext->TransitionManager()-> - GetAnimationPlayers(aAnimations->mElement, - aAnimations->PseudoElementType(), false); - UpdateCascadeResults(transitions, aAnimations); -} - -void -nsTransitionManager::UpdateCascadeResults( - AnimationPlayerCollection* aTransitions, - const AnimationPlayerCollection* aAnimations) -{ - if (!aTransitions) { - // Nothing to do. - return; - } - - nsCSSPropertySet propertiesUsed; -#ifdef DEBUG - nsCSSPropertySet propertiesWithTransitions; -#endif - - // http://dev.w3.org/csswg/css-transitions/#application says that - // transitions do not apply when the same property has a CSS Animation - // on that element (even though animations are lower in the cascade). - if (aAnimations && aAnimations->mStyleRule) { - aAnimations->mStyleRule->AddPropertiesToSet(propertiesUsed); - } - - // Since we should never have more than one transition for the same - // property, it doesn't matter what order we iterate the transitions. - // But let's go the same way as animations. - bool changed = false; - AnimationPlayerPtrArray& players = aTransitions->mPlayers; - for (size_t playerIdx = players.Length(); playerIdx-- != 0; ) { - MOZ_ASSERT(players[playerIdx]->GetSource() && - players[playerIdx]->GetSource()->Properties().Length() == 1, - "Should have one animation property for a transition"); - AnimationProperty& prop = players[playerIdx]->GetSource()->Properties()[0]; - bool newWinsInCascade = !propertiesUsed.HasProperty(prop.mProperty); - if (prop.mWinsInCascade != newWinsInCascade) { - changed = true; - } - prop.mWinsInCascade = newWinsInCascade; - // assert that we don't need to bother adding the transitioned - // properties into propertiesUsed -#ifdef DEBUG - MOZ_ASSERT(!propertiesWithTransitions.HasProperty(prop.mProperty), - "we're assuming we have only one transition per property"); - propertiesWithTransitions.AddProperty(prop.mProperty); -#endif - } - - if (changed) { - aTransitions->UpdateAnimationGeneration(mPresContext); - - // Invalidate our style rule. - aTransitions->mStyleRuleRefreshTime = TimeStamp(); - aTransitions->mNeedsRefreshes = true; - } -} - /* * nsIStyleRuleProcessor implementation */ diff --git a/layout/style/nsTransitionManager.h b/layout/style/nsTransitionManager.h index 8c8cb6c0c8c7..83490f002d1c 100644 --- a/layout/style/nsTransitionManager.h +++ b/layout/style/nsTransitionManager.h @@ -128,13 +128,6 @@ public: nsStyleContext *aOldStyleContext, nsRefPtr* aNewStyleContext /* inout */); - void UpdateCascadeResultsWithTransitions( - AnimationPlayerCollection* aTransitions); - void UpdateCascadeResultsWithAnimations( - const AnimationPlayerCollection* aAnimations); - void UpdateCascadeResults(AnimationPlayerCollection* aTransitions, - const AnimationPlayerCollection* aAnimations); - void SetInAnimationOnlyStyleUpdate(bool aInAnimationOnlyUpdate) { mInAnimationOnlyStyleUpdate = aInAnimationOnlyUpdate; } diff --git a/layout/style/test/test_animations.html b/layout/style/test/test_animations.html index a5f58fc603f5..2310464f1752 100644 --- a/layout/style/test/test_animations.html +++ b/layout/style/test/test_animations.html @@ -158,11 +158,6 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=435442 @keyframes overridetop { 0%, 100% { top: 0px } } - - @keyframes opacitymid { - 0% { opacity: 0.2 } - 100% { opacity: 0.8 } - } @@ -2016,22 +2011,6 @@ ancestor.parentNode.removeChild(ancestor); done_div(); -/* - * Bug 1125455 - Transitions should not run when animations are running. - */ -new_div("transition: opacity 2s linear; opacity: 0.8"); -advance_clock(0); -is(cs.getPropertyValue("opacity"), "0.8", "initial opacity"); -div.style.opacity = "0.2"; -is(cs.getPropertyValue("opacity"), "0.8", "opacity transition at 0s"); -advance_clock(500); -is(cs.getPropertyValue("opacity"), "0.65", "opacity transition at 0.5s"); -div.style.animation = "opacitymid 2s linear"; -is(cs.getPropertyValue("opacity"), "0.2", "opacity animation overriding transition at 0s"); -advance_clock(500); -is(cs.getPropertyValue("opacity"), "0.35", "opacity animation overriding transition at 0.5s"); -done_div(); - SpecialPowers.DOMWindowUtils.restoreNormalRefresh(); diff --git a/layout/style/test/test_animations_omta.html b/layout/style/test/test_animations_omta.html index b56aed0e642c..9dd19d4e8f22 100644 --- a/layout/style/test/test_animations_omta.html +++ b/layout/style/test/test_animations_omta.html @@ -154,11 +154,6 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=964646 #visitedLink:link { background-color: yellow } #visitedLink:visited { background-color: blue } - - @keyframes opacitymid { - 0% { opacity: 0.2 } - 100% { opacity: 0.8 } - } @@ -2134,28 +2129,5 @@ addAsyncAnimTest(function *() { done_div(); }); -// Bug 1125455 - Transitions should not run when animations are running. -addAsyncAnimTest(function *() { - new_div("transition: opacity 2s linear; opacity: 0.8"); - yield waitForPaintsFlushed(); - omta_is("opacity", 0.8, RunningOn.MainThread, - "initial opacity"); - gDiv.style.opacity = "0.2"; - yield waitForPaintsFlushed(); - omta_is("opacity", 0.8, RunningOn.Compositor, - "opacity transition at 0s"); - advance_clock(500); - omta_is("opacity", 0.65, RunningOn.Compositor, - "opacity transition at 0.5s"); - gDiv.style.animation = "opacitymid 2s linear"; - yield waitForPaintsFlushed(); - omta_is("opacity", 0.2, RunningOn.Compositor, - "opacity animation overriding transition at 0s"); - advance_clock(500); - omta_is("opacity", 0.35, RunningOn.Compositor, - "opacity animation overriding transition at 0.5s"); - done_div(); -}); -