diff --git a/dom/base/Element.h b/dom/base/Element.h index e35a2486c89..336b3e8009d 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -82,40 +82,29 @@ enum { // descendants). ELEMENT_IS_POTENTIAL_RESTYLE_ROOT = ELEMENT_FLAG_BIT(1), - // Set if the element has a pending animation style change. - ELEMENT_HAS_PENDING_ANIMATION_RESTYLE = ELEMENT_FLAG_BIT(2), - - // Set if the element is a potential animation restyle root (that is, - // has an animation style change pending _and_ that style change - // will attempt to restyle descendants). - ELEMENT_IS_POTENTIAL_ANIMATION_RESTYLE_ROOT = ELEMENT_FLAG_BIT(3), - // Set if the element has a pending animation-only style change as // part of an animation-only style update (where we update styles from // animation to the current refresh tick, but leave everything else as // it was). - ELEMENT_HAS_PENDING_ANIMATION_ONLY_RESTYLE = ELEMENT_FLAG_BIT(4), + ELEMENT_HAS_PENDING_ANIMATION_ONLY_RESTYLE = ELEMENT_FLAG_BIT(2), // Set if the element is a potential animation-only restyle root (that // is, has an animation-only style change pending _and_ that style // change will attempt to restyle descendants). - ELEMENT_IS_POTENTIAL_ANIMATION_ONLY_RESTYLE_ROOT = ELEMENT_FLAG_BIT(5), + ELEMENT_IS_POTENTIAL_ANIMATION_ONLY_RESTYLE_ROOT = ELEMENT_FLAG_BIT(3), // All of those bits together, for convenience. ELEMENT_ALL_RESTYLE_FLAGS = ELEMENT_HAS_PENDING_RESTYLE | ELEMENT_IS_POTENTIAL_RESTYLE_ROOT | - ELEMENT_HAS_PENDING_ANIMATION_RESTYLE | - ELEMENT_IS_POTENTIAL_ANIMATION_RESTYLE_ROOT | ELEMENT_HAS_PENDING_ANIMATION_ONLY_RESTYLE | ELEMENT_IS_POTENTIAL_ANIMATION_ONLY_RESTYLE_ROOT, // Just the HAS_PENDING bits, for convenience ELEMENT_PENDING_RESTYLE_FLAGS = ELEMENT_HAS_PENDING_RESTYLE | - ELEMENT_HAS_PENDING_ANIMATION_RESTYLE | ELEMENT_HAS_PENDING_ANIMATION_ONLY_RESTYLE, // Remaining bits are for subclasses - ELEMENT_TYPE_SPECIFIC_BITS_OFFSET = NODE_TYPE_SPECIFIC_BITS_OFFSET + 6 + ELEMENT_TYPE_SPECIFIC_BITS_OFFSET = NODE_TYPE_SPECIFIC_BITS_OFFSET + 4 }; #undef ELEMENT_FLAG_BIT diff --git a/dom/svg/nsSVGElement.cpp b/dom/svg/nsSVGElement.cpp index 78a0f64124d..60060ae8016 100644 --- a/dom/svg/nsSVGElement.cpp +++ b/dom/svg/nsSVGElement.cpp @@ -918,21 +918,9 @@ nsSVGElement::WalkAnimatedContentStyleRules(nsRuleWalker* aRuleWalker) // whether this is a "no-animation restyle". (This should match the check // in nsHTMLCSSStyleSheet::RulesMatching(), where we determine whether to // apply the SMILOverrideStyle.) - nsPresContext* context = aRuleWalker->PresContext(); - nsIPresShell* shell = context->PresShell(); - RestyleManager* restyleManager = context->RestyleManager(); - if (restyleManager->SkipAnimationRules()) { - if (restyleManager->PostAnimationRestyles()) { - // Any style changes right now could trigger CSS Transitions. We don't - // want that to happen from SMIL-animated value of mapped attrs, so - // ignore animated value for now, and request an animation restyle to - // get our animated value noticed. - shell->RestyleForAnimation(this, - eRestyle_SVGAttrAnimations | eRestyle_ChangeAnimationPhase); - } - } else { - // Ok, this is an animation restyle -- go ahead and update/walk the - // animated content style rule. + RestyleManager* restyleManager = aRuleWalker->PresContext()->RestyleManager(); + if (!restyleManager->SkipAnimationRules()) { + // update/walk the animated content style rule. css::StyleRule* animContentStyleRule = GetAnimatedContentStyleRule(); if (!animContentStyleRule) { UpdateAnimatedContentStyleRule(); diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index 6c78d7beb1d..f105883cc95 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -70,8 +70,6 @@ RestyleManager::RestyleManager(nsPresContext* aPresContext) , mObservingRefreshDriver(false) , mInStyleRefresh(false) , mSkipAnimationRules(false) - , mPostAnimationRestyles(false) - , mIsProcessingAnimationStyleChange(false) , mHavePendingNonAnimationRestyles(false) , mHoverGeneration(0) , mRebuildAllExtraHint(nsChangeHint(0)) @@ -82,8 +80,6 @@ RestyleManager::RestyleManager(nsPresContext* aPresContext) , mReframingStyleContexts(nullptr) , mPendingRestyles(ELEMENT_HAS_PENDING_RESTYLE | ELEMENT_IS_POTENTIAL_RESTYLE_ROOT) - , mPendingAnimationRestyles(ELEMENT_HAS_PENDING_ANIMATION_RESTYLE | - ELEMENT_IS_POTENTIAL_ANIMATION_RESTYLE_ROOT) #ifdef DEBUG , mIsProcessingRestyles(false) #endif @@ -92,7 +88,6 @@ RestyleManager::RestyleManager(nsPresContext* aPresContext) #endif { mPendingRestyles.Init(this); - mPendingAnimationRestyles.Init(this); } void @@ -1572,11 +1567,6 @@ RestyleManager::StartRebuildAllStyleData(RestyleTracker& aRestyleTracker) mRebuildAllExtraHint = nsChangeHint(0); mRebuildAllRestyleHint = nsRestyleHint(0); - // Until we get rid of these phases in bug 960465, we need to add - // eRestyle_ChangeAnimationPhaseDescendants so that we actually honor - // these booleans in all cases. - restyleHint |= eRestyle_ChangeAnimationPhaseDescendants; - restyleHint |= eRestyle_ForceDescendants; if (!(restyleHint & eRestyle_Subtree) && @@ -1676,36 +1666,8 @@ RestyleManager::ProcessPendingRestyles() mPresContext->TransitionManager()->SetInAnimationOnlyStyleUpdate(true); } - // Until we get rid of these phases in bug 960465, we need to skip - // animation restyles during the non-animation phase, and post - // animation restyles so that we restyle those elements again in the - // animation phase. - mSkipAnimationRules = true; - mPostAnimationRestyles = true; - ProcessRestyles(mPendingRestyles); - mPostAnimationRestyles = false; - mSkipAnimationRules = false; - -#ifdef DEBUG - uint32_t oldPendingRestyleCount = mPendingRestyles.Count(); -#endif - - // ...and then process animation restyles. This needs to happen - // second because we need to start animations that resulted from the - // first set of restyles (e.g., CSS transitions with negative - // transition-delay), and because we need to immediately - // restyle-with-animation any just-restyled elements that are - // mid-transition (since processing the non-animation restyle ignores - // the running transition so it can check for a new change on the same - // property, and then posts an immediate animation style change). - MOZ_ASSERT(!mIsProcessingAnimationStyleChange, "nesting forbidden"); - mIsProcessingAnimationStyleChange = true; - ProcessRestyles(mPendingAnimationRestyles); - MOZ_ASSERT(mIsProcessingAnimationStyleChange, "nesting forbidden"); - mIsProcessingAnimationStyleChange = false; - if (!haveNonAnimation) { mPresContext->TransitionManager()->SetInAnimationOnlyStyleUpdate(false); } @@ -1713,9 +1675,6 @@ RestyleManager::ProcessPendingRestyles() #ifdef DEBUG mIsProcessingRestyles = false; #endif - NS_POSTCONDITION(mPendingRestyles.Count() == oldPendingRestyleCount, - "We should not have posted new non-animation restyles while " - "processing animation restyles"); NS_ASSERTION(haveNonAnimation || !mHavePendingNonAnimationRestyles, "should not have added restyles"); @@ -1822,12 +1781,12 @@ RestyleManager::UpdateOnlyAnimationStyles() } void -RestyleManager::PostRestyleEventCommon(Element* aElement, - nsRestyleHint aRestyleHint, - nsChangeHint aMinChangeHint, - bool aForAnimation) +RestyleManager::PostRestyleEvent(Element* aElement, + nsRestyleHint aRestyleHint, + nsChangeHint aMinChangeHint) { - if (MOZ_UNLIKELY(mPresContext->PresShell()->IsDestroying())) { + if (MOZ_UNLIKELY(!mPresContext) || + MOZ_UNLIKELY(mPresContext->PresShell()->IsDestroying())) { return; } @@ -1836,9 +1795,7 @@ RestyleManager::PostRestyleEventCommon(Element* aElement, return; } - RestyleTracker& tracker = - aForAnimation ? mPendingAnimationRestyles : mPendingRestyles; - tracker.AddPendingRestyle(aElement, aRestyleHint, aMinChangeHint); + mPendingRestyles.AddPendingRestyle(aElement, aRestyleHint, aMinChangeHint); // Set mHavePendingNonAnimationRestyles for any restyle that could // possibly contain non-animation styles. Unfortunately there's one diff --git a/layout/base/RestyleManager.h b/layout/base/RestyleManager.h index 13324ae150f..635033c4785 100644 --- a/layout/base/RestyleManager.h +++ b/layout/base/RestyleManager.h @@ -109,35 +109,12 @@ public: void IncrementAnimationGeneration() { ++mAnimationGeneration; } // Whether rule matching should skip styles associated with animation - bool SkipAnimationRules() const { - MOZ_ASSERT(mSkipAnimationRules || !mPostAnimationRestyles, - "inconsistent state"); - return mSkipAnimationRules; - } + bool SkipAnimationRules() const { return mSkipAnimationRules; } void SetSkipAnimationRules(bool aSkipAnimationRules) { mSkipAnimationRules = aSkipAnimationRules; } - // Whether rule matching should post animation restyles when it skips - // styles associated with animation. Only true when - // SkipAnimationRules() is also true. - bool PostAnimationRestyles() const { - MOZ_ASSERT(mSkipAnimationRules || !mPostAnimationRestyles, - "inconsistent state"); - return mPostAnimationRestyles; - } - - void SetPostAnimationRestyles(bool aPostAnimationRestyles) { - mPostAnimationRestyles = aPostAnimationRestyles; - } - - // Whether we're currently in the animation phase of restyle - // processing (to be eliminated in bug 960465) - bool IsProcessingAnimationStyleChange() const { - return mIsProcessingAnimationStyleChange; - } - /** * Reparent the style contexts of this frame subtree. The parent frame of * aFrame must be changed to the new parent before this function is called; @@ -347,24 +324,18 @@ public: void RebuildAllStyleData(nsChangeHint aExtraHint, nsRestyleHint aRestyleHint); - // See PostRestyleEventCommon below. + /** + * Notify the frame constructor that an element needs to have its + * style recomputed. + * @param aElement: The element to be restyled. + * @param aRestyleHint: Which nodes need to have selector matching run + * on them. + * @param aMinChangeHint: A minimum change hint for aContent and its + * descendants. + */ void PostRestyleEvent(Element* aElement, nsRestyleHint aRestyleHint, - nsChangeHint aMinChangeHint) - { - if (mPresContext) { - PostRestyleEventCommon(aElement, aRestyleHint, aMinChangeHint, - IsProcessingAnimationStyleChange()); - } - } - - // See PostRestyleEventCommon below. - void PostAnimationRestyleEvent(Element* aElement, - nsRestyleHint aRestyleHint, - nsChangeHint aMinChangeHint) - { - PostRestyleEventCommon(aElement, aRestyleHint, aMinChangeHint, true); - } + nsChangeHint aMinChangeHint); void PostRestyleEventForLazyConstruction() { @@ -382,25 +353,6 @@ public: #endif private: - /** - * Notify the frame constructor that an element needs to have its - * style recomputed. - * @param aElement: The element to be restyled. - * @param aRestyleHint: Which nodes need to have selector matching run - * on them. - * @param aMinChangeHint: A minimum change hint for aContent and its - * descendants. - * @param aForAnimation: Whether the style should be computed with or - * without animation data. Animation code - * sometimes needs to pass true; other code - * should generally pass the the pres context's - * IsProcessingAnimationStyleChange() value - * (which is the default value). - */ - void PostRestyleEventCommon(Element* aElement, - nsRestyleHint aRestyleHint, - nsChangeHint aMinChangeHint, - bool aForAnimation); void PostRestyleEventInternal(bool aForLazyConstruction); public: @@ -511,13 +463,6 @@ private: bool mInStyleRefresh : 1; // Whether rule matching should skip styles associated with animation bool mSkipAnimationRules : 1; - // Whether rule matching should post animation restyles when it skips - // styles associated with animation. Only true when - // mSkipAnimationRules is also true. - bool mPostAnimationRestyles : 1; - // Whether we're currently in the animation phase of restyle - // processing (to be eliminated in bug 960465) - bool mIsProcessingAnimationStyleChange : 1; bool mHavePendingNonAnimationRestyles : 1; uint32_t mHoverGeneration; @@ -535,7 +480,6 @@ private: ReframingStyleContexts* mReframingStyleContexts; RestyleTracker mPendingRestyles; - RestyleTracker mPendingAnimationRestyles; #ifdef DEBUG bool mIsProcessingRestyles; diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index 82f389ee7d0..6c5242a7e15 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -3117,8 +3117,12 @@ nsIPresShell::PostRecreateFramesFor(Element* aElement) void nsIPresShell::RestyleForAnimation(Element* aElement, nsRestyleHint aHint) { - mPresContext->RestyleManager()->PostAnimationRestyleEvent(aElement, aHint, - NS_STYLE_HINT_NONE); + // Now that we no longer have separate non-animation and animation + // restyles, this method having a distinct identity is less important, + // but it still seems useful to offer as a "more public" API and as a + // chokepoint for these restyles to go through. + mPresContext->RestyleManager()->PostRestyleEvent(aElement, aHint, + NS_STYLE_HINT_NONE); } void diff --git a/layout/style/AnimationCommon.cpp b/layout/style/AnimationCommon.cpp index 8c81d45b034..12a3e4ac3eb 100644 --- a/layout/style/AnimationCommon.cpp +++ b/layout/style/AnimationCommon.cpp @@ -373,13 +373,6 @@ CommonAnimationManager::GetAnimationRule(mozilla::dom::Element* aElement, RestyleManager* restyleManager = mPresContext->RestyleManager(); if (restyleManager->SkipAnimationRules()) { - // During the non-animation part of processing restyles, we don't - // add the animation rule. - - if (collection->mStyleRule && restyleManager->PostAnimationRestyles()) { - collection->PostRestyleForAnimation(mPresContext); - } - return nullptr; } diff --git a/layout/style/nsAnimationManager.cpp b/layout/style/nsAnimationManager.cpp index 6990cabc37a..3a070db78d9 100644 --- a/layout/style/nsAnimationManager.cpp +++ b/layout/style/nsAnimationManager.cpp @@ -224,142 +224,139 @@ nsIStyleRule* nsAnimationManager::CheckAnimationRule(nsStyleContext* aStyleContext, mozilla::dom::Element* aElement) { - // FIXME (bug 960465): This test should go away. - if (!mPresContext->RestyleManager()->IsProcessingAnimationStyleChange()) { - if (!mPresContext->IsDynamic()) { - // For print or print preview, ignore animations. - return nullptr; - } + if (!mPresContext->IsDynamic()) { + // For print or print preview, ignore animations. + return nullptr; + } - // Everything that causes our animation data to change triggers a - // style change, which in turn triggers a non-animation restyle. - // Likewise, when we initially construct frames, we're not in a - // style change, but also not in an animation restyle. + // Everything that causes our animation data to change triggers a + // style change, which in turn triggers a non-animation restyle. + // Likewise, when we initially construct frames, we're not in a + // style change, but also not in an animation restyle. - const nsStyleDisplay* disp = aStyleContext->StyleDisplay(); - AnimationPlayerCollection* collection = - GetAnimationPlayers(aElement, aStyleContext->GetPseudoType(), false); - if (!collection && - disp->mAnimationNameCount == 1 && - disp->mAnimations[0].GetName().IsEmpty()) { - return nullptr; - } + const nsStyleDisplay* disp = aStyleContext->StyleDisplay(); + AnimationPlayerCollection* collection = + GetAnimationPlayers(aElement, aStyleContext->GetPseudoType(), false); + if (!collection && + disp->mAnimationNameCount == 1 && + disp->mAnimations[0].GetName().IsEmpty()) { + return nullptr; + } - // build the animations list - dom::AnimationTimeline* timeline = aElement->OwnerDoc()->Timeline(); - AnimationPlayerPtrArray newPlayers; - BuildAnimations(aStyleContext, aElement, timeline, newPlayers); - - if (newPlayers.IsEmpty()) { - if (collection) { - collection->Destroy(); - } - return nullptr; - } + // build the animations list + dom::AnimationTimeline* timeline = aElement->OwnerDoc()->Timeline(); + AnimationPlayerPtrArray newPlayers; + BuildAnimations(aStyleContext, aElement, timeline, newPlayers); + if (newPlayers.IsEmpty()) { if (collection) { - collection->mStyleRule = nullptr; - collection->mStyleRuleRefreshTime = TimeStamp(); - collection->UpdateAnimationGeneration(mPresContext); + collection->Destroy(); + } + return nullptr; + } - // Copy over the start times and (if still paused) pause starts - // for each animation (matching on name only) that was also in the - // old list of animations. - // This means that we honor dynamic changes, which isn't what the - // spec says to do, but WebKit seems to honor at least some of - // them. See - // http://lists.w3.org/Archives/Public/www-style/2011Apr/0079.html - // In order to honor what the spec said, we'd copy more data over - // (or potentially optimize BuildAnimations to avoid rebuilding it - // in the first place). - if (!collection->mPlayers.IsEmpty()) { + if (collection) { + collection->mStyleRule = nullptr; + collection->mStyleRuleRefreshTime = TimeStamp(); + collection->UpdateAnimationGeneration(mPresContext); - for (size_t newIdx = newPlayers.Length(); newIdx-- != 0;) { - AnimationPlayer* newPlayer = newPlayers[newIdx]; + // Copy over the start times and (if still paused) pause starts + // for each animation (matching on name only) that was also in the + // old list of animations. + // This means that we honor dynamic changes, which isn't what the + // spec says to do, but WebKit seems to honor at least some of + // them. See + // http://lists.w3.org/Archives/Public/www-style/2011Apr/0079.html + // In order to honor what the spec said, we'd copy more data over + // (or potentially optimize BuildAnimations to avoid rebuilding it + // in the first place). + if (!collection->mPlayers.IsEmpty()) { - // Find the matching animation with this name in the old list - // of animations. We iterate through both lists in a backwards - // direction which means that if there are more animations in - // the new list of animations with a given name than in the old - // list, it will be the animations towards the of the beginning of - // the list that do not match and are treated as new animations. - nsRefPtr oldPlayer; - size_t oldIdx = collection->mPlayers.Length(); - while (oldIdx-- != 0) { - CSSAnimationPlayer* a = - collection->mPlayers[oldIdx]->AsCSSAnimationPlayer(); - MOZ_ASSERT(a, "All players in the CSS Animation collection should" - " be CSSAnimationPlayer objects"); - if (a->Name() == newPlayer->Name()) { - oldPlayer = a; - break; - } + for (size_t newIdx = newPlayers.Length(); newIdx-- != 0;) { + AnimationPlayer* newPlayer = newPlayers[newIdx]; + + // Find the matching animation with this name in the old list + // of animations. We iterate through both lists in a backwards + // direction which means that if there are more animations in + // the new list of animations with a given name than in the old + // list, it will be the animations towards the of the beginning of + // the list that do not match and are treated as new animations. + nsRefPtr oldPlayer; + size_t oldIdx = collection->mPlayers.Length(); + while (oldIdx-- != 0) { + CSSAnimationPlayer* a = + collection->mPlayers[oldIdx]->AsCSSAnimationPlayer(); + MOZ_ASSERT(a, "All players in the CSS Animation collection should" + " be CSSAnimationPlayer objects"); + if (a->Name() == newPlayer->Name()) { + oldPlayer = a; + break; } - if (!oldPlayer) { - continue; - } - - // Update the old from the new so we can keep the original object - // identity (and any expando properties attached to it). - if (oldPlayer->GetSource() && newPlayer->GetSource()) { - Animation* oldAnim = oldPlayer->GetSource(); - Animation* newAnim = newPlayer->GetSource(); - oldAnim->Timing() = newAnim->Timing(); - oldAnim->Properties() = newAnim->Properties(); - } - - // Reset compositor state so animation will be re-synchronized. - oldPlayer->ClearIsRunningOnCompositor(); - - // Handle changes in play state. - // CSSAnimationPlayer takes care of override behavior so that, - // for example, if the author has called pause(), that will - // override the animation-play-state. - // (We should check newPlayer->IsStylePaused() but that requires - // downcasting to CSSAnimationPlayer and we happen to know that - // newPlayer will only ever be paused by calling PauseFromStyle - // making IsPaused synonymous in this case.) - if (!oldPlayer->IsStylePaused() && newPlayer->IsPaused()) { - oldPlayer->PauseFromStyle(); - } else if (oldPlayer->IsStylePaused() && !newPlayer->IsPaused()) { - oldPlayer->PlayFromStyle(); - } - - // Replace new animation with the (updated) old one and remove the - // old one from the array so we don't try to match it any more. - // - // Although we're doing this while iterating this is safe because - // we're not changing the length of newPlayers and we've finished - // iterating over the list of old iterations. - newPlayer->Cancel(); - newPlayer = nullptr; - newPlayers.ReplaceElementAt(newIdx, oldPlayer); - collection->mPlayers.RemoveElementAt(oldIdx); } + if (!oldPlayer) { + continue; + } + + // Update the old from the new so we can keep the original object + // identity (and any expando properties attached to it). + if (oldPlayer->GetSource() && newPlayer->GetSource()) { + Animation* oldAnim = oldPlayer->GetSource(); + Animation* newAnim = newPlayer->GetSource(); + oldAnim->Timing() = newAnim->Timing(); + oldAnim->Properties() = newAnim->Properties(); + } + + // Reset compositor state so animation will be re-synchronized. + oldPlayer->ClearIsRunningOnCompositor(); + + // Handle changes in play state. + // CSSAnimationPlayer takes care of override behavior so that, + // for example, if the author has called pause(), that will + // override the animation-play-state. + // (We should check newPlayer->IsStylePaused() but that requires + // downcasting to CSSAnimationPlayer and we happen to know that + // newPlayer will only ever be paused by calling PauseFromStyle + // making IsPaused synonymous in this case.) + if (!oldPlayer->IsStylePaused() && newPlayer->IsPaused()) { + oldPlayer->PauseFromStyle(); + } else if (oldPlayer->IsStylePaused() && !newPlayer->IsPaused()) { + oldPlayer->PlayFromStyle(); + } + + // Replace new animation with the (updated) old one and remove the + // old one from the array so we don't try to match it any more. + // + // Although we're doing this while iterating this is safe because + // we're not changing the length of newPlayers and we've finished + // iterating over the list of old iterations. + newPlayer->Cancel(); + newPlayer = nullptr; + newPlayers.ReplaceElementAt(newIdx, oldPlayer); + collection->mPlayers.RemoveElementAt(oldIdx); } - } else { - collection = - GetAnimationPlayers(aElement, aStyleContext->GetPseudoType(), true); } - collection->mPlayers.SwapElements(newPlayers); - collection->mNeedsRefreshes = true; - collection->Tick(); + } else { + collection = + GetAnimationPlayers(aElement, aStyleContext->GetPseudoType(), true); + } + collection->mPlayers.SwapElements(newPlayers); + collection->mNeedsRefreshes = true; + collection->Tick(); - // Cancel removed animations - for (size_t newPlayerIdx = newPlayers.Length(); newPlayerIdx-- != 0; ) { - newPlayers[newPlayerIdx]->Cancel(); - } + // Cancel removed animations + for (size_t newPlayerIdx = newPlayers.Length(); newPlayerIdx-- != 0; ) { + newPlayers[newPlayerIdx]->Cancel(); + } - TimeStamp refreshTime = mPresContext->RefreshDriver()->MostRecentRefresh(); - UpdateStyleAndEvents(collection, refreshTime, - EnsureStyleRule_IsNotThrottled); - // We don't actually dispatch the mPendingEvents now. We'll either - // dispatch them the next time we get a refresh driver notification - // or the next time somebody calls - // nsPresShell::FlushPendingNotifications. - if (!mPendingEvents.IsEmpty()) { - mPresContext->Document()->SetNeedStyleFlush(); - } + TimeStamp refreshTime = mPresContext->RefreshDriver()->MostRecentRefresh(); + UpdateStyleAndEvents(collection, refreshTime, + EnsureStyleRule_IsNotThrottled); + // We don't actually dispatch the mPendingEvents now. We'll either + // dispatch them the next time we get a refresh driver notification + // or the next time somebody calls + // nsPresShell::FlushPendingNotifications. + if (!mPendingEvents.IsEmpty()) { + mPresContext->Document()->SetNeedStyleFlush(); } return GetAnimationRule(aElement, aStyleContext->GetPseudoType()); diff --git a/layout/style/nsHTMLCSSStyleSheet.cpp b/layout/style/nsHTMLCSSStyleSheet.cpp index 8a08d7ebbd2..7ee12e27e16 100644 --- a/layout/style/nsHTMLCSSStyleSheet.cpp +++ b/layout/style/nsHTMLCSSStyleSheet.cpp @@ -74,15 +74,7 @@ nsHTMLCSSStyleSheet::ElementRulesMatching(nsPresContext* aPresContext, rule = aElement->GetSMILOverrideStyleRule(); if (rule) { RestyleManager* restyleManager = aPresContext->RestyleManager(); - if (restyleManager->SkipAnimationRules()) { - // Non-animation restyle -- don't process SMIL override style, because we - // don't want SMIL animation to trigger new CSS transitions. Instead, - // request an Animation restyle, so we still get noticed. - if (restyleManager->PostAnimationRestyles()) { - aPresContext->PresShell()->RestyleForAnimation(aElement, - eRestyle_StyleAttribute | eRestyle_ChangeAnimationPhase); - } - } else { + if (!restyleManager->SkipAnimationRules()) { // Animation restyle (or non-restyle traversal of rules) // Now we can walk SMIL overrride style, without triggering transitions. rule->RuleMatched(); diff --git a/layout/style/nsStyleSet.cpp b/layout/style/nsStyleSet.cpp index bc64493728d..6c656e8a5c6 100644 --- a/layout/style/nsStyleSet.cpp +++ b/layout/style/nsStyleSet.cpp @@ -1658,15 +1658,12 @@ nsStyleSet::ResolveStyleWithoutAnimation(dom::Element* aTarget, bool oldSkipAnimationRules = restyleManager->SkipAnimationRules(); restyleManager->SetSkipAnimationRules(true); - bool oldPostAnimationRestyles = restyleManager->PostAnimationRestyles(); - restyleManager->SetPostAnimationRestyles(false); nsRefPtr result = ResolveStyleWithReplacement(aTarget, aStyleContext->GetParent(), aStyleContext, aWhichToRemove, eSkipStartingAnimations); - restyleManager->SetPostAnimationRestyles(oldPostAnimationRestyles); restyleManager->SetSkipAnimationRules(oldSkipAnimationRules); return result.forget(); @@ -2142,43 +2139,6 @@ nsStyleSet::GCRuleTrees() } } -/** - * Return an equivalent to aRuleNode with both animation and transition - * rules removed, and post a restyle if needed. - */ -static inline nsRuleNode* -SkipAnimationRules(nsRuleNode* aRuleNode, Element* aElementOrPseudoElement, - bool aPostAnimationRestyles) -{ - nsRuleNode* ruleNode = aRuleNode; - // The transition rule must be at the top of the cascade. - if (!ruleNode->IsRoot() && - ruleNode->GetLevel() == nsStyleSet::eTransitionSheet) { - ruleNode = ruleNode->GetParent(); - } - MOZ_ASSERT(ruleNode->IsRoot() || - ruleNode->GetLevel() != nsStyleSet::eTransitionSheet, - "can't have more than one transition rule"); - - // Use our existing ReplaceAnimationRule function to replace the - // animation rule, if present. - nsIStyleRule* animationRule = GetAnimationRule(ruleNode); - if (animationRule) { - ruleNode = ReplaceAnimationRule(ruleNode, animationRule, nullptr); - } - - if (ruleNode != aRuleNode && aPostAnimationRestyles) { - NS_ASSERTION(aElementOrPseudoElement, - "How can we have transition rules but no element?"); - // Need to do an animation restyle, just like - // nsTransitionManager::WalkTransitionRule and - // nsAnimationManager::GetAnimationRule would. - aRuleNode->PresContext()->PresShell()-> - RestyleForAnimation(aElementOrPseudoElement, eRestyle_Self); - } - return ruleNode; -} - already_AddRefed nsStyleSet::ReparentStyleContext(nsStyleContext* aStyleContext, nsStyleContext* aNewParentContext, @@ -2201,18 +2161,8 @@ nsStyleSet::ReparentStyleContext(nsStyleContext* aStyleContext, nsCSSPseudoElements::Type pseudoType = aStyleContext->GetPseudoType(); nsRuleNode* ruleNode = aStyleContext->RuleNode(); - // Skip transition rules as needed just like - // nsTransitionManager::WalkTransitionRule would. - RestyleManager* restyleManager = PresContext()->RestyleManager(); - bool skipAnimationRules = restyleManager->SkipAnimationRules(); - bool postAnimationRestyles = restyleManager->PostAnimationRestyles(); - if (skipAnimationRules) { - // Make sure that we're not using transition rules or animation rules for - // our new style context. If we need them, an animation restyle will - // provide. - ruleNode = SkipAnimationRules(ruleNode, aElementOrPseudoElement, - postAnimationRestyles); - } + NS_ASSERTION(!PresContext()->RestyleManager()->SkipAnimationRules(), + "we no longer handle SkipAnimationRules()"); nsRuleNode* visitedRuleNode = nullptr; nsStyleContext* visitedContext = aStyleContext->GetStyleIfVisited(); @@ -2221,14 +2171,7 @@ nsStyleSet::ReparentStyleContext(nsStyleContext* aStyleContext, // particular, it doesn't change whether this is a style context for // a link. if (visitedContext) { - visitedRuleNode = visitedContext->RuleNode(); - // Again, skip transition rules as needed - if (skipAnimationRules) { - // FIXME do something here for animations? - visitedRuleNode = - SkipAnimationRules(visitedRuleNode, aElementOrPseudoElement, - postAnimationRestyles); - } + visitedRuleNode = visitedContext->RuleNode(); } uint32_t flags = eNoFlags; diff --git a/layout/style/nsTransitionManager.cpp b/layout/style/nsTransitionManager.cpp index d6dc370689f..f07de83c910 100644 --- a/layout/style/nsTransitionManager.cpp +++ b/layout/style/nsTransitionManager.cpp @@ -192,13 +192,6 @@ nsTransitionManager::StyleContextChanged(dom::Element *aElement, // which causes us to return above). Thus we shouldn't do anything. return; } - - // FIXME (bug 960465): This test should go away. - if (newStyleContext->PresContext()->RestyleManager()-> - IsProcessingAnimationStyleChange()) { - return; - } - if (newStyleContext->GetParent() && newStyleContext->GetParent()->HasPseudoElementData()) { // Ignore transitions on things that inherit properties from diff --git a/layout/style/test/test_animations.html b/layout/style/test/test_animations.html index ec86765bd9d..8aa95fe02f6 100644 --- a/layout/style/test/test_animations.html +++ b/layout/style/test/test_animations.html @@ -1491,8 +1491,7 @@ dyn2.appendRule("50% { margin-left: 0px }"); is(cs.marginLeft, "50px", "dynamic rule change test, @keyframes appendRule"); var dyn2_kf1 = dyn2.cssRules[0]; // currently 0% { margin-left: 100px } dyn2_kf1.style.marginLeft = "-100px"; -// FIXME: Bug 978833 (keyframe rules used as nsIStyleRule but doesn't follow immutability contract) -todo_is(cs.marginLeft, "-50px", "dynamic rule change test, keyframe style set"); +is(cs.marginLeft, "-50px", "dynamic rule change test, keyframe style set"); dyn2.name = "dyn2"; is(cs.marginLeft, "25px", "dynamic rule change test, change in @keyframes name applies (second time)"); var dyn1_kf2 = dyn1.cssRules[1]; // currently 50% { margin-left: 50px } diff --git a/layout/style/test/test_animations_omta.html b/layout/style/test/test_animations_omta.html index 6f7caf34fee..0733aed6328 100644 --- a/layout/style/test/test_animations_omta.html +++ b/layout/style/test/test_animations_omta.html @@ -1758,24 +1758,8 @@ addAsyncAnimTest(function *() { var dyn2_kf1 = dyn2.cssRules[0]; dyn2_kf1.style.transform = "translate(-100px)"; yield waitForPaintsFlushed(); - // FIXME: Bug 978833 (keyframe rules used as nsIStyleRule but doesn't follow - // immutability contract) - var csTransform = window.getComputedStyle(gDiv).transform; - // omta_todo_is asserts that the OMTA style does NOT match the computed style - // but in this case we have a bug in the computed style so we assert it is - // broken and that the OMTA style is likewise broken. When bug 978833 is fixed - // we should replace the following two tests with: - // - // omta_is("transform", { tx: -50 }, RunningOn.Compositor, - // "dynamic rule change test, keyframe style set"); - todo_is(csTransform, "matrix(1, 0, 0, 1, -50, 0)", + omta_is("transform", { tx: -50 }, RunningOn.Compositor, "dynamic rule change test, keyframe style set"); - var brokenOMTATransform = convertTo3dMatrix( - SpecialPowers.DOMWindowUtils.getOMTAStyle(gDiv, "transform")); - var brokenComputedTransform = convertTo3dMatrix(csTransform); - ok(matricesRoughlyEqual(brokenOMTATransform, brokenComputedTransform), - "dynamic rule change test, keyframe style set" + - " - OMTA style is equally as broken as computed style"); dyn2.name = "dyn2"; yield waitForPaintsFlushed(); omta_is("transform", { tx: 25 }, RunningOn.Compositor,