Bug 996796 patch 20 - Make restyling exact - Avoid rerunning selector matching on everything when the basis of rem units changes. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Sun, 03 Aug 2014 13:11:55 -0700
changeset 197622 dd86a9d3fd78d0e20c24623e0bb80c464d16c6ee
parent 197621 69c78fb96b8ab6e1fc71ccff46f9dc41354bcd95
child 197623 2f9429f3db79ee16261146709b41b51f3d66aee4
push id27249
push userryanvm@gmail.com
push dateMon, 04 Aug 2014 20:14:35 +0000
treeherdermozilla-central@7f81be7db528 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs996796
milestone34.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 996796 patch 20 - Make restyling exact - Avoid rerunning selector matching on everything when the basis of rem units changes. r=heycam
layout/base/RestyleManager.cpp
layout/base/RestyleManager.h
layout/style/nsStyleSet.cpp
layout/style/nsStyleSet.h
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -885,17 +885,17 @@ RestyleManager::RestyleElement(Element* 
     nsStyleContext *oldContext = aPrimaryFrame->StyleContext();
     if (!oldContext->GetParent()) { // check that we're the root element
       nsRefPtr<nsStyleContext> newContext = mPresContext->StyleSet()->
         ResolveStyleFor(aElement, nullptr /* == oldContext->GetParent() */);
       if (oldContext->StyleFont()->mFont.size !=
           newContext->StyleFont()->mFont.size) {
         // The basis for 'rem' units has changed.
         newContext = nullptr;
-        DoRebuildAllStyleData(aRestyleTracker, nsChangeHint(0));
+        DoRebuildAllStyleData(aRestyleTracker, nsChangeHint(0), aRestyleHint);
         if (aMinHint == 0) {
           return;
         }
         aPrimaryFrame = aElement->GetPrimaryFrame();
       }
     }
   }
 
@@ -1401,51 +1401,72 @@ RestyleManager::RebuildAllStyleData(nsCh
   // We may reconstruct frames below and hence process anything that is in the
   // tree. We don't want to get notified to process those items again after.
   presShell->GetDocument()->FlushPendingNotifications(Flush_ContentAndNotify);
 
   nsAutoScriptBlocker scriptBlocker;
 
   mPresContext->SetProcessingRestyles(true);
 
-  DoRebuildAllStyleData(mPendingRestyles, aExtraHint);
+  // FIXME (bug 1047928): Many of the callers probably don't need
+  // eRestyle_Subtree because they're changing things that affect data
+  // computation rather than selector matching; we could have a restyle
+  // hint passed in, and substantially improve the performance of things
+  // like pref changes and the restyling that we do for downloadable
+  // font loads.
+  DoRebuildAllStyleData(mPendingRestyles, aExtraHint, eRestyle_Subtree);
 
   mPresContext->SetProcessingRestyles(false);
 
   // Make sure that we process any pending animation restyles from the
   // above style change.  Note that we can *almost* implement the above
   // by just posting a style change -- except we really need to restyle
   // the root frame rather than the root element's primary frame.
   ProcessPendingRestyles();
 }
 
 void
 RestyleManager::DoRebuildAllStyleData(RestyleTracker& aRestyleTracker,
-                                      nsChangeHint aExtraHint)
+                                      nsChangeHint aExtraHint,
+                                      nsRestyleHint aRestyleHint)
 {
   // Tell the style set to get the old rule tree out of the way
   // so we can recalculate while maintaining rule tree immutability
   nsresult rv = mPresContext->StyleSet()->BeginReconstruct();
   if (NS_FAILED(rv)) {
     return;
   }
 
+  if (aRestyleHint & ~eRestyle_Subtree) {
+    // We want this hint to apply to the root node's primary frame
+    // rather than the root frame, since it's the primary frame that has
+    // the styles for the root element (rather than the ancestors of the
+    // primary frame whose mContent is the root node but which have
+    // different styles).  If we use up the hint for one of the
+    // ancestors that we hit first, then we'll fail to do the restyling
+    // we need to do.
+    aRestyleTracker.AddPendingRestyle(mPresContext->Document()->GetRootElement(),
+                                      aRestyleHint, nsChangeHint(0));
+    aRestyleHint = nsRestyleHint(0);
+  }
+
   // Recalculate all of the style contexts for the document
   // Note that we can ignore the return value of ComputeStyleChangeFor
   // because we never need to reframe the root frame
   // XXX This could be made faster by not rerunning rule matching
   // (but note that nsPresShell::SetPreferenceStyleRules currently depends
   // on us re-running rule matching here
   nsStyleChangeList changeList;
   // XXX Does it matter that we're passing aExtraHint to the real root
-  // frame and not the root node's primary frame?
+  // frame and not the root node's primary frame?  (We could do
+  // roughly what we do for aRestyleHint above.)
   // Note: The restyle tracker we pass in here doesn't matter.
   ComputeStyleChangeFor(mPresContext->PresShell()->GetRootFrame(),
                         &changeList, aExtraHint,
-                        aRestyleTracker, eRestyle_Subtree);
+                        aRestyleTracker, aRestyleHint);
   // Process the required changes
   ProcessRestyledFrames(changeList);
   FlushOverflowChangedTracker();
 
   // Tell the style set it's safe to destroy the old rule tree.  We
   // must do this after the ProcessRestyledFrames call in case the
   // change list has frame reconstructs in it (since frames to be
   // reconstructed will still have their old style context pointers
@@ -2335,17 +2356,26 @@ ElementRestyler::Restyle(nsRestyleHint a
 
   NS_ASSERTION(!GetPrevContinuationWithSameStyle(mFrame),
                "should not be trying to restyle this frame separately");
 
   MOZ_ASSERT(!(aRestyleHint & eRestyle_LaterSiblings),
              "eRestyle_LaterSiblings must not be part of aRestyleHint");
 
   nsRestyleHint hintToRestore = nsRestyleHint(0);
-  if (mContent && mContent->IsElement()) {
+  if (mContent && mContent->IsElement() &&
+      // If we're we're resolving from the root of the frame tree (which
+      // we do in DoRebuildAllStyleData), we need to avoid getting the
+      // root's restyle data until we get to its primary frame, since
+      // it's the primary frame that has the styles for the root element
+      // (rather than the ancestors of the primary frame whose mContent
+      // is the root node but which have different styles).  If we use
+      // up the hint for one of the ancestors that we hit first, then
+      // we'll fail to do the restyling we need to do.
+      (mContent->GetParent() || mContent->GetPrimaryFrame() == mFrame)) {
     mContent->OwnerDoc()->FlushPendingLinkUpdates();
     RestyleTracker::RestyleData restyleData;
     if (mRestyleTracker.GetRestyleData(mContent->AsElement(), &restyleData)) {
       if (NS_UpdateHint(mHintsHandled, restyleData.mChangeHint)) {
         mChangeList->AppendChange(mFrame, mContent, restyleData.mChangeHint);
       }
       hintToRestore = restyleData.mRestyleHint;
       aRestyleHint = nsRestyleHint(aRestyleHint | restyleData.mRestyleHint);
@@ -2468,20 +2498,26 @@ ElementRestyler::RestyleSelf(nsIFrame* a
   }
   else if (pseudoTag == nsCSSAnonBoxes::mozNonElement) {
     NS_ASSERTION(aSelf->GetContent(),
                  "non pseudo-element frame without content node");
     newContext = styleSet->ResolveStyleForNonElement(parentContext);
   }
   else if (!(aRestyleHint & (eRestyle_Self | eRestyle_Subtree))) {
     Element* element = ElementForStyleContext(mParentContent, aSelf, pseudoType);
-    if (aRestyleHint == nsRestyleHint(0)) {
+    if (aRestyleHint == nsRestyleHint(0) &&
+        !styleSet->IsInRuleTreeReconstruct()) {
       newContext =
         styleSet->ReparentStyleContext(oldContext, parentContext, element);
     } else {
+      // Use ResolveStyleWithReplacement either for actual replacements
+      // or, with no replacements, as a substitute for
+      // ReparentStyleContext that rebuilds the path in the rule tree
+      // rather than reusing the rule node, as we need to do during a
+      // rule tree reconstruct.
       newContext =
         styleSet->ResolveStyleWithReplacement(element, parentContext, oldContext,
                                               aRestyleHint);
     }
   } else if (pseudoType == nsCSSPseudoElements::ePseudo_AnonBox) {
     newContext = styleSet->ResolveAnonymousBoxStyle(pseudoTag,
                                                     parentContext);
   }
@@ -2581,18 +2617,29 @@ ElementRestyler::RestyleSelf(nsIFrame* a
     const nsCSSPseudoElements::Type extraPseudoType =
       oldExtraContext->GetPseudoType();
     NS_ASSERTION(extraPseudoTag &&
                  extraPseudoTag != nsCSSAnonBoxes::mozNonElement,
                  "extra style context is not pseudo element");
     if (!(aRestyleHint & (eRestyle_Self | eRestyle_Subtree))) {
       Element* element = extraPseudoType != nsCSSPseudoElements::ePseudo_AnonBox
                            ? mContent->AsElement() : nullptr;
-      newExtraContext =
-        styleSet->ReparentStyleContext(oldExtraContext, newContext, element);
+      if (styleSet->IsInRuleTreeReconstruct()) {
+        // Use ResolveStyleWithReplacement as a substitute for
+        // ReparentStyleContext that rebuilds the path in the rule tree
+        // rather than reusing the rule node, as we need to do during a
+        // rule tree reconstruct.
+        newExtraContext =
+          styleSet->ResolveStyleWithReplacement(element, newContext,
+                                                oldExtraContext,
+                                                nsRestyleHint(0));
+      } else {
+        newExtraContext =
+          styleSet->ReparentStyleContext(oldExtraContext, newContext, element);
+      }
     } else if (extraPseudoType == nsCSSPseudoElements::ePseudo_AnonBox) {
       newExtraContext = styleSet->ResolveAnonymousBoxStyle(extraPseudoTag,
                                                            newContext);
     } else {
       // Don't expect XUL tree stuff here, since it needs a comparator and
       // all.
       NS_ASSERTION(extraPseudoType <
                      nsCSSPseudoElements::ePseudo_PseudoElementCount,
@@ -2706,39 +2753,46 @@ ElementRestyler::RestyleUndisplayedChild
       nsIContent* parent = undisplayed->mContent->GetParent();
       TreeMatchContext::AutoAncestorPusher insertionPointPusher(mTreeMatchContext);
       if (parent && nsContentUtils::IsContentInsertionPoint(parent)) {
         insertionPointPusher.PushAncestorAndStyleScope(parent);
       }
 
       nsRestyleHint thisChildHint = aChildRestyleHint;
       RestyleTracker::RestyleData undisplayedRestyleData;
-      if (mRestyleTracker.GetRestyleData(undisplayed->mContent->AsElement(),
+      Element* element = undisplayed->mContent->AsElement();
+      if (mRestyleTracker.GetRestyleData(element,
                                          &undisplayedRestyleData)) {
         thisChildHint =
           nsRestyleHint(thisChildHint | undisplayedRestyleData.mRestyleHint);
       }
       nsRefPtr<nsStyleContext> undisplayedContext;
       nsStyleSet* styleSet = mPresContext->StyleSet();
       if (thisChildHint & (eRestyle_Self | eRestyle_Subtree)) {
         undisplayedContext =
-          styleSet->ResolveStyleFor(undisplayed->mContent->AsElement(),
+          styleSet->ResolveStyleFor(element,
                                     mFrame->StyleContext(),
                                     mTreeMatchContext);
-      } else if (thisChildHint) {
+      } else if (thisChildHint ||
+                 styleSet->IsInRuleTreeReconstruct()) {
+        // Use ResolveStyleWithReplacement either for actual
+        // replacements, or as a substitute for ReparentStyleContext
+        // that rebuilds the path in the rule tree rather than reusing
+        // the rule node, as we need to do during a rule tree
+        // reconstruct.
         undisplayedContext =
-          styleSet->ResolveStyleWithReplacement(undisplayed->mContent->AsElement(),
+          styleSet->ResolveStyleWithReplacement(element,
                                                 mFrame->StyleContext(),
                                                 undisplayed->mStyle,
                                                 thisChildHint);
       } else {
         undisplayedContext =
           styleSet->ReparentStyleContext(undisplayed->mStyle,
                                          mFrame->StyleContext(),
-                                         undisplayed->mContent->AsElement());
+                                         element);
       }
       const nsStyleDisplay* display = undisplayedContext->StyleDisplay();
       if (display->mDisplay != NS_STYLE_DISPLAY_NONE) {
         NS_ASSERTION(undisplayed->mContent,
                      "Must have undisplayed content");
         mChangeList->AppendChange(nullptr, undisplayed->mContent,
                                   NS_STYLE_HINT_FRAMECHANGE);
         // The node should be removed from the undisplayed map when
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -174,17 +174,18 @@ public:
   // Rebuilds all style data by throwing out the old rule tree and
   // building a new one, and additionally applying aExtraHint (which
   // must not contain nsChangeHint_ReconstructFrame) to the root frame.
   void RebuildAllStyleData(nsChangeHint aExtraHint);
 
   // Helper that does part of the work of RebuildAllStyleData, shared by
   // RestyleElement for 'rem' handling.
   void DoRebuildAllStyleData(RestyleTracker& aRestyleTracker,
-                             nsChangeHint aExtraHint);
+                             nsChangeHint aExtraHint,
+                             nsRestyleHint aRestyleHint);
 
   // See PostRestyleEventCommon below.
   void PostRestyleEvent(Element* aElement,
                         nsRestyleHint aRestyleHint,
                         nsChangeHint aMinChangeHint)
   {
     if (mPresContext) {
       PostRestyleEventCommon(aElement, aRestyleHint, aMinChangeHint,
--- a/layout/style/nsStyleSet.cpp
+++ b/layout/style/nsStyleSet.cpp
@@ -1340,16 +1340,19 @@ nsStyleSet::RuleNodeWithReplacement(Elem
                     // way to print these.
                     nsPrintfCString("unexpected replacement bits 0x%lX",
                                     uint32_t(aReplacements)).get());
 
   // FIXME (perf): This should probably not rebuild the whole path, but
   // only the path from the last change in the rule tree, like
   // ReplaceAnimationRule in nsStyleSet.cpp does.  (That could then
   // perhaps share this code, too?)
+  // But if we do that, we'll need to pass whether we are rebuilding the
+  // rule tree from ElementRestyler::RestyleSelf to avoid taking that
+  // path when we're rebuilding the rule tree.
 
   nsTArray<RuleNodeInfo> rules;
   for (nsRuleNode* ruleNode = aOldRuleNode; !ruleNode->IsRoot();
        ruleNode = ruleNode->GetParent()) {
     RuleNodeInfo* curRule = rules.AppendElement();
     curRule->mRule = ruleNode->GetRule();
     curRule->mLevel = ruleNode->GetLevel();
     curRule->mIsImportant = ruleNode->IsImportantRule();
--- a/layout/style/nsStyleSet.h
+++ b/layout/style/nsStyleSet.h
@@ -313,16 +313,20 @@ class nsStyleSet
 
   // Methods for reconstructing the tree; BeginReconstruct basically moves the
   // old rule tree root and style context roots out of the way,
   // and EndReconstruct destroys the old rule tree when we're done
   nsresult BeginReconstruct();
   // Note: EndReconstruct should not be called if BeginReconstruct fails
   void EndReconstruct();
 
+  bool IsInRuleTreeReconstruct() const {
+    return mInReconstruct;
+  }
+
   // Let the style set know that a particular sheet is the quirks sheet.  This
   // sheet must already have been added to the UA sheets.  The pointer must not
   // be null.  This should only be called once for a given style set.
   void SetQuirkStyleSheet(nsIStyleSheet* aQuirkStyleSheet);
 
   // Return whether the rule tree has cached data such that we need to
   // do dynamic change handling for changes that change the results of
   // media queries or require rebuilding all style data.