Bug 1362991: Pass StyleSet by reference in ServoRestyleManager. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 08 May 2017 14:42:05 +0200
changeset 574204 2926ff44c2c2e99c773549db1e0a916ad4d5bf88
parent 574203 b63898b7a7fe8e125da6bfb545c07b582f1116ec
child 574205 be16210cf5d772515ce2a69a0be2f9fe1ef18ffd
child 574215 506a89523ae8d7da95678612770ab6ac196d286d
push id57604
push userbmo:emilio+bugs@crisal.io
push dateMon, 08 May 2017 12:52:09 +0000
reviewersheycam
bugs1362991
milestone55.0a1
Bug 1362991: Pass StyleSet by reference in ServoRestyleManager. r?heycam UpdateStyleOfOwnedAnonBoxes already does this, and I think it's cleaner. Happy to revert if wanted, but I'd like us to do this more in general... :/ MozReview-Commit-ID: CdiZFDKAaPr
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -69,17 +69,17 @@ ServoRestyleManager::PostRestyleEventFor
 {
   Servo_NoteExplicitHints(aElement, aRestyleHint, nsChangeHint(0));
 }
 
 void
 ServoRestyleManager::RebuildAllStyleData(nsChangeHint aExtraHint,
                                          nsRestyleHint aRestyleHint)
 {
-  StyleSet()->RebuildData();
+  StyleSet().RebuildData();
 
   mHaveNonAnimationRestyles = true;
 
   // NOTE(emilio): GeckoRestlyeManager does a sync style flush, which seems
   // not to be needed in my testing.
   //
   // If it is, we can just do a content flush and call ProcessPendingRestyles.
   if (Element* root = mPresContext->Document()->GetRootElement()) {
@@ -189,17 +189,17 @@ struct ServoRestyleManager::TextPostTrav
       aChangeList.AppendChange(aTextFrame, aContent, mComputedHint);
     }
   }
 };
 
 void
 ServoRestyleManager::ProcessPostTraversal(Element* aElement,
                                           nsStyleContext* aParentContext,
-                                          ServoStyleSet* aStyleSet,
+                                          ServoStyleSet& aStyleSet,
                                           nsStyleChangeList& aChangeList)
 {
   nsIFrame* styleFrame = nsLayoutUtils::GetStyleFrame(aElement);
 
   // Grab the change hint from Servo.
   nsChangeHint changeHint = Servo_TakeChangeHint(aElement);
 
   // Handle lazy frame construction by posting a reconstruct for any lazily-
@@ -247,17 +247,17 @@ ServoRestyleManager::ProcessPostTraversa
     displayContentsNode =
       PresContext()->FrameConstructor()->GetDisplayContentsNodeFor(aElement);
     if (displayContentsNode) {
       oldStyleContext = displayContentsNode->mStyle;
     }
   }
 
   RefPtr<ServoComputedValues> computedValues =
-    aStyleSet->ResolveServoStyle(aElement);
+    aStyleSet.ResolveServoStyle(aElement);
 
   // Note that we rely in the fact that we don't cascade pseudo-element styles
   // separately right now (that is, if a pseudo style changes, the normal style
   // changes too).
   //
   // Otherwise we should probably encode that information somehow to avoid
   // expensive checks in the common case.
   //
@@ -271,21 +271,21 @@ ServoRestyleManager::ProcessPostTraversa
   if (recreateContext) {
     MOZ_ASSERT(styleFrame || displayContentsNode);
 
     auto pseudo = aElement->GetPseudoElementType();
     nsIAtom* pseudoTag = pseudo == CSSPseudoElementType::NotPseudo
       ? nullptr : nsCSSPseudoElements::GetPseudoAtom(pseudo);
 
     newContext =
-      aStyleSet->GetContext(computedValues.forget(),
-                            aParentContext,
-                            pseudoTag,
-                            pseudo,
-                            aElement);
+      aStyleSet.GetContext(computedValues.forget(),
+                           aParentContext,
+                           pseudoTag,
+                           pseudo,
+                           aElement);
 
     newContext->EnsureSameStructsCached(oldStyleContext);
 
     // XXX This could not always work as expected: there are kinds of content
     // with the first split and the last sharing style, but others not. We
     // should handle those properly.
     // XXXbz I think the UpdateStyleOfOwnedAnonBoxes call below handles _that_
     // right, but not other cases where we happen to have different styles on
@@ -296,17 +296,17 @@ ServoRestyleManager::ProcessPostTraversa
     }
 
     if (MOZ_UNLIKELY(displayContentsNode)) {
       MOZ_ASSERT(!styleFrame);
       displayContentsNode->mStyle = newContext;
     }
 
     if (styleFrame) {
-      styleFrame->UpdateStyleOfOwnedAnonBoxes(*aStyleSet, aChangeList, changeHint);
+      styleFrame->UpdateStyleOfOwnedAnonBoxes(aStyleSet, aChangeList, changeHint);
     }
   }
 
   const bool descendantsNeedFrames =
     aElement->HasFlag(NODE_DESCENDANTS_NEED_FRAMES);
   const bool traverseElementChildren =
     aElement->HasDirtyDescendantsForServo() || descendantsNeedFrames;
   const bool traverseTextChildren = recreateContext || descendantsNeedFrames;
@@ -330,17 +330,17 @@ ServoRestyleManager::ProcessPostTraversa
   aElement->UnsetHasDirtyDescendantsForServo();
   aElement->UnsetFlags(NODE_DESCENDANTS_NEED_FRAMES);
 }
 
 void
 ServoRestyleManager::ProcessPostTraversalForText(
     nsIContent* aTextNode,
     nsStyleContext* aParentContext,
-    ServoStyleSet* aStyleSet,
+    ServoStyleSet& aStyleSet,
     nsStyleChangeList& aChangeList,
     TextPostTraversalState& aPostTraversalState)
 {
   // Handle lazy frame construction.
   if (aTextNode->HasFlag(NODE_NEEDS_FRAME)) {
     aChangeList.AppendChange(nullptr, aTextNode, nsChangeHint_ReconstructFrame);
     return;
   }
@@ -437,33 +437,33 @@ ServoRestyleManager::ProcessPendingResty
     return;
   }
 
   // Create a AnimationsWithDestroyedFrame during restyling process to
   // stop animations and transitions on elements that have no frame at the end
   // of the restyling process.
   AnimationsWithDestroyedFrame animationsWithDestroyedFrame(this);
 
-  ServoStyleSet* styleSet = StyleSet();
+  ServoStyleSet& styleSet = StyleSet();
   nsIDocument* doc = PresContext()->Document();
 
   // Ensure the refresh driver is active during traversal to avoid mutating
   // mActiveTimer and mMostRecentRefresh time.
   PresContext()->RefreshDriver()->MostRecentRefresh();
 
 
   // Perform the Servo traversal, and the post-traversal if required. We do this
   // in a loop because certain rare paths in the frame constructor (like
   // uninstalling XBL bindings) can trigger additional style validations.
   mInStyleRefresh = true;
   if (mHaveNonAnimationRestyles) {
     ++mAnimationGeneration;
   }
 
-  while (styleSet->StyleDocument()) {
+  while (styleSet.StyleDocument()) {
     ClearSnapshots();
 
     // Recreate style contexts, and queue up change hints (which also handle
     // lazy frame construction).
     nsStyleChangeList currentChanges(StyleBackendType::Servo);
     DocumentStyleRootIterator iter(doc);
     while (Element* root = iter.GetNextStyleRoot()) {
       ProcessPostTraversal(root, nullptr, styleSet, currentChanges);
@@ -490,17 +490,17 @@ ServoRestyleManager::ProcessPendingResty
     IncrementRestyleGeneration();
   }
 
   ClearSnapshots();
   FlushOverflowChangedTracker();
 
   mHaveNonAnimationRestyles = false;
   mInStyleRefresh = false;
-  styleSet->AssertTreeIsClean();
+  styleSet.AssertTreeIsClean();
 
   // Note: We are in the scope of |animationsWithDestroyedFrame|, so
   //       |mAnimationsWithDestroyedFrame| is still valid.
   MOZ_ASSERT(mAnimationsWithDestroyedFrame);
   mAnimationsWithDestroyedFrame->StopAnimationsForElementsWithoutFrames();
 }
 
 void
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -113,32 +113,32 @@ protected:
   }
 
 private:
   /**
    * Performs post-Servo-traversal processing on this element and its descendants.
    */
   void ProcessPostTraversal(Element* aElement,
                             nsStyleContext* aParentContext,
-                            ServoStyleSet* aStyleSet,
+                            ServoStyleSet& aStyleSet,
                             nsStyleChangeList& aChangeList);
 
   struct TextPostTraversalState;
   void ProcessPostTraversalForText(nsIContent* aTextNode,
                                    nsStyleContext* aParentContext,
-                                   ServoStyleSet* aStyleSet,
+                                   ServoStyleSet& aStyleSet,
                                    nsStyleChangeList& aChangeList,
                                    TextPostTraversalState& aState);
 
-  inline ServoStyleSet* StyleSet() const
+  inline ServoStyleSet& StyleSet() const
   {
     MOZ_ASSERT(PresContext()->StyleSet()->IsServo(),
                "ServoRestyleManager should only be used with a Servo-flavored "
                "style backend");
-    return PresContext()->StyleSet()->AsServo();
+    return *PresContext()->StyleSet()->AsServo();
   }
 
   const SnapshotTable& Snapshots() const { return mSnapshots; }
   void ClearSnapshots();
   ServoElementSnapshot& SnapshotFor(mozilla::dom::Element* aElement);
 
   // We use a separate data structure from nsStyleChangeList because we need a
   // frame to create nsStyleChangeList entries, and the primary frame may not be