Bug 1115812 patch 3 - Pass the hints to DoRebuildAllStyleData via the member variables, in preparation for future refactoring. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Tue, 13 Jan 2015 21:03:11 -0800
changeset 250801 4d145190cf56bfd03ec2e801557852b7f91efe1c
parent 250800 8927072cd0fb0216ce2205c8652c217c0af91eef
child 250802 5f64ba26810f9b63dcfcb5a815dd157384159393
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1115812
milestone38.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 1115812 patch 3 - Pass the hints to DoRebuildAllStyleData via the member variables, in preparation for future refactoring. r=heycam Part of this refactoring involves the ability to start the rebuild-all process within the processing of restyles. This means we can't pass parameters directly from RebuildAllStyleData into DoRebuildAllStyleData. So this continues storing the hints as member variables a little bit deeper into the process. (I tried to move in a different direction in this patch queue, and store these hints in mPendingRestyles, for the root element. But that broke layout/style/test/test_counter_style.html and layout/style/test/test_font_loading_api.html, and I didn't want to figure out why. It would be somewhat better in the long run, since currently these hints will get processed if we do a rebuild-all on a RestyleTracker other than mPendingRestyles, which can happen if we have 'rem' units and have a root element font size change in the animation-only update or in mPendingAnimationRestyles.)
layout/base/RestyleManager.cpp
layout/base/RestyleManager.h
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -944,17 +944,18 @@ 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), aRestyleHint);
+        mRebuildAllRestyleHint |= aRestyleHint;
+        DoRebuildAllStyleData(aRestyleTracker);
         if (aMinHint == 0) {
           return;
         }
         aPrimaryFrame = aElement->GetPrimaryFrame();
       }
     }
   }
 
@@ -1473,20 +1474,18 @@ void
 RestyleManager::RebuildAllStyleData(nsChangeHint aExtraHint,
                                     nsRestyleHint aRestyleHint)
 {
   NS_ASSERTION(!(aExtraHint & nsChangeHint_ReconstructFrame),
                "Should not reconstruct the root of the frame tree.  "
                "Use ReconstructDocElementHierarchy instead.");
 
   mRebuildAllStyleData = false;
-  NS_UpdateHint(aExtraHint, mRebuildAllExtraHint);
-  aRestyleHint |= mRebuildAllRestyleHint;
-  mRebuildAllExtraHint = nsChangeHint(0);
-  mRebuildAllRestyleHint = nsRestyleHint(0);
+  NS_UpdateHint(mRebuildAllExtraHint, aExtraHint);
+  mRebuildAllRestyleHint |= aRestyleHint;
 
   nsIPresShell* presShell = mPresContext->GetPresShell();
   if (!presShell || !presShell->GetRootFrame())
     return;
 
   // Make sure that the viewmanager will outlive the presshell
   nsRefPtr<nsViewManager> vm = presShell->GetViewManager();
 
@@ -1507,77 +1506,80 @@ RestyleManager::RebuildAllStyleData(nsCh
 
   // Until we get rid of these phases in bug 960465, we need to skip
   // animation restyles during the non-animation phase, and post
   // animation restyles so that we restyle those elements again in the
   // animation phase.
   mSkipAnimationRules = true;
   mPostAnimationRestyles = true;
 
-  DoRebuildAllStyleData(mPendingRestyles, aExtraHint, aRestyleHint);
+  DoRebuildAllStyleData(mPendingRestyles);
 
   mPostAnimationRestyles = false;
   mSkipAnimationRules = false;
 #ifdef DEBUG
   mIsProcessingRestyles = false;
 #endif
 
   // 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,
-                                      nsRestyleHint aRestyleHint)
+RestyleManager::DoRebuildAllStyleData(RestyleTracker& aRestyleTracker)
 {
   // 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;
   }
 
+  nsRestyleHint restyleHint = mRebuildAllRestyleHint;
+  nsChangeHint changeHint = mRebuildAllExtraHint;
+  mRebuildAllExtraHint = nsChangeHint(0);
+  mRebuildAllRestyleHint = nsRestyleHint(0);
+
   // Until we get rid of these phases in bug 960465, we need to add
   // eRestyle_ChangeAnimationPhaseDescendants so that we actually honor
   // these booleans in all cases.
-  aRestyleHint |= eRestyle_ChangeAnimationPhaseDescendants;
-
-  aRestyleHint = aRestyleHint | eRestyle_ForceDescendants;
-
-  if (!(aRestyleHint & eRestyle_Subtree) &&
-      (aRestyleHint & ~(eRestyle_Force | eRestyle_ForceDescendants))) {
+  restyleHint |= eRestyle_ChangeAnimationPhaseDescendants;
+
+  restyleHint |= eRestyle_ForceDescendants;
+
+  if (!(restyleHint & eRestyle_Subtree) &&
+      (restyleHint & ~(eRestyle_Force | eRestyle_ForceDescendants))) {
     // 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.
     Element* root = mPresContext->Document()->GetRootElement();
     if (root) {
       // If the root element is gone, dropping the hint on the floor
       // should be fine.
-      aRestyleTracker.AddPendingRestyle(root, aRestyleHint, nsChangeHint(0));
+      aRestyleTracker.AddPendingRestyle(root, restyleHint, nsChangeHint(0));
     }
-    aRestyleHint = nsRestyleHint(0);
+    restyleHint = 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 Does it matter that we're passing aExtraHint to the real root
   // 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.
   ComputeAndProcessStyleChange(mPresContext->PresShell()->GetRootFrame(),
-                               aExtraHint, aRestyleTracker, aRestyleHint);
+                               changeHint, aRestyleTracker, restyleHint);
   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
   // until they are destroyed).
   mPresContext->StyleSet()->EndReconstruct();
--- a/layout/base/RestyleManager.h
+++ b/layout/base/RestyleManager.h
@@ -324,19 +324,17 @@ public:
   // in the restyle hint is harmless; some callers (e.g.,
   // nsPresContext::MediaFeatureValuesChanged) might do this for their
   // own reasons.)
   void RebuildAllStyleData(nsChangeHint aExtraHint,
                            nsRestyleHint aRestyleHint);
 
   // Helper that does part of the work of RebuildAllStyleData, shared by
   // RestyleElement for 'rem' handling.
-  void DoRebuildAllStyleData(RestyleTracker& aRestyleTracker,
-                             nsChangeHint aExtraHint,
-                             nsRestyleHint aRestyleHint);
+  void DoRebuildAllStyleData(RestyleTracker& aRestyleTracker);
 
   // See PostRestyleEventCommon below.
   void PostRestyleEvent(Element* aElement,
                         nsRestyleHint aRestyleHint,
                         nsChangeHint aMinChangeHint)
   {
     if (mPresContext) {
       PostRestyleEventCommon(aElement, aRestyleHint, aMinChangeHint,