Bug 822721: Call CalcStyleDifference and process the style change list resulting from the miniflush we do to update throttled animations prior to seeing if we need to start transitions. r=bzbarsky a=blocking-basecamp
authorL. David Baron <dbaron@dbaron.org>
Tue, 08 Jan 2013 20:37:29 -0800
changeset 128794 e349cfb9114f6124a600d811005c577b0f5eb5f3
parent 128793 7bce868864bf9d16ff59da3fb560b8016996ae44
child 128795 9a99dad5b25306aaf7279d5a9c1cd60589dc6ff0
push id317
push userbbajaj@mozilla.com
push dateTue, 07 May 2013 01:20:33 +0000
treeherdermozilla-release@159a10910249 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky, blocking-basecamp
bugs822721
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 822721: Call CalcStyleDifference and process the style change list resulting from the miniflush we do to update throttled animations prior to seeing if we need to start transitions. r=bzbarsky a=blocking-basecamp The CalcStyleDifference call is absolutely necessary even if we didn't need to process the change list, because it causes the new style context to have cached structs that we might need for a later comparison. This is important because, as an optimization, we only compare structs that have been retrieved. This optimization requires that when we replace a style context, we fetch all the structs on the new style context that had been fetched on the old style context (which is normally necessary anyway in order to do comparison so we can process the changes appropriately). However, actually processing the change list is also necessary to fix the bug; it's the actual change from the miniflush that matters here. Based on dholbert's debugging information, I think it's mostly likely because we were failing to process the UpdateOverflow hint.
layout/style/nsTransitionManager.cpp
layout/style/nsTransitionManager.h
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -23,16 +23,17 @@
 #include "nsEventDispatcher.h"
 #include "nsGUIEvent.h"
 #include "mozilla/dom/Element.h"
 #include "nsIFrame.h"
 #include "nsCSSFrameConstructor.h"
 #include "Layers.h"
 #include "FrameLayerBuilder.h"
 #include "nsDisplayList.h"
+#include "nsStyleChangeList.h"
 
 using mozilla::TimeStamp;
 using mozilla::TimeDuration;
 
 using namespace mozilla;
 using namespace mozilla::layers;
 using namespace mozilla::css;
 
@@ -246,17 +247,18 @@ ForceLayerRerendering(nsIFrame* aFrame, 
           aFrame, nsDisplayItem::TYPE_TRANSFORM)) {
       layer->RemoveUserData(nsIFrame::LayerIsPrerenderedDataKey());
     }
   }
 }
 
 nsStyleContext*
 nsTransitionManager::UpdateThrottledStyle(dom::Element* aElement,
-                                          nsStyleContext* aParentStyle)
+                                          nsStyleContext* aParentStyle,
+                                          nsStyleChangeList& aChangeList)
 {
   NS_ASSERTION(GetElementTransitions(aElement,
                                      nsCSSPseudoElements::ePseudo_NotPseudoElement,
                                      false), "element not transitioning");
 
   nsIFrame* primaryFrame = aElement->GetPrimaryFrame();
   if (!primaryFrame) {
     return nullptr;
@@ -278,64 +280,82 @@ nsTransitionManager::UpdateThrottledStyl
         mPresContext->AnimationManager()->GetElementAnimations(aElement,
                                                                oldStyle->GetPseudoType(),
                                                                false);
       NS_ASSERTION(ea, "Rule has level eAnimationSheet without animation on manager");
 
       mPresContext->AnimationManager()->EnsureStyleRuleFor(ea);
       curRule.mRule = ea->mStyleRule;
 
+      // FIXME: maybe not needed anymore:
       ForceLayerRerendering(primaryFrame, ea);
     } else if (curRule.mLevel == nsStyleSet::eTransitionSheet) {
       ElementTransitions *et =
         GetElementTransitions(aElement, oldStyle->GetPseudoType(), false);
       NS_ASSERTION(et, "Rule has level eTransitionSheet without transition on manager");
       
       et->EnsureStyleRuleFor(mPresContext->RefreshDriver()->MostRecentRefresh());
       curRule.mRule = et->mStyleRule;
 
+      // FIXME: maybe not needed anymore:
       ForceLayerRerendering(primaryFrame, et);
     } else {
       curRule.mRule = ruleNode->GetRule();
     }
 
     if (curRule.mRule) {
       rules.AppendElement(curRule);
     }
   } while ((ruleNode = ruleNode->GetParent()));
 
   nsRefPtr<nsStyleContext> newStyle = mPresContext->PresShell()->StyleSet()->
     ResolveStyleForRules(aParentStyle, oldStyle, rules);
+
+  // We absolutely must call CalcStyleDifference in order to ensure the
+  // new context has all the structs cached that the old context had.
+  // We also need it for processing of the changes.
+  nsChangeHint styleChange =
+    oldStyle->CalcStyleDifference(newStyle, nsChangeHint(0));
+  // This isn't particularly dangerous, but I want to catch if it happens:
+  NS_ABORT_IF_FALSE(NS_IsHintSubset(styleChange,
+                                    NS_CombineHint(nsChangeHint_UpdateOpacityLayer,
+                                      NS_CombineHint(nsChangeHint_UpdateTransformLayer,
+                                                     nsChangeHint_UpdateOverflow))),
+                    "unexpected change hint");
+  aChangeList.AppendChange(primaryFrame, primaryFrame->GetContent(),
+                           styleChange);
+
   primaryFrame->SetStyleContextWithoutNotification(newStyle);
 
   ReparentBeforeAndAfter(aElement, primaryFrame, newStyle, mPresContext->PresShell()->StyleSet());
 
   return newStyle;
 }
 
 void
 nsTransitionManager::UpdateThrottledStylesForSubtree(nsIContent* aContent,
-                                                     nsStyleContext* aParentStyle)
+                                                     nsStyleContext* aParentStyle,
+                                                     nsStyleChangeList& aChangeList)
 {
   dom::Element* element;
   if (aContent->IsElement()) {
     element = aContent->AsElement();
   } else {
     element = nullptr;
   }
 
   nsRefPtr<nsStyleContext> newStyle;
 
   ElementTransitions* et;
   if (element &&
       (et = GetElementTransitions(element,
                                   nsCSSPseudoElements::ePseudo_NotPseudoElement,
                                   false))) {
     // re-resolve our style
-    newStyle = UpdateThrottledStyle(element, aParentStyle);
+    newStyle = UpdateThrottledStyle(element, aParentStyle, aChangeList);
     // remove the current transition from the working set
     et->mFlushGeneration = mPresContext->RefreshDriver()->MostRecentRefresh();
   } else {
     // reparent the element's style
     nsStyleSet* styleSet = mPresContext->PresShell()->StyleSet();
     nsIFrame* primaryFrame = aContent->GetPrimaryFrame();
     if (!primaryFrame) {
       return;
@@ -346,17 +366,17 @@ nsTransitionManager::UpdateThrottledStyl
     primaryFrame->SetStyleContextWithoutNotification(newStyle);
     ReparentBeforeAndAfter(element, primaryFrame, newStyle, styleSet);
   }
 
   // walk the children
   if (newStyle) {
     for (nsIContent *child = aContent->GetFirstChild(); child;
          child = child->GetNextSibling()) {
-      UpdateThrottledStylesForSubtree(child, newStyle);
+      UpdateThrottledStylesForSubtree(child, newStyle, aChangeList);
     }
   }
 }
 
 void
 nsTransitionManager::UpdateAllThrottledStyles()
 {
   if (PR_CLIST_IS_EMPTY(&mElementData)) {
@@ -368,16 +388,18 @@ nsTransitionManager::UpdateAllThrottledS
   if (mPresContext->ThrottledStyleIsUpToDate()) {
     // throttled transitions are up to date, leave early
     return;
   }
 
   mPresContext->TickLastUpdateThrottledStyle();
   TimeStamp now = mPresContext->RefreshDriver()->MostRecentRefresh();
 
+  nsStyleChangeList changeList;
+
   // update each transitioning element by finding its root-most ancestor with a
   // transition, and flushing the style on that ancestor and all its descendants
   PRCList *next = PR_LIST_HEAD(&mElementData);
   while (next != &mElementData) {
     ElementTransitions* et = static_cast<ElementTransitions*>(next);
     next = PR_NEXT_LINK(next);
 
     if (et->mFlushGeneration == now) {
@@ -404,19 +426,22 @@ nsTransitionManager::UpdateAllThrottledS
         break;
       }
     }
 
     nsIFrame* primaryFrame;
     if (element &&
         (primaryFrame = element->GetPrimaryFrame())) {
       UpdateThrottledStylesForSubtree(element,
-        primaryFrame->GetStyleContext()->GetParent());
+        primaryFrame->GetStyleContext()->GetParent(), changeList);
     }
   }
+
+  mPresContext->PresShell()->FrameConstructor()->
+    ProcessRestyledFrames(changeList);
 }
 
 already_AddRefed<nsIStyleRule>
 nsTransitionManager::StyleContextChanged(dom::Element *aElement,
                                          nsStyleContext *aOldStyleContext,
                                          nsStyleContext *aNewStyleContext)
 {
   NS_PRECONDITION(aOldStyleContext->GetPseudo() ==
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -215,17 +215,19 @@ private:
                                             bool aCreateIfNeeded);
   void WalkTransitionRule(ElementDependentRuleProcessorData* aData,
                           nsCSSPseudoElements::Type aPseudoType);
 
   // Update the animated styles of an element and its descendants.
   // If the element has a transition, it is flushed back to its primary frame.
   // If the element does not have a transition, then its style is reparented.
   void UpdateThrottledStylesForSubtree(nsIContent* aContent,
-                                       nsStyleContext* aParentStyle);
+                                       nsStyleContext* aParentStyle,
+                                       nsStyleChangeList &aChangeList);
   // Update the style on aElement from the transition stored in this manager and
   // the new parent style - aParentStyle. aElement must be transitioning or
   // animated. Returns the updated style.
   nsStyleContext* UpdateThrottledStyle(mozilla::dom::Element* aElement,
-                                       nsStyleContext* aParentStyle);
+                                       nsStyleContext* aParentStyle,
+                                       nsStyleChangeList &aChangeList);
 };
 
 #endif /* !defined(nsTransitionManager_h_) */