Bug 1385789: Refactor RestyleManager::ContentStateChangedInternal to move nsRestyleHint calculation out to GeckoRestyleManager. r=bholley

MozReview-Commit-ID: 7GXkPcjfYe6

--HG--
extra : rebase_source : d493227f872f2698d2f7d48df4132377b2d43c36

layout/base/GeckoRestyleManager.cpp

@@ -293,8 +293,36 @@ GeckoRestyleManager::ContentStateChanged(nsIContent* aContent,
   Element* aElement = aContent->AsElement();
 
   nsChangeHint changeHint;
+  ContentStateChangedInternal(aElement, aStateMask, &changeHint);
+
+  // Assemble what we'll need to calculate the nsRestyleHint.
+  nsIFrame* primaryFrame = aElement->GetPrimaryFrame();
+  CSSPseudoElementType pseudoType = CSSPseudoElementType::NotPseudo;
+  if (primaryFrame) {
+    pseudoType = primaryFrame->StyleContext()->GetPseudoType();
+  }
+
+  StyleSetHandle styleSet = PresContext()->StyleSet();
+  MOZ_ASSERT(styleSet);
+
   nsRestyleHint restyleHint;
-  ContentStateChangedInternal(aElement, aStateMask, &changeHint, &restyleHint);
+  if (pseudoType >= CSSPseudoElementType::Count) {
+    restyleHint = styleSet->HasStateDependentStyle(aElement, aStateMask);
+  } else if (nsCSSPseudoElements::PseudoElementSupportsUserActionState(
+               pseudoType)) {
+    // If aElement is a pseudo-element, we want to check to see whether there
+    // are any state-dependent rules applying to that pseudo.
+    Element* ancestor =
+      ElementForStyleContext(nullptr, primaryFrame, pseudoType);
+    restyleHint = styleSet->HasStateDependentStyle(ancestor, pseudoType,
+                                                   aElement, aStateMask);
+  } else {
+    restyleHint = nsRestyleHint(0);
+  }
+
+  if (aStateMask.HasState(NS_EVENT_STATE_HOVER) && restyleHint != 0) {
+    IncrementHoverGeneration();
+  }
 
   PostRestyleEvent(aElement, restyleHint, changeHint);
 }

layout/base/RestyleManager.cpp

@@ -331,15 +331,10 @@ RestyleManager::ContentRemoved(nsINode* aContainer,
 void
 RestyleManager::ContentStateChangedInternal(Element* aElement,
                                             EventStates aStateMask,
-                                            nsChangeHint* aOutChangeHint,
-                                            nsRestyleHint* aOutRestyleHint)
+                                            nsChangeHint* aOutChangeHint)
 {
   MOZ_ASSERT(!mInStyleRefresh);
   MOZ_ASSERT(aOutChangeHint);
-  MOZ_ASSERT(aOutRestyleHint);
-
-  StyleSetHandle styleSet = PresContext()->StyleSet();
-  NS_ASSERTION(styleSet, "couldn't get style set");
 
   *aOutChangeHint = nsChangeHint(0);
   // Any change to a content state that affects which frames we construct
@@ -349,7 +344,6 @@ RestyleManager::ContentStateChangedInternal(Element* aElement,
   // need to force a reframe -- if it's needed, the HasStateDependentStyle
   // call will handle things.
   nsIFrame* primaryFrame = aElement->GetPrimaryFrame();
-  CSSPseudoElementType pseudoType = CSSPseudoElementType::NotPseudo;
   if (primaryFrame) {
     // If it's generated content, ignore LOADING/etc state changes on it.
     if (!primaryFrame->IsGeneratedContentFrame() &&
@@ -374,29 +368,9 @@ RestyleManager::ContentStateChangedInternal(Element* aElement,
       }
     }
 
-    pseudoType = primaryFrame->StyleContext()->GetPseudoType();
-
     primaryFrame->ContentStatesChanged(aStateMask);
   }
 
-  if (pseudoType >= CSSPseudoElementType::Count) {
-    *aOutRestyleHint = styleSet->HasStateDependentStyle(aElement, aStateMask);
-  } else if (nsCSSPseudoElements::PseudoElementSupportsUserActionState(
-               pseudoType)) {
-    // If aElement is a pseudo-element, we want to check to see whether there
-    // are any state-dependent rules applying to that pseudo.
-    Element* ancestor =
-      ElementForStyleContext(nullptr, primaryFrame, pseudoType);
-    *aOutRestyleHint = styleSet->HasStateDependentStyle(ancestor, pseudoType,
-                                                        aElement, aStateMask);
-  } else {
-    *aOutRestyleHint = nsRestyleHint(0);
-  }
-
-  if (aStateMask.HasState(NS_EVENT_STATE_HOVER) && *aOutRestyleHint != 0) {
-    IncrementHoverGeneration();
-  }
-
   if (aStateMask.HasState(NS_EVENT_STATE_VISITED)) {
     // Exposing information to the page about whether the link is
     // visited or not isn't really something we can worry about here.

layout/base/RestyleManager.h

@@ -226,8 +226,7 @@ protected:
 
   void ContentStateChangedInternal(Element* aElement,
                                    EventStates aStateMask,
-                                   nsChangeHint* aOutChangeHint,
-                                   nsRestyleHint* aOutRestyleHint);
+                                   nsChangeHint* aOutChangeHint);
 
   bool IsDisconnected() { return mPresContext == nullptr; }
 

layout/base/ServoRestyleManager.cpp

@@ -1221,34 +1221,12 @@ ServoRestyleManager::ContentStateChanged(nsIContent* aContent,
   }
 
   Element* aElement = aContent->AsElement();
-  nsChangeHint changeHint;
-  nsRestyleHint restyleHint;
-
   if (!aElement->HasServoData()) {
     return;
   }
 
-  // NOTE: restyleHint here is effectively always 0, since that's what
-  // ServoStyleSet::HasStateDependentStyle returns. Servo computes on
-  // ProcessPendingRestyles using the ElementSnapshot, but in theory could
-  // compute it sequentially easily.
-  //
-  // Determine what's the best way to do it, and how much work do we save
-  // processing the restyle hint early (i.e., computing the style hint here
-  // sequentially, potentially saving the snapshot), vs lazily (snapshot
-  // approach).
-  //
-  // If we take the sequential approach we need to specialize Servo's restyle
-  // hints system a bit more, and mesure whether we save something storing the
-  // restyle hint in the table and deferring the dirtiness setting until
-  // ProcessPendingRestyles (that's a requirement if we store snapshots though),
-  // vs processing the restyle hint in-place, dirtying the nodes on
-  // PostRestyleEvent.
-  //
-  // If we definitely take the snapshot approach, we should take rid of
-  // HasStateDependentStyle, etc (though right now they're no-ops).
-  ContentStateChangedInternal(aElement, aChangedBits, &changeHint,
-                              &restyleHint);
+  nsChangeHint changeHint;
+  ContentStateChangedInternal(aElement, aChangedBits, &changeHint);
 
   // Don't bother taking a snapshot if no rules depend on these state bits.
   //
@@ -1264,8 +1242,8 @@ ServoRestyleManager::ContentStateChanged(nsIContent* aContent,
   EventStates previousState = aElement->StyleState() ^ aChangedBits;
   snapshot.AddState(previousState);
 
-  if (restyleHint || changeHint) {
-    Servo_NoteExplicitHints(aElement, restyleHint, changeHint);
+  if (changeHint) {
+    Servo_NoteExplicitHints(aElement, nsRestyleHint(0), changeHint);
   }
 
   // Assuming we need to invalidate cached style in getComputedStyle for