Bug 1385789: Refactor RestyleManager::ContentStateChangedInternal to move nsRestyleHint calculation out to GeckoRestyleManager. r=bholley
authorBrad Werth <bwerth@mozilla.com>
Tue, 29 Aug 2017 15:50:50 -0700
changeset 377836 194d51097e24aae71fbb688f5e62d79d5ebeac96
parent 377835 28bcad7387a02939beeb4c1bfb9cd13bdfeca8b8
child 377837 0b6e08391e33a958839624332d1be5634038928c
push id32416
push userarchaeopteryx@coole-files.de
push dateThu, 31 Aug 2017 12:35:23 +0000
treeherdermozilla-central@c9079d347aaa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1385789
milestone57.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 1385789: Refactor RestyleManager::ContentStateChangedInternal to move nsRestyleHint calculation out to GeckoRestyleManager. r=bholley MozReview-Commit-ID: 7GXkPcjfYe6
layout/base/GeckoRestyleManager.cpp
layout/base/RestyleManager.cpp
layout/base/RestyleManager.h
layout/base/ServoRestyleManager.cpp
--- a/layout/base/GeckoRestyleManager.cpp
+++ b/layout/base/GeckoRestyleManager.cpp
@@ -288,18 +288,46 @@ GeckoRestyleManager::ContentStateChanged
   // we'd have to make ESM guarantee that usefully.
   if (!aContent->IsElement()) {
     return;
   }
 
   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);
 }
 
 // Forwarded nsIMutationObserver method, to handle restyling.
 void
 GeckoRestyleManager::AttributeWillChange(Element* aElement,
                                          int32_t aNameSpaceID,
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -326,35 +326,29 @@ RestyleManager::ContentRemoved(nsINode* 
  * Calculates the change hint and the restyle hint for a given content state
  * change.
  *
  * This is called from both Restyle managers.
  */
 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
   // must lead to a frame reconstruct here if we already have a frame.
   // Note that we never decide through non-CSS means to not create frames
   // based on content states, so if we already don't have a frame we don't
   // 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() &&
         aStateMask.HasAtLeastOneOfStates(NS_EVENT_STATE_BROKEN |
                                          NS_EVENT_STATE_USERDISABLED |
                                          NS_EVENT_STATE_SUPPRESSED |
                                          NS_EVENT_STATE_LOADING)) {
       *aOutChangeHint = nsChangeHint_ReconstructFrame;
@@ -369,39 +363,19 @@ RestyleManager::ContentStateChangedInter
                                     nullptr);
           if (repaint) {
             *aOutChangeHint |= nsChangeHint_RepaintFrame;
           }
         }
       }
     }
 
-    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.
     // FIXME: We could probably do this a bit better.
     *aOutChangeHint |= nsChangeHint_RepaintFrame;
   }
 }
 
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -221,18 +221,17 @@ protected:
     MOZ_ASSERT(!mAnimationsWithDestroyedFrame,
                "leaving dangling pointers from AnimationsWithDestroyedFrame");
   }
 
   void RestyleForEmptyChange(Element* aContainer);
 
   void ContentStateChangedInternal(Element* aElement,
                                    EventStates aStateMask,
-                                   nsChangeHint* aOutChangeHint,
-                                   nsRestyleHint* aOutRestyleHint);
+                                   nsChangeHint* aOutChangeHint);
 
   bool IsDisconnected() { return mPresContext == nullptr; }
 
   void IncrementHoverGeneration() {
     ++mHoverGeneration;
   }
 
   void IncrementRestyleGeneration() {
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -1216,61 +1216,39 @@ ServoRestyleManager::ContentStateChanged
 {
   MOZ_ASSERT(!mInStyleRefresh);
 
   if (!aContent->IsElement()) {
     return;
   }
 
   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.
   //
   // We always take a snapshot for the LTR/RTL event states, since Servo doesn't
   // track those bits in the same way, and we know that :dir() rules are always
   // present in UA style sheets.
   if (!aChangedBits.HasAtLeastOneOfStates(DIRECTION_STATES) &&
       !StyleSet()->HasStateDependency(*aElement, aChangedBits)) {
     return;
   }
 
   ServoElementSnapshot& snapshot = SnapshotFor(aElement);
   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
   // undisplayed elements, since we don't know if it is needed.
   IncrementUndisplayedRestyleGeneration();
 }
 
 static inline bool