Bug 1392170: Don't call ResolveServoStyle unnecessarily for undisplayed and not restyled stuff. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 23 Aug 2017 13:34:17 +0200
changeset 651264 f4f77eea3311c25cc8e8757af0956b852becb28d
parent 651263 c58f1fe858956d18124c693e00524c81297af458
child 727638 4c647c4b5c520211f0f4932bb413bcad6dc788fd
push id75648
push userbmo:emilio+bugs@crisal.io
push dateWed, 23 Aug 2017 11:37:13 +0000
reviewersheycam
bugs1392170
milestone57.0a1
Bug 1392170: Don't call ResolveServoStyle unnecessarily for undisplayed and not restyled stuff. r?heycam In particular, this avoids the stupid calls in display: none roots. The cache stuff should be unnecessary, but there's still some fishy stuff going on. MozReview-Commit-ID: LgnW0k1gmsN
layout/base/ServoRestyleManager.cpp
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -432,34 +432,36 @@ ServoRestyleManager::ClearRestyleStateFr
  *
  * This is currently used to properly compute change hints when the parent
  * element of this node is a display: contents node, and also to avoid computing
  * the style for text children more than once per element.
  */
 struct ServoRestyleManager::TextPostTraversalState
 {
 public:
-  TextPostTraversalState(ServoStyleContext& aParentContext,
+  TextPostTraversalState(Element& aParentElement,
+                         ServoStyleContext* aParentContext,
                          bool aDisplayContentsParentStyleChanged,
                          ServoRestyleState& aParentRestyleState)
-    : mParentContext(aParentContext)
+    : mParentElement(aParentElement)
+    , mParentContext(aParentContext)
     , mParentRestyleState(aParentRestyleState)
     , mStyle(nullptr)
     , mShouldPostHints(aDisplayContentsParentStyleChanged)
     , mShouldComputeHints(aDisplayContentsParentStyleChanged)
     , mComputedHint(nsChangeHint_Empty)
   {}
 
   nsStyleChangeList& ChangeList() { return mParentRestyleState.ChangeList(); }
 
   nsStyleContext& ComputeStyle(nsIContent* aTextNode)
   {
     if (!mStyle) {
       mStyle = mParentRestyleState.StyleSet().ResolveStyleForText(
-        aTextNode, &mParentContext);
+        aTextNode, &ParentStyle());
     }
     MOZ_ASSERT(mStyle);
     return *mStyle;
   }
 
   void ComputeHintIfNeeded(nsIContent* aContent,
                            nsIFrame* aTextFrame,
                            nsStyleContext& aNewContext)
@@ -492,17 +494,28 @@ public:
 
     if (mComputedHint) {
       mParentRestyleState.ChangeList().AppendChange(
         aTextFrame, aContent, mComputedHint);
     }
   }
 
 private:
-  ServoStyleContext& mParentContext;
+  ServoStyleContext& ParentStyle() {
+    if (!mParentContext) {
+      mLazilyResolvedParentContext =
+        mParentRestyleState.StyleSet().ResolveServoStyle(&mParentElement);
+      mParentContext = mLazilyResolvedParentContext;
+    }
+    return *mParentContext;
+  }
+
+  Element& mParentElement;
+  ServoStyleContext* mParentContext;
+  RefPtr<ServoStyleContext> mLazilyResolvedParentContext;
   ServoRestyleState& mParentRestyleState;
   RefPtr<nsStyleContext> mStyle;
   bool mShouldPostHints;
   bool mShouldComputeHints;
   nsChangeHint mComputedHint;
 };
 
 static void
@@ -736,22 +749,20 @@ ServoRestyleManager::ProcessPostTraversa
   }
 
   // We can't really assume as used changes from display: contents elements (or
   // other elements without frames).
   ServoRestyleState& childrenRestyleState =
     thisFrameRestyleState ? *thisFrameRestyleState : aRestyleState;
 
   RefPtr<ServoStyleContext> upToDateContext =
-    (wasRestyled || !oldStyleContext)
+    wasRestyled
       ? aRestyleState.StyleSet().ResolveServoStyle(aElement)
       : oldStyleContext;
 
-  MOZ_ASSERT(upToDateContext);
-
   if (wasRestyled && oldStyleContext) {
     MOZ_ASSERT(styleFrame || displayContentsStyle);
     MOZ_ASSERT(oldStyleContext->ComputedData() != upToDateContext->ComputedData());
 
     upToDateContext->ResolveSameStructsAs(oldStyleContext);
 
     // We want to walk all the continuations here, even the ones with different
     // styles.  In practice, the only reason we get continuations with different
@@ -804,17 +815,18 @@ ServoRestyleManager::ProcessPostTraversa
 
   const bool traverseElementChildren =
     aElement->HasAnyOfFlags(Element::kAllServoDescendantBits);
   const bool traverseTextChildren =
     wasRestyled || aElement->HasFlag(NODE_DESCENDANTS_NEED_FRAMES);
   bool recreatedAnyContext = wasRestyled;
   if (traverseElementChildren || traverseTextChildren) {
     StyleChildrenIterator it(aElement);
-    TextPostTraversalState textState(*upToDateContext,
+    TextPostTraversalState textState(*aElement,
+                                     upToDateContext,
                                      displayContentsStyle && wasRestyled,
                                      childrenRestyleState);
     for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
       if (traverseElementChildren && n->IsElement()) {
         recreatedAnyContext |= ProcessPostTraversal(n->AsElement(),
                                                     upToDateContext,
                                                     childrenRestyleState,
                                                     aFlags,