Bug 598832 part 7. Stop caching the content state of elements; just reget it from the DOM as needed. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 29 Mar 2011 13:29:21 -0400
changeset 64149 9620fda1e190e4de031e5d0c1fa73b96e9fb83af
parent 64148 b97449d870661d974eac07ad0e9c60a1eb46a3bc
child 64150 d5e60cc85ae6b3d87cc509af9595d9a936d89848
push idunknown
push userunknown
push dateunknown
reviewersdbaron
bugs598832
milestone2.2a1pre
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 598832 part 7. Stop caching the content state of elements; just reget it from the DOM as needed. r=dbaron
layout/style/nsCSSRuleProcessor.cpp
layout/style/nsCSSRuleProcessor.h
layout/style/nsHTMLStyleSheet.cpp
layout/style/nsRuleProcessorData.h
layout/style/nsStyleSet.cpp
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1116,18 +1116,17 @@ RuleProcessorData::RuleProcessorData(nsP
                      aRuleWalker ?
                        aRuleWalker->VisitedHandling() :
                        nsRuleWalker::eLinksVisitedOrUnvisited,
                      aElement->GetOwnerDoc()),
     mPresContext(aPresContext),
     mElement(aElement),
     mRuleWalker(aRuleWalker),
     mPreviousSiblingData(nsnull),
-    mParentData(nsnull),
-    mGotContentState(PR_FALSE)
+    mParentData(nsnull)
 {
   MOZ_COUNT_CTOR(RuleProcessorData);
 
   NS_ASSERTION(aElement, "null element leaked into SelectorMatches");
 
   NS_ASSERTION(aElement->GetOwnerDoc(), "Document-less node here?");
     
   // get the tag and parent
@@ -1142,19 +1141,16 @@ RuleProcessorData::RuleProcessorData(nsP
     mClasses = aElement->GetClasses();
   } else {
     mContentID = nsnull;
     mClasses = nsnull;
   }
 
   // get the namespace
   mNameSpaceID = aElement->GetNameSpaceID();
-
-  // No need to initialize mContentState; the ContentState() accessor will handle
-  // that.
 }
 
 RuleProcessorData::~RuleProcessorData()
 {
   MOZ_COUNT_DTOR(RuleProcessorData);
 
   // Destroy potentially long chains of previous sibling and parent data
   // without more than one level of recursion.
@@ -1200,60 +1196,66 @@ static void GetLang(nsIContent* aContent
                    "GetAttr that returns false should not make string non-empty");
       if (hasAttr) {
         return;
       }
     }
   }
 }
 
+/* static */
 nsEventStates
-RuleProcessorData::ContentState()
+nsCSSRuleProcessor::GetContentState(Element* aElement)
 {
-  if (!mGotContentState) {
-    mGotContentState = PR_TRUE;
-    mContentState = mPresContext ?
-      mPresContext->EventStateManager()->GetContentState(mElement) :
-      mElement->IntrinsicState();
+  nsIPresShell* shell = aElement->GetOwnerDoc()->GetShell();
+  nsPresContext* presContext;
+  nsEventStates state;
+  if (shell && (presContext = shell->GetPresContext())) {
+    state = presContext->EventStateManager()->GetContentState(aElement);
+  } else {
+    state = aElement->IntrinsicState();
+  }
 
-    // If we are not supposed to mark visited links as such, be sure to
-    // flip the bits appropriately.  We want to do this here, rather
-    // than in GetContentStateForVisitedHandling, so that we don't
-    // expose that :visited support is disabled to the Web page.
-    if ((!gSupportVisitedPseudo ||
-        gPrivateBrowsingObserver->InPrivateBrowsing()) &&
-        mContentState.HasState(NS_EVENT_STATE_VISITED)) {
-      mContentState &= ~NS_EVENT_STATE_VISITED;
-      mContentState |= NS_EVENT_STATE_UNVISITED;
-    }
+  // If we are not supposed to mark visited links as such, be sure to
+  // flip the bits appropriately.  We want to do this here, rather
+  // than in GetContentStateForVisitedHandling, so that we don't
+  // expose that :visited support is disabled to the Web page.
+  if ((!gSupportVisitedPseudo ||
+      gPrivateBrowsingObserver->InPrivateBrowsing()) &&
+      state.HasState(NS_EVENT_STATE_VISITED)) {
+    state &= ~NS_EVENT_STATE_VISITED;
+    state |= NS_EVENT_STATE_UNVISITED;
   }
-  return mContentState;
+  return state;
 }
 
 nsEventStates
 RuleProcessorData::DocumentState()
 {
   return mElement->GetOwnerDoc()->GetDocumentState();
 }
 
+/* static */
 PRBool
-RuleProcessorData::IsLink()
+nsCSSRuleProcessor::IsLink(Element* aElement)
 {
-  nsEventStates state = ContentState();
+  nsEventStates state = aElement->IntrinsicState();
   return state.HasAtLeastOneOfStates(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED);
 }
 
+/* static */
 nsEventStates
-RuleProcessorData::GetContentStateForVisitedHandling(
+nsCSSRuleProcessor::GetContentStateForVisitedHandling(
+                     Element* aElement,
                      nsRuleWalker::VisitedHandlingType aVisitedHandling,
                      PRBool aIsRelevantLink)
 {
-  nsEventStates contentState = ContentState();
+  nsEventStates contentState = GetContentState(aElement);
   if (contentState.HasAtLeastOneOfStates(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED)) {
-    NS_ABORT_IF_FALSE(IsLink(), "IsLink() should match state");
+    NS_ABORT_IF_FALSE(IsLink(aElement), "IsLink() should match state");
     contentState &= ~(NS_EVENT_STATE_VISITED | NS_EVENT_STATE_UNVISITED);
     if (aIsRelevantLink) {
       switch (aVisitedHandling) {
         case nsRuleWalker::eRelevantLinkUnvisited:
           contentState |= NS_EVENT_STATE_UNVISITED;
           break;
         case nsRuleWalker::eRelevantLinkVisited:
           contentState |= NS_EVENT_STATE_VISITED;
@@ -1337,17 +1339,17 @@ static PRBool ValueIncludes(const nsSubs
 
     ++p; // we know the next character is not whitespace
   }
   return PR_FALSE;
 }
 
 // Return whether we should apply a "global" (i.e., universal-tag)
 // selector for event states in quirks mode.  Note that
-// |data.IsLink()| is checked separately by the caller, so we return
+// |IsLink()| is checked separately by the caller, so we return
 // false for |nsGkAtoms::a|, which here means a named anchor.
 inline PRBool IsQuirkEventSensitive(nsIAtom *aContentTag)
 {
   return PRBool ((nsGkAtoms::button == aContentTag) ||
                  (nsGkAtoms::img == aContentTag)    ||
                  (nsGkAtoms::input == aContentTag)  ||
                  (nsGkAtoms::label == aContentTag)  ||
                  (nsGkAtoms::select == aContentTag) ||
@@ -1987,27 +1989,29 @@ static PRBool SelectorMatches(Element* a
           !aSelector->HasTagSelector() && !aSelector->mIDList && 
           !aSelector->mAttrList &&
           // This (or the other way around) both make :not() asymmetric
           // in quirks mode (and it's hard to work around since we're
           // testing the current mNegations, not the first
           // (unnegated)). This at least makes it closer to the spec.
           !isNegated &&
           // important for |IsQuirkEventSensitive|:
-          aElement->IsHTML() && !data.IsLink() &&
+          aElement->IsHTML() && !nsCSSRuleProcessor::IsLink(aElement) &&
           !IsQuirkEventSensitive(data.mContentTag)) {
         // In quirks mode, only make certain elements sensitive to
         // selectors ":hover" and ":active".
         return PR_FALSE;
       } else {
         if (aNodeMatchContext.mStateMask.HasAtLeastOneOfStates(statesToCheck)) {
           if (aDependence)
             *aDependence = PR_TRUE;
         } else {
-          nsEventStates contentState = data.GetContentStateForVisitedHandling(
+          nsEventStates contentState =
+            nsCSSRuleProcessor::GetContentStateForVisitedHandling(
+                                         aElement,
                                          aTreeMatchContext.mVisitedHandling,
                                          aNodeMatchContext.mIsRelevantLink);
           if (!contentState.HasAtLeastOneOfStates(statesToCheck)) {
             return PR_FALSE;
           }
         }
       }
     }
@@ -2178,18 +2182,19 @@ static PRBool SelectorMatchesTree(Elemen
                                            prevdata->mRuleWalker);
           prevdata->mParentData = data;
         }
       }
     }
     if (! data) {
       return PR_FALSE;
     }
-    NodeMatchContext nodeContext(nsEventStates(), aLookForRelevantLink &&
-                                                  data->IsLink());
+    NodeMatchContext nodeContext(nsEventStates(),
+                                 aLookForRelevantLink &&
+                                   nsCSSRuleProcessor::IsLink(data->mElement));
     if (nodeContext.mIsRelevantLink) {
       // If we find an ancestor of the matched node that is a link
       // during the matching process, then it's the relevant link (see
       // constructor call above).
       // Since we are still matching against selectors that contain
       // :visited (they'll just fail), we will always find such a node
       // during the selector matching process if there is a relevant
       // link that can influence selector matching.
@@ -2235,17 +2240,18 @@ static PRBool SelectorMatchesTree(Elemen
   }
   return PR_TRUE; // all the selectors matched.
 }
 
 static inline
 void ContentEnumFunc(css::StyleRule* aRule, nsCSSSelector* aSelector,
                      RuleProcessorData* data)
 {
-  NodeMatchContext nodeContext(nsEventStates(), data->IsLink());
+  NodeMatchContext nodeContext(nsEventStates(),
+                               nsCSSRuleProcessor::IsLink(data->mElement));
   if (nodeContext.mIsRelevantLink) {
     data->mHaveRelevantLink = PR_TRUE;
   }
   if (SelectorMatches(data->mElement, *data, aSelector, nodeContext, *data)) {
     nsCSSSelector *next = aSelector->mNext;
     if (!next || SelectorMatchesTree(data->mElement, *data, next,
                                      *data, !nodeContext.mIsRelevantLink)) {
       aRule->RuleMatched();
--- a/layout/style/nsCSSRuleProcessor.h
+++ b/layout/style/nsCSSRuleProcessor.h
@@ -45,16 +45,17 @@
 #ifndef nsCSSRuleProcessor_h_
 #define nsCSSRuleProcessor_h_
 
 #include "nsIStyleRuleProcessor.h"
 #include "nsCSSStyleSheet.h"
 #include "nsTArray.h"
 #include "nsAutoPtr.h"
 #include "nsCSSRules.h"
+#include "nsRuleWalker.h"
 
 struct RuleCascadeData;
 struct nsCSSSelectorList;
 struct CascadeEnumData;
 
 /**
  * The CSS style rule processor provides a mechanism for sibling style
  * sheets to combine their rule processing in order to allow proper
@@ -89,16 +90,35 @@ public:
    * the matching is not for styling purposes.  aSelectorList must not
    * include any pseudo-element selectors.  aSelectorList is allowed
    * to be null; in this case PR_FALSE will be returned.
    */
   static PRBool SelectorListMatches(mozilla::dom::Element* aElement,
                                     RuleProcessorData& aData,
                                     nsCSSSelectorList* aSelectorList);
 
+  /*
+   * Helper to get the content state for a content node.  This may be
+   * slightly adjusted from IntrinsicState().
+   */
+  static nsEventStates GetContentState(mozilla::dom::Element* aElement);
+
+  /*
+   * Helper to get the content state for :visited handling for an element
+   */
+  static nsEventStates GetContentStateForVisitedHandling(
+             mozilla::dom::Element* aElement,
+             nsRuleWalker::VisitedHandlingType aVisitedHandling,
+             PRBool aIsRelevantLink);
+
+  /*
+   * Helper to test whether a node is a link
+   */
+  static PRBool IsLink(mozilla::dom::Element* aElement);
+
   // nsIStyleRuleProcessor
   virtual void RulesMatching(ElementRuleProcessorData* aData);
 
   virtual void RulesMatching(PseudoElementRuleProcessorData* aData);
 
   virtual void RulesMatching(AnonBoxRuleProcessorData* aData);
 
 #ifdef MOZ_XUL
--- a/layout/style/nsHTMLStyleSheet.cpp
+++ b/layout/style/nsHTMLStyleSheet.cpp
@@ -65,16 +65,17 @@
 #include "nsStyleConsts.h"
 #include "nsIHTMLDocument.h"
 #include "nsIDOMHTMLElement.h"
 #include "nsCSSAnonBoxes.h"
 #include "nsRuleWalker.h"
 #include "nsRuleData.h"
 #include "nsContentErrors.h"
 #include "nsRuleProcessorData.h"
+#include "nsCSSRuleProcessor.h"
 #include "mozilla/dom/Element.h"
 #include "nsCSSFrameConstructor.h"
 
 using namespace mozilla::dom;
 
 NS_IMPL_ISUPPORTS1(nsHTMLStyleSheet::HTMLColorRule, nsIStyleRule)
 
 /* virtual */ void
@@ -216,32 +217,33 @@ nsHTMLStyleSheet::RulesMatching(ElementR
 {
   nsRuleWalker *ruleWalker = aData->mRuleWalker;
   if (aData->mElement->IsHTML()) {
     nsIAtom* tag = aData->mContentTag;
 
     // if we have anchor colors, check if this is an anchor with an href
     if (tag == nsGkAtoms::a) {
       if (mLinkRule || mVisitedRule || mActiveRule) {
-        nsEventStates state = aData->GetContentStateForVisitedHandling(
+        nsEventStates state = nsCSSRuleProcessor::GetContentStateForVisitedHandling(
+                                  aData->mElement,
                                   ruleWalker->VisitedHandling(),
                                   // If the node being matched is a link,
                                   // it's the relevant link.
-                                  aData->IsLink());
+                                  nsCSSRuleProcessor::IsLink(aData->mElement));
         if (mLinkRule && state.HasState(NS_EVENT_STATE_UNVISITED)) {
           ruleWalker->Forward(mLinkRule);
           ruleWalker->SetHaveRelevantLink();
         }
         else if (mVisitedRule && state.HasState(NS_EVENT_STATE_VISITED)) {
           ruleWalker->Forward(mVisitedRule);
           ruleWalker->SetHaveRelevantLink();
         }
 
         // No need to add to the active rule if it's not a link
-        if (mActiveRule && aData->IsLink() &&
+        if (mActiveRule && nsCSSRuleProcessor::IsLink(aData->mElement) &&
             state.HasState(NS_EVENT_STATE_ACTIVE)) {
           ruleWalker->Forward(mActiveRule);
         }
       } // end link/visited/active rules
     } // end A tag
     // add the rule to handle text-align for a <th>
     else if (tag == nsGkAtoms::th) {
       ruleWalker->Forward(mTableTHRule);
@@ -270,17 +272,17 @@ nsHTMLStyleSheet::RulesMatching(ElementR
 }
 
 // Test if style is dependent on content state
 /* virtual */ nsRestyleHint
 nsHTMLStyleSheet::HasStateDependentStyle(StateRuleProcessorData* aData)
 {
   if (aData->mElement->IsHTML() &&
       aData->mContentTag == nsGkAtoms::a &&
-      aData->IsLink() &&
+      nsCSSRuleProcessor::IsLink(aData->mElement) &&
       ((mActiveRule && aData->mStateMask.HasState(NS_EVENT_STATE_ACTIVE)) ||
        (mLinkRule && aData->mStateMask.HasState(NS_EVENT_STATE_VISITED)) ||
        (mVisitedRule && aData->mStateMask.HasState(NS_EVENT_STATE_VISITED)))) {
     return eRestyle_Self;
   }
   
   return nsRestyleHint(0);
 }
--- a/layout/style/nsRuleProcessorData.h
+++ b/layout/style/nsRuleProcessorData.h
@@ -167,48 +167,33 @@ public:
   // Reset this data for matching for the style-if-:visited.  This should
   // only be called for RuleProcessorData objects which have a rulewalker.
   void ResetForVisitedMatching() {
     mHaveRelevantLink = PR_FALSE;
     mRuleWalker->ResetForVisitedMatching();
     mVisitedHandling = mRuleWalker->VisitedHandling();
   }
   
-  nsEventStates ContentState();
   nsEventStates DocumentState();
-  PRBool IsLink();
-
-  nsEventStates GetContentStateForVisitedHandling(
-                  nsRuleWalker::VisitedHandlingType aVisitedHandling,
-                  PRBool aIsRelevantLink);
 
   nsPresContext*    mPresContext;
   mozilla::dom::Element* mElement;       // weak ref, must not be null
   nsIContent*       mParentContent; // mElement->GetParent(); weak ref
   nsRuleWalker*     mRuleWalker; // Used to add rules to our results.
   
   nsIAtom*          mContentTag;    // mElement->GetTag()
   nsIAtom*          mContentID;     // mElement->GetID()
   PRPackedBool      mHasAttributes; // mElement->GetAttrCount() > 0
   PRInt32           mNameSpaceID;   // mElement->GetNameSapce()
   const nsAttrValue* mClasses;      // mElement->GetClasses()
   // mPreviousSiblingData and mParentData are always RuleProcessorData
   // and never a derived class.  They are allocated lazily, when
   // selectors require matching of prior siblings or ancestors.
   RuleProcessorData* mPreviousSiblingData;
   RuleProcessorData* mParentData;
-
-private:
-  // mContentState is initialized lazily.
-  nsEventStates mContentState;  // eventStateMgr->GetContentState() or
-                                // mElement->IntrinsicState() if we have no ESM
-                                // adjusted for not supporting :visited (but
-                                // with visitedness information when we support
-                                // it).
-  PRPackedBool mGotContentState;
 };
 
 struct ElementRuleProcessorData : public RuleProcessorData {
   ElementRuleProcessorData(nsPresContext* aPresContext,
                            mozilla::dom::Element* aElement, 
                            nsRuleWalker* aRuleWalker,
                            PRBool aForStyling)
   : RuleProcessorData(aPresContext, aElement, aRuleWalker, aForStyling)
--- a/layout/style/nsStyleSet.cpp
+++ b/layout/style/nsStyleSet.cpp
@@ -776,18 +776,19 @@ nsStyleSet::ResolveStyleFor(Element* aEl
   if (ruleWalker.HaveRelevantLink()) {
     data.ResetForVisitedMatching();
     FileRules(EnumRulesMatching<ElementRuleProcessorData>, &data, aElement,
               &ruleWalker);
     visitedRuleNode = ruleWalker.CurrentNode();
   }
 
   return GetContext(aParentContext, ruleNode, visitedRuleNode,
-                    data.IsLink(),
-                    data.ContentState().HasState(NS_EVENT_STATE_VISITED),
+                    nsCSSRuleProcessor::IsLink(aElement),
+                    nsCSSRuleProcessor::GetContentState(aElement).
+                      HasState(NS_EVENT_STATE_VISITED),
                     nsnull, nsCSSPseudoElements::ePseudo_NotPseudoElement);
 }
 
 already_AddRefed<nsStyleContext>
 nsStyleSet::ResolveStyleForRules(nsStyleContext* aParentContext,
                                  const nsCOMArray<nsIStyleRule> &aRules)
 {
   NS_ENSURE_FALSE(mInShutdown, nsnull);