Abstract nsChangeHint_NonInherited_Hints into a function so that it accurately reports the reflow cases to all callers. (Bug 779968, patch 4) r=bzbarsky
authorL. David Baron <dbaron@dbaron.org>
Fri, 07 Sep 2012 10:13:36 -0700
changeset 104579 2d8810ba0412f4984376ebf4beb3de2fa97e9180
parent 104578 c86123a966b1ab2213d0a8952b7d824fe2e2dd3a
child 104580 309d87857ce0499251f2b2e927530a82e9642b3b
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersbzbarsky
bugs779968
milestone18.0a1
Abstract nsChangeHint_NonInherited_Hints into a function so that it accurately reports the reflow cases to all callers. (Bug 779968, patch 4) r=bzbarsky This is in preparation for adding an additional caller. nsChangeHint_NonInherited_Hints will be reintroduced in patch 6, but as the maximum set of such hints rather than the minimal set, and with the less confusing name nsChangeHint_Hints_NotHandledForDescendants.
layout/base/nsChangeHint.h
layout/base/nsFrameManager.cpp
layout/style/nsStyleContext.cpp
--- a/layout/base/nsChangeHint.h
+++ b/layout/base/nsChangeHint.h
@@ -102,35 +102,20 @@ enum nsChangeHint {
   nsChangeHint_RecomputePosition = 0x2000,
 
   /**
    * Behaves like ReconstructFrame, but only if the frame has descendants
    * that are absolutely or fixed position. Use this hint when a style change
    * has changed whether the frame is a container for fixed-pos or abs-pos
    * elements, but reframing is otherwise not needed.
    */
-  nsChangeHint_AddOrRemoveTransform = 0x4000,
+  nsChangeHint_AddOrRemoveTransform = 0x4000
 
-  /**
-   * We have an optimization when processing change hints which prevents
-   * us from visiting the descendants of a node when a hint on that node
-   * is being processed.  This optimization does not apply in some of the
-   * cases where applying a hint to an element does not necessarily result
-   * in the same hint being handled on the descendants.
-   *
-   * If you're adding such a hint, you should add your hint to this list.
-   */
-  nsChangeHint_NonInherited_Hints =
-    nsChangeHint_UpdateTransformLayer |
-    nsChangeHint_UpdateEffects |
-    nsChangeHint_UpdateOpacityLayer |
-    nsChangeHint_UpdateOverflow |
-    nsChangeHint_ChildrenOnlyTransform |
-    nsChangeHint_RecomputePosition |
-    nsChangeHint_AddOrRemoveTransform
+  // IMPORTANT NOTE: When adding new hints, consider whether you need to
+  // add them to NS_HintsNotHandledForDescendantsIn() below.
 };
 
 // Redefine these operators to return nothing. This will catch any use
 // of these operators on hints. We should not be using these operators
 // on nsChangeHints
 inline void operator<(nsChangeHint s1, nsChangeHint s2) {}
 inline void operator>(nsChangeHint s1, nsChangeHint s2) {}
 inline void operator!=(nsChangeHint s1, nsChangeHint s2) {}
@@ -159,16 +144,50 @@ inline bool NS_UpdateHint(nsChangeHint& 
   return changed;
 }
 
 // Returns true iff the second hint contains all the hints of the first hint
 inline bool NS_IsHintSubset(nsChangeHint aSubset, nsChangeHint aSuperSet) {
   return (aSubset & aSuperSet) == aSubset;
 }
 
+/**
+ * We have an optimization when processing change hints which prevents
+ * us from visiting the descendants of a node when a hint on that node
+ * is being processed.  This optimization does not apply in some of the
+ * cases where applying a hint to an element does not necessarily result
+ * in the same hint being handled on the descendants.
+ */
+inline nsChangeHint NS_HintsNotHandledForDescendantsIn(nsChangeHint aChangeHint) {
+  nsChangeHint result = nsChangeHint(aChangeHint & (
+    nsChangeHint_UpdateTransformLayer |
+    nsChangeHint_UpdateEffects |
+    nsChangeHint_UpdateOpacityLayer |
+    nsChangeHint_UpdateOverflow |
+    nsChangeHint_ChildrenOnlyTransform |
+    nsChangeHint_RecomputePosition |
+    nsChangeHint_AddOrRemoveTransform));
+
+  if (!NS_IsHintSubset(nsChangeHint_NeedDirtyReflow, aChangeHint) &&
+      NS_IsHintSubset(nsChangeHint_NeedReflow, aChangeHint)) {
+    // If NeedDirtyReflow is *not* set, then NeedReflow is a
+    // non-inherited hint.
+    NS_UpdateHint(result, nsChangeHint_NeedReflow);
+  }
+
+  if (!NS_IsHintSubset(nsChangeHint_ClearDescendantIntrinsics, aChangeHint) &&
+      NS_IsHintSubset(nsChangeHint_ClearAncestorIntrinsics, aChangeHint)) {
+    // If ClearDescendantIntrinsics is *not* set, then
+    // ClearAncestorIntrinsics is a non-inherited hint.
+    NS_UpdateHint(result, nsChangeHint_ClearAncestorIntrinsics);
+  }
+
+  return result;
+}
+
 // Redefine the old NS_STYLE_HINT constants in terms of the new hint structure
 #define NS_STYLE_HINT_NONE \
   nsChangeHint(0)
 #define NS_STYLE_HINT_VISUAL \
   nsChangeHint(nsChangeHint_RepaintFrame | nsChangeHint_SyncFrameView)
 #define nsChangeHint_ReflowFrame                        \
   nsChangeHint(nsChangeHint_NeedReflow |                \
                nsChangeHint_ClearAncestorIntrinsics |   \
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -1006,38 +1006,20 @@ nsFrameManager::ReResolveStyleContext(ns
                                       nsStyleChangeList *aChangeList, 
                                       nsChangeHint       aMinChange,
                                       nsRestyleHint      aRestyleHint,
                                       RestyleTracker&    aRestyleTracker,
                                       DesiredA11yNotifications aDesiredA11yNotifications,
                                       nsTArray<nsIContent*>& aVisibleKidsOfHiddenElement,
                                       TreeMatchContext &aTreeMatchContext)
 {
-  if (!NS_IsHintSubset(nsChangeHint_NeedDirtyReflow, aMinChange)) {
-    // If aMinChange doesn't include nsChangeHint_NeedDirtyReflow, clear out
-    // all the reflow change bits from it, so that we'll make sure to append a
-    // change to the list for ourselves if we need a reflow.  We need this
-    // because the parent may or may not actually end up reflowing us
-    // otherwise.
-    aMinChange = NS_SubtractHint(aMinChange, nsChangeHint_ReflowFrame);
-  } else if (!NS_IsHintSubset(nsChangeHint_ClearDescendantIntrinsics,
-                              aMinChange)) {
-    // If aMinChange doesn't include nsChangeHint_ClearDescendantIntrinsics,
-    // clear out the nsChangeHint_ClearAncestorIntrinsics flag, since it's
-    // possible that we had some random ancestor that cleared ancestor
-    // intrinsic widths, but we still need to clear intrinsic widths on frames
-    // that are our ancestors but its descendants.
-    aMinChange =
-      NS_SubtractHint(aMinChange, nsChangeHint_ClearAncestorIntrinsics);
-  }
-
   // We need to generate a new change list entry for every frame whose style
   // comparision returns one of these hints. These hints don't automatically
   // update all their descendant frames.
-  aMinChange = NS_SubtractHint(aMinChange, nsChangeHint_NonInherited_Hints);
+  aMinChange = NS_SubtractHint(aMinChange, NS_HintsNotHandledForDescendantsIn(aMinChange));
 
   // It would be nice if we could make stronger assertions here; they
   // would let us simplify the ?: expressions below setting |content|
   // and |pseudoContent| in sensible ways as well as making what
   // |localContent|, |content|, and |pseudoContent| mean make more
   // sense.  However, we can't, because of frame trees like the one in
   // https://bugzilla.mozilla.org/show_bug.cgi?id=472353#c14 .  Once we
   // fix bug 242277 we should be able to make this make more sense.
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -394,18 +394,17 @@ nsStyleContext::CalcStyleDifference(nsSt
       if ((compare || nsStyle##struct_::ForceCompare()) &&                    \
           !NS_IsHintSubset(nsStyle##struct_::MaxDifference(), hint) &&        \
           this##struct_ != other##struct_) {                                  \
         NS_ASSERTION(NS_IsHintSubset(                                         \
              this##struct_->CalcDifference(*other##struct_),                  \
              nsStyle##struct_::MaxDifference()),                              \
              "CalcDifference() returned bigger hint than MaxDifference()");   \
         NS_ASSERTION(nsStyle##struct_::ForceCompare() ||                      \
-             NS_IsHintSubset(nsStyle##struct_::MaxDifference(),               \
-                             nsChangeHint(~nsChangeHint_NonInherited_Hints)), \
+             NS_HintsNotHandledForDescendantsIn(nsStyle##struct_::MaxDifference()) == 0,  \
              "Structs that can return non-inherited hints must return true "  \
              "from ForceCompare");                                            \
         NS_UpdateHint(hint, this##struct_->CalcDifference(*other##struct_));  \
       }                                                                       \
     }                                                                         \
   PR_END_MACRO
 
   // In general, we want to examine structs starting with those that can