Bug 494117 part 2. Don't force selector matching on the whole subtree rooted at an element when the element's style changes. r=dbaron

layout/base/RestyleTracker.cpp

@@ -143,9 +143,10 @@ RestyleTracker::ProcessOneRestyle(Element* aElement,
                   "Element has unexpected document");
 
   nsIFrame* primaryFrame = aElement->GetPrimaryFrame();
-  if (aRestyleHint & eRestyle_Subtree) {
+  if (aRestyleHint & (eRestyle_Self | eRestyle_Subtree)) {
     mFrameConstructor->RestyleElement(aElement, primaryFrame, aChangeHint,
-                                      *this);
+                                      *this,
+                                      (aRestyleHint & eRestyle_Subtree) != 0);
   } else if (aChangeHint &&
              (primaryFrame ||
               (aChangeHint & nsChangeHint_ReconstructFrame))) {

layout/base/RestyleTracker.h

@@ -196,7 +196,7 @@ inline PRBool RestyleTracker::AddPendingRestyle(Element* aElement,
   // We can only treat this element as a restyle root if we would
   // actually restyle its descendants (so either call
   // ReResolveStyleContext on it or just reframe it).
-  if ((aRestyleHint & eRestyle_Subtree) ||
+  if ((aRestyleHint & (eRestyle_Self | eRestyle_Subtree)) ||
       (aMinChangeHint & nsChangeHint_ReconstructFrame)) {
     for (const Element* cur = aElement; !cur->HasFlag(RootBit()); ) {
       nsIContent* parent = cur->GetFlattenedTreeParent();

layout/base/nsCSSFrameConstructor.cpp

@@ -8102,7 +8102,8 @@ void
 nsCSSFrameConstructor::RestyleElement(Element        *aElement,
                                       nsIFrame       *aPrimaryFrame,
                                       nsChangeHint   aMinHint,
-                                      RestyleTracker& aRestyleTracker)
+                                      RestyleTracker& aRestyleTracker,
+                                      PRBool          aRestyleDescendants)
 {
   NS_ASSERTION(aPrimaryFrame == aElement->GetPrimaryFrame(),
                "frame/content mismatch");
@@ -8120,7 +8121,7 @@ nsCSSFrameConstructor::RestyleElement(Element        *aElement,
     nsStyleChangeList changeList;
     mPresShell->FrameManager()->
       ComputeStyleChangeFor(aPrimaryFrame, &changeList, aMinHint,
-                            aRestyleTracker);
+                            aRestyleTracker, aRestyleDescendants);
     ProcessRestyledFrames(changeList);
   } else {
     // no frames, reconstruct for content
@@ -11356,8 +11357,7 @@ void
 nsCSSFrameConstructor::RestyleForEmptyChange(Element* aContainer)
 {
   // In some cases (:empty + E, :empty ~ E), a change if the content of
-  // an element requires restyling its grandparent, because it changes
+  // an element requires restyling its parent's siblings.
-  // its parent's :empty state.
   nsRestyleHint hint = eRestyle_Subtree;
   nsIContent* grandparent = aContainer->GetParent();
   if (grandparent &&
@@ -11662,7 +11662,7 @@ nsCSSFrameConstructor::RebuildAllStyleData(nsChangeHint aExtraHint)
   // Note: The restyle tracker we pass in here doesn't matter.
   mPresShell->FrameManager()->ComputeStyleChangeFor(mPresShell->GetRootFrame(),
                                                     &changeList, aExtraHint,
-                                                    mPendingRestyles);
+                                                    mPendingRestyles, PR_TRUE);
   // Process the required changes
   ProcessRestyledFrames(changeList);
   // Tell the style set it's safe to destroy the old rule tree.  We

layout/base/nsCSSFrameConstructor.h

@@ -453,7 +453,8 @@ private:
   void RestyleElement(Element* aElement,
                       nsIFrame*       aPrimaryFrame,
                       nsChangeHint    aMinHint,
-                      RestyleTracker& aRestyleTracker);
+                      RestyleTracker& aRestyleTracker,
+                      PRBool          aRestyleDescendants);
 
   nsresult InitAndRestoreFrame (const nsFrameConstructorState& aState,
                                 nsIContent*                    aContent,

layout/base/nsChangeHint.h

@@ -139,13 +139,14 @@ inline PRBool NS_IsHintSubset(nsChangeHint aSubset, nsChangeHint aSuperSet) {
   nsChangeHint(NS_STYLE_HINT_REFLOW | nsChangeHint_ReconstructFrame)
 
 /**
- * |nsRestyleHint| is a bitfield for the result of |HasStateDependentStyle|
+ * |nsRestyleHint| is a bitfield for the result of
- * and |HasAttributeDependentStyle|.  All values have an implied "and
+ * |HasStateDependentStyle| and |HasAttributeDependentStyle|.  When no
- * descendants."  When no restyling is necessary, use |nsRestyleHint(0)|.
+ * restyling is necessary, use |nsRestyleHint(0)|.
  */
 enum nsRestyleHint {
-  eRestyle_Subtree = 0x1,
+  eRestyle_Self = 0x1,
-  eRestyle_LaterSiblings = 0x2
+  eRestyle_Subtree = 0x2, /* self and descendants */
+  eRestyle_LaterSiblings = 0x4 /* implies "and descendants" */
 };
 
 

layout/base/nsFrameManager.cpp

@@ -962,6 +962,7 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
                                       nsIContent        *aParentContent,
                                       nsStyleChangeList *aChangeList, 
                                       nsChangeHint       aMinChange,
+                                      nsRestyleHint      aRestyleHint,
                                       PRBool             aFireAccessibilityEvents,
                                       RestyleTracker&    aRestyleTracker)
 {
@@ -1029,9 +1030,16 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
         if (NS_UpdateHint(aMinChange, restyleData.mChangeHint)) {
           aChangeList->AppendChange(aFrame, content, restyleData.mChangeHint);
         }
+        aRestyleHint = nsRestyleHint(aRestyleHint | restyleData.mRestyleHint);
       }
     }
 
+    nsRestyleHint childRestyleHint = aRestyleHint;
+
+    if (childRestyleHint == eRestyle_Self) {
+      childRestyleHint = nsRestyleHint(0);
+    }
+
     nsStyleContext* parentContext;
     nsIFrame* resolvedChild = nsnull;
     // Get the frame providing the parent style context.  If it is a
@@ -1060,8 +1068,8 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
       // frame because it is visible or hidden withitn this frame.
       assumeDifferenceHint = ReResolveStyleContext(aPresContext, providerFrame,
                                                    aParentContent, aChangeList,
-                                                   aMinChange, PR_FALSE,
+                                                   aMinChange, aRestyleHint,
-                                                   aRestyleTracker);
+                                                   PR_FALSE, aRestyleTracker);
 
       // The provider's new context becomes the parent context of
       // aFrame's context.
@@ -1111,6 +1119,9 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
       // continuation).
       newContext = prevContinuationContext;
     }
+    else if (!aRestyleHint) {
+      newContext = styleSet->ReparentStyleContext(oldContext, parentContext);
+    }
     else if (pseudoTag == nsCSSAnonBoxes::mozNonElement) {
       NS_ASSERTION(localContent,
                    "non pseudo-element frame without content node");
@@ -1162,6 +1173,7 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
                    "non pseudo-element frame without content node");
       newContext = styleSet->ResolveStyleFor(content->AsElement(), parentContext);
     }
+
     NS_ASSERTION(newContext, "failed to get new style context");
     if (newContext) {
       if (!parentContext) {
@@ -1197,6 +1209,8 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
     }
 
     // do additional contexts
+    // XXXbz might be able to avoid selector matching here in some
+    // cases; won't worry about it for now.
     PRInt32 contextIndex = -1;
     while (1 == 1) {
       nsStyleContext* oldExtraContext = nsnull;
@@ -1265,16 +1279,22 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
         NS_ASSERTION(!undisplayed->mStyle->GetPseudo(),
                      "Shouldn't have random pseudo style contexts in the "
                      "undisplayed map");
+        nsRestyleHint thisChildHint = childRestyleHint;
         RestyleTracker::RestyleData undisplayedRestyleData;
         if (aRestyleTracker.GetRestyleData(undisplayed->mContent->AsElement(),
                                            &undisplayedRestyleData)) {
-          // Nothing to do with it for now; when we don't
+          thisChildHint =
-          // automatically restyle our kids we'll need to handle that
+            nsRestyleHint(thisChildHint | undisplayedRestyleData.mRestyleHint);
-          // here.  We do want the GetRestyleData call, though, to
+        }
-          // preserve the restyle tracker's invariants.
+        nsRefPtr<nsStyleContext> undisplayedContext;
+        if (thisChildHint) {
+          undisplayedContext =
+            styleSet->ResolveStyleFor(undisplayed->mContent->AsElement(),
+                                      newContext);
+        } else {
+          undisplayedContext =
+            styleSet->ReparentStyleContext(undisplayed->mStyle, newContext);
         }
-        nsRefPtr<nsStyleContext> undisplayedContext =
-          styleSet->ResolveStyleFor(undisplayed->mContent->AsElement(), newContext);
         if (undisplayedContext) {
           const nsStyleDisplay* display = undisplayedContext->GetStyleDisplay();
           if (display->mDisplay != NS_STYLE_DISPLAY_NONE) {
@@ -1410,6 +1430,7 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
                                     content, aChangeList,
                                     NS_SubtractHint(aMinChange,
                                                     nsChangeHint_ReflowFrame),
+                                    childRestyleHint,
                                     fireAccessibilityEvents,
                                     aRestyleTracker);
 
@@ -1417,6 +1438,7 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
               // as the out-of-flow frame
               ReResolveStyleContext(aPresContext, child, content,
                                     aChangeList, aMinChange,
+                                    childRestyleHint,
                                     fireAccessibilityEvents,
                                     aRestyleTracker);
             }
@@ -1424,6 +1446,7 @@ nsFrameManager::ReResolveStyleContext(nsPresContext     *aPresContext,
               if (child != resolvedChild) {
                 ReResolveStyleContext(aPresContext, child, content,
                                       aChangeList, aMinChange,
+                                      childRestyleHint,
                                       fireAccessibilityEvents,
                                       aRestyleTracker);
               } else {
@@ -1448,7 +1471,8 @@ void
 nsFrameManager::ComputeStyleChangeFor(nsIFrame          *aFrame, 
                                       nsStyleChangeList *aChangeList,
                                       nsChangeHint       aMinChange,
-                                      RestyleTracker&    aRestyleTracker)
+                                      RestyleTracker&    aRestyleTracker,
+                                      PRBool             aRestyleDescendants)
 {
   if (aMinChange) {
     aChangeList->AppendChange(aFrame, aFrame->GetContent(), aMinChange);
@@ -1473,7 +1497,10 @@ nsFrameManager::ComputeStyleChangeFor(nsIFrame          *aFrame,
       // Inner loop over next-in-flows of the current frame
       nsChangeHint frameChange =
         ReResolveStyleContext(GetPresContext(), frame, nsnull,
-                              aChangeList, topLevelChange, PR_TRUE,
+                              aChangeList, topLevelChange,
+                              aRestyleDescendants ?
+                                eRestyle_Subtree : eRestyle_Self,
+                              PR_TRUE,
                               aRestyleTracker);
       NS_UpdateHint(topLevelChange, frameChange);
 

layout/base/nsFrameManager.h

@@ -160,7 +160,8 @@ public:
     ComputeStyleChangeFor(nsIFrame* aFrame,
                           nsStyleChangeList* aChangeList,
                           nsChangeHint aMinChange,
-                          RestyleTracker& aRestyleTracker);
+                          RestyleTracker& aRestyleTracker,
+                          PRBool aRestyleDescendants);
 
   /*
    * Capture/restore frame state for the frame subtree rooted at aFrame.
@@ -201,12 +202,18 @@ public:
   }
 
 private:
+  // Use eRestyle_Self for the aRestyleHint argument to mean
+  // "reresolve our style context but not kids", use eRestyle_Subtree
+  // to mean "reresolve our style context and kids", and use
+  // nsRestyleHint(0) to mean recompute a new style context for our
+  // current parent and existing rulenode, and the same for kids.
   NS_HIDDEN_(nsChangeHint)
     ReResolveStyleContext(nsPresContext    *aPresContext,
                           nsIFrame          *aFrame,
                           nsIContent        *aParentContent,
                           nsStyleChangeList *aChangeList, 
                           nsChangeHint       aMinChange,
+                          nsRestyleHint      aRestyleHint,
                           PRBool             aFireAccessibilityEvents,
                           RestyleTracker&    aRestyleTracker);
 };

layout/base/nsPresShell.cpp

@@ -3626,14 +3626,19 @@ PresShell::RecreateFramesFor(nsIContent* aContent)
 void
 nsIPresShell::PostRecreateFramesFor(Element* aElement)
 {
-  FrameConstructor()->PostRestyleEvent(aElement, eRestyle_Subtree,
+  FrameConstructor()->PostRestyleEvent(aElement, nsRestyleHint(0),
                                        nsChangeHint_ReconstructFrame);
 }
 
 void
 nsIPresShell::RestyleForAnimation(Element* aElement)
 {
-  FrameConstructor()->PostAnimationRestyleEvent(aElement, eRestyle_Subtree,
+  // eRestyle_Self is ok here because animations are always tied to a
+  // particular element and don't directly affect its kids.  The kids
+  // might have animations of their own, or inherit from aElement, but
+  // we handle all that during restyling; we don't need to _force_
+  // animation rule matching on the kids here.
+  FrameConstructor()->PostAnimationRestyleEvent(aElement, eRestyle_Self,
                                                 NS_STYLE_HINT_NONE);
 }
 

layout/style/Makefile.in

@@ -147,6 +147,9 @@ FORCE_STATIC_LIB = 1
 include $(topsrcdir)/config/rules.mk
 
 LOCAL_INCLUDES	= \
+		-I$(srcdir)/../base \
+		-I$(srcdir)/../generic \
+		-I$(srcdir)/../xul/base/src \
 		-I$(srcdir)/../../content/base/src \
 		-I$(srcdir)/../../content/html/content/src \
 		-I$(srcdir)/../../content/xbl/src \

layout/style/nsCSSRuleProcessor.cpp

@@ -2329,10 +2329,17 @@ nsCSSRuleProcessor::RulesMatching(XULTreeRuleProcessorData* aData)
 }
 #endif
 
-inline PRBool
+static inline nsRestyleHint RestyleHintForOp(PRUnichar oper)
-IsSiblingOperator(PRUnichar oper)
 {
-  return oper == PRUnichar('+') || oper == PRUnichar('~');
+  if (oper == PRUnichar('+') || oper == PRUnichar('~')) {
+    return eRestyle_LaterSiblings;
+  }
+
+  if (oper != PRUnichar(0)) {
+    return eRestyle_Subtree;
+  }
+
+  return eRestyle_Self;
 }
 
 nsRestyleHint
@@ -2355,8 +2362,7 @@ nsCSSRuleProcessor::HasStateDependentStyle(StateRuleProcessorData* aData)
     for(; iter != end; ++iter) {
       nsCSSSelector* selector = *iter;
 
-      nsRestyleHint possibleChange = IsSiblingOperator(selector->mOperator) ?
+      nsRestyleHint possibleChange = RestyleHintForOp(selector->mOperator);
-        eRestyle_LaterSiblings : eRestyle_Subtree;
 
       // If hint already includes all the bits of possibleChange,
       // don't bother calling SelectorMatches, since even if it returns false
@@ -2397,8 +2403,7 @@ AttributeEnumFunc(nsCSSSelector* aSelector, AttributeEnumData* aData)
 {
   AttributeRuleProcessorData *data = aData->data;
 
-  nsRestyleHint possibleChange = IsSiblingOperator(aSelector->mOperator) ?
+  nsRestyleHint possibleChange = RestyleHintForOp(aSelector->mOperator);
-    eRestyle_LaterSiblings : eRestyle_Subtree;
 
   // If enumData->change already includes all the bits of possibleChange, don't
   // bother calling SelectorMatches, since even if it returns false

layout/style/nsHTMLCSSStyleSheet.cpp

@@ -156,7 +156,7 @@ nsHTMLCSSStyleSheet::HasAttributeDependentStyle(AttributeRuleProcessorData* aDat
   // Perhaps should check that it's XUL, SVG, (or HTML) namespace, but
   // it doesn't really matter.
   if (aData->mAttrHasChanged && aData->mAttribute == nsGkAtoms::style) {
-    return eRestyle_Subtree;
+    return eRestyle_Self;
   }
 
   return nsRestyleHint(0);

layout/style/nsHTMLStyleSheet.cpp

@@ -71,6 +71,7 @@
 #include "nsContentErrors.h"
 #include "nsRuleProcessorData.h"
 #include "mozilla/dom/Element.h"
+#include "nsCSSFrameConstructor.h"
 
 using namespace mozilla::dom;
 
@@ -279,7 +280,7 @@ nsHTMLStyleSheet::HasStateDependentStyle(StateRuleProcessorData* aData)
       ((mActiveRule && (aData->mStateMask & NS_EVENT_STATE_ACTIVE)) ||
        (mLinkRule && (aData->mStateMask & NS_EVENT_STATE_VISITED)) ||
        (mVisitedRule && (aData->mStateMask & NS_EVENT_STATE_VISITED)))) {
-    return eRestyle_Subtree;
+    return eRestyle_Self;
   }
   
   return nsRestyleHint(0);
@@ -309,7 +310,7 @@ nsHTMLStyleSheet::HasAttributeDependentStyle(AttributeRuleProcessorData* aData)
       (mLinkRule || mVisitedRule || mActiveRule) &&
       element->IsHTML() &&
       aData->mContentTag == nsGkAtoms::a) {
-    return eRestyle_Subtree;
+    return eRestyle_Self;
   }
 
   // Don't worry about the mDocumentColorRule since it only applies
@@ -317,7 +318,7 @@ nsHTMLStyleSheet::HasAttributeDependentStyle(AttributeRuleProcessorData* aData)
 
   // Handle the content style rules.
   if (element->IsAttributeMapped(aData->mAttribute)) {
-    return eRestyle_Subtree;
+    return eRestyle_Self;
   }
 
   return nsRestyleHint(0);
@@ -455,52 +456,46 @@ nsHTMLStyleSheet::Reset(nsIURI* aURL)
 }
 
 nsresult
-nsHTMLStyleSheet::SetLinkColor(nscolor aColor)
+nsHTMLStyleSheet::ImplLinkColorSetter(nsRefPtr<HTMLColorRule>& aRule, nscolor aColor)
 {
-  if (mLinkRule) {
+  if (aRule && aRule->mColor == aColor) {
-    if (mLinkRule->mColor == aColor)
       return NS_OK;
   }
 
-  mLinkRule = new HTMLColorRule();
+  aRule = new HTMLColorRule();
-  if (!mLinkRule)
+  if (!aRule)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  mLinkRule->mColor = aColor;
+  aRule->mColor = aColor;
+  // Now make sure we restyle any links that might need it.  This
+  // shouldn't happen often, so just rebuilding everything is ok.
+  if (mDocument && mDocument->GetPrimaryShell()) {
+    Element* root = mDocument->GetRootElement();
+    if (root) {
+      mDocument->GetPrimaryShell()->FrameConstructor()->
+        PostRestyleEvent(root, eRestyle_Subtree, NS_STYLE_HINT_NONE);
+    }
+  }
   return NS_OK;
 }
 
+nsresult
+nsHTMLStyleSheet::SetLinkColor(nscolor aColor)
+{
+  return ImplLinkColorSetter(mLinkRule, aColor);
+}
+
 
 nsresult
 nsHTMLStyleSheet::SetActiveLinkColor(nscolor aColor)
 {
-  if (mActiveRule) {
+  return ImplLinkColorSetter(mActiveRule, aColor);
-    if (mActiveRule->mColor == aColor)
-      return NS_OK;
-  }
-
-  mActiveRule = new HTMLColorRule();
-  if (!mActiveRule)
-    return NS_ERROR_OUT_OF_MEMORY;
-
-  mActiveRule->mColor = aColor;
-  return NS_OK;
 }
 
 nsresult
 nsHTMLStyleSheet::SetVisitedLinkColor(nscolor aColor)
 {
-  if (mVisitedRule) {
+  return ImplLinkColorSetter(mVisitedRule, aColor);
-    if (mVisitedRule->mColor == aColor)
-      return NS_OK;
-  }
-
-  mVisitedRule = new HTMLColorRule();
-  if (!mVisitedRule)
-    return NS_ERROR_OUT_OF_MEMORY;
-
-  mVisitedRule->mColor = aColor;
-  return NS_OK;
 }
 
 already_AddRefed<nsMappedAttributes>

layout/style/nsHTMLStyleSheet.h

@@ -127,6 +127,8 @@ private:
     nscolor             mColor;
   };
 
+  // Implementation of SetLink/VisitedLink/ActiveLinkColor
+  nsresult ImplLinkColorSetter(nsRefPtr<HTMLColorRule>& aRule, nscolor aColor);
 
   class GenericTableRule;
   friend class GenericTableRule;

layout/style/nsStyleSet.cpp

@@ -1181,7 +1181,7 @@ static PRBool SheetHasDocumentStateStyle(nsIStyleRuleProcessor* aProcessor,
 {
   StatefulData* data = (StatefulData*)aData;
   if (aProcessor->HasDocumentStateDependentStyle(data)) {
-    data->mHint = eRestyle_Subtree;
+    data->mHint = eRestyle_Self;
     return PR_FALSE; // don't continue
   }
   return PR_TRUE; // continue