Make ReResolveStyleContext no longer rerun selector matching on next-continuations by making both ReResolveStyleContext and ReParentStyleContext enforce the invariant that all continuations have the same style context. (Bug 522563) r=bzbarsky
authorL. David Baron <dbaron@dbaron.org>
Sat, 17 Oct 2009 10:31:47 -0400
changeset 33987 886ab0434035bec4f7d7c152c2b523e45e067066
parent 33986 4f3a789f3c3ff341d481927c57ab50304742bb35
child 33988 f8a2075294dc852a6140c0aa6219dc699ad82421
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs522563
milestone1.9.3a1pre
Make ReResolveStyleContext no longer rerun selector matching on next-continuations by making both ReResolveStyleContext and ReParentStyleContext enforce the invariant that all continuations have the same style context. (Bug 522563) r=bzbarsky
layout/base/nsFrameManager.cpp
layout/style/nsTransitionManager.cpp
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -990,28 +990,70 @@ nsFrameManager::ReParentStyleContext(nsI
                     "Shouldn't happen!");
     }
     // XXX need to do something here to produce the correct style context
     // for an IB split whose first inline part is inside a first-line frame.
     // Currently the IB anonymous block's style context takes the first part's
     // style context as parent, which is wrong since first-line style should
     // not apply to the anonymous block.
 
-    newContext = mStyleSet->ReParentStyleContext(presContext, oldContext,
-                                                 newParentContext);
+#ifdef DEBUG
+    {
+      // Check that our assumption that continuations of the same
+      // pseudo-type and with the same style context parent have the
+      // same style context is valid before the reresolution.  (We need
+      // to check the pseudo-type and style context parent because of
+      // :first-letter and :first-line, where we create styled and
+      // unstyled letter/line frames distinguished by pseudo-type, and
+      // then need to distinguish their descendants based on having
+      // different parents.)
+      nsIFrame *nextContinuation = aFrame->GetNextContinuation();
+      if (nextContinuation) {
+        nsStyleContext *nextContinuationContext =
+          nextContinuation->GetStyleContext();
+        NS_ASSERTION(oldContext == nextContinuationContext ||
+                     oldContext->GetPseudoType() !=
+                       nextContinuationContext->GetPseudoType() ||
+                     oldContext->GetParent() !=
+                       nextContinuationContext->GetParent(),
+                     "continuations should have the same style context");
+      }
+    }
+#endif
+
+    nsIFrame *prevContinuation = aFrame->GetPrevContinuation();
+    nsStyleContext *prevContinuationContext;
+    PRBool copyFromContinuation =
+      prevContinuation &&
+      (prevContinuationContext = prevContinuation->GetStyleContext())
+        ->GetPseudoType() == oldContext->GetPseudoType() &&
+       prevContinuationContext->GetParent() == newParentContext;
+    if (copyFromContinuation) {
+      // Just use the style context from the frame's previous
+      // continuation (see assertion about aFrame->GetNextContinuation()
+      // above, which we would have previously hit for aFrame's previous
+      // continuation).
+      newContext = prevContinuationContext;
+    } else {
+      newContext = mStyleSet->ReParentStyleContext(presContext, oldContext,
+                                                   newParentContext);
+    }
+
     if (newContext) {
       if (newContext != oldContext) {
         // We probably don't want to initiate transitions from
         // ReParentStyleContext, since we call it during frame
         // construction rather than in response to dynamic changes.
         // Also see the comment at the start of
         // nsTransitionManager::ConsiderStartingTransition.
 #if 0
-        TryStartingTransition(presContext, aFrame->GetContent(),
-                              oldContext, &newContext);
+        if (!copyFromContinuation) {
+          TryStartingTransition(presContext, aFrame->GetContent(),
+                                oldContext, &newContext);
+        }
 #endif
 
         // Make sure to call CalcStyleDifference so that the new context ends
         // up resolving all the structs the old context resolved.
         nsChangeHint styleChange = oldContext->CalcStyleDifference(newContext);
         // The style change is always 0 because we have the same rulenode and
         // CalcStyleDifference optimizes us away.  That's OK, though:
         // reparenting should never trigger a frame reconstruct, and whenever
@@ -1231,20 +1273,58 @@ nsFrameManager::ReResolveStyleContext(ns
 
       // The provider's new context becomes the parent context of
       // aFrame's context.
       parentContext = providerFrame->GetStyleContext();
       // Set |resolvedChild| so we don't bother resolving the
       // provider again.
       resolvedChild = providerFrame;
     }
-    
+
+#ifdef DEBUG
+    {
+      // Check that our assumption that continuations of the same
+      // pseudo-type and with the same style context parent have the
+      // same style context is valid before the reresolution.  (We need
+      // to check the pseudo-type and style context parent because of
+      // :first-letter and :first-line, where we create styled and
+      // unstyled letter/line frames distinguished by pseudo-type, and
+      // then need to distinguish their descendants based on having
+      // different parents.)
+      nsIFrame *nextContinuation = aFrame->GetNextContinuation();
+      if (nextContinuation) {
+        nsStyleContext *nextContinuationContext =
+          nextContinuation->GetStyleContext();
+        NS_ASSERTION(oldContext == nextContinuationContext ||
+                     oldContext->GetPseudoType() !=
+                       nextContinuationContext->GetPseudoType() ||
+                     oldContext->GetParent() !=
+                       nextContinuationContext->GetParent(),
+                     "continuations should have the same style context");
+      }
+    }
+#endif
+
     // do primary context
     nsRefPtr<nsStyleContext> newContext;
-    if (pseudoTag == nsCSSAnonBoxes::mozNonElement) {
+    nsIFrame *prevContinuation = aFrame->GetPrevContinuation();
+    nsStyleContext *prevContinuationContext;
+    PRBool copyFromContinuation =
+      prevContinuation &&
+      (prevContinuationContext = prevContinuation->GetStyleContext())
+        ->GetPseudoType() == oldContext->GetPseudoType() &&
+       prevContinuationContext->GetParent() == parentContext;
+    if (copyFromContinuation) {
+      // Just use the style context from the frame's previous
+      // continuation (see assertion about aFrame->GetNextContinuation()
+      // above, which we would have previously hit for aFrame's previous
+      // continuation).
+      newContext = prevContinuationContext;
+    }
+    else if (pseudoTag == nsCSSAnonBoxes::mozNonElement) {
       NS_ASSERTION(localContent,
                    "non pseudo-element frame without content node");
       newContext = styleSet->ResolveStyleForNonElement(parentContext);
     }
     else if (pseudoTag) {
       // XXXldb This choice of pseudoContent seems incorrect for anon
       // boxes and perhaps other cases.
       // See also the comment above the assertion at the start of this
@@ -1293,18 +1373,20 @@ nsFrameManager::ReResolveStyleContext(ns
           // we can use FindChildWithRules to keep a lot of the old
           // style contexts around.  However, we need to start from the
           // same root.
           newContext = oldContext;
         }
       }
 
       if (newContext != oldContext) {
-        TryStartingTransition(aPresContext, aFrame->GetContent(),
-                              oldContext, &newContext);
+        if (!copyFromContinuation) {
+          TryStartingTransition(aPresContext, aFrame->GetContent(),
+                                oldContext, &newContext);
+        }
 
         aMinChange = CaptureChange(oldContext, newContext, aFrame,
                                    content, aChangeList, aMinChange,
                                    assumeDifferenceHint);
         if (!(aMinChange & nsChangeHint_ReconstructFrame)) {
           // if frame gets regenerated, let it keep old context
           aFrame->SetStyleContext(newContext);
         }
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -377,22 +377,16 @@ nsTransitionManager::StyleContextChanged
   if (aNewStyleContext->GetParent() &&
       aNewStyleContext->GetParent()->HasPseudoElementData()) {
     // Ignore transitions on things that inherit properties from
     // pseudo-elements.
     // FIXME (Bug 522599): Add tests for this.
     return nsnull;
   }
 
-  // FIXME (Bug 522563): When we have multiple continuations, we
-  // actually repeat this for each one, and if we have transitions we
-  // create separate cover rules for each one.  However, since we're
-  // attaching the transition data to the element, during the animation
-  // we create the same style rule, so it's not too horrible.
-
   // Per http://lists.w3.org/Archives/Public/www-style/2009Aug/0109.html
   // I'll consider only the transitions from the number of items in
   // 'transition-property' on down, and later ones will override earlier
   // ones (tracked using |whichStarted|).
   PRBool startedAny = PR_FALSE;
   nsCSSPropertySet whichStarted;
   ElementTransitions *et = nsnull;
   for (PRUint32 i = disp->mTransitionPropertyCount; i-- != 0; ) {