Bug 1321698 part 2: Use the new frame state bit to check for -webkit-box containers. r?mats draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 02 Dec 2016 10:32:31 -0800
changeset 447131 0a9e41868f9a01758956834b925a6c1f92543126
parent 447013 2beb57f80e5481e2e412a78375ba50ef6f476e6e
child 447132 0e31d268dd118daebd1c86875b7a7c0b14ea5d02
push id37996
push userdholbert@mozilla.com
push dateFri, 02 Dec 2016 18:51:22 +0000
reviewersmats
bugs1321698
milestone53.0a1
Bug 1321698 part 2: Use the new frame state bit to check for -webkit-box containers. r?mats Note that at the callsites in nsCSSFrameConstructor.cpp, we have to also check the frame type (since the frame state bit is in a range of bits whose meaning differs depending on frame type). The first change in this patch is the addition of a convenience fucntion that checks both the frame type as well as the frame state bit. MozReview-Commit-ID: DEOThTX5NAO
layout/base/nsCSSFrameConstructor.cpp
layout/generic/nsFlexContainerFrame.cpp
layout/generic/nsFlexContainerFrame.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -335,16 +335,30 @@ IsAnonymousFlexOrGridItem(const nsIFrame
 static inline bool
 IsFlexOrGridContainer(const nsIFrame* aFrame)
 {
   const nsIAtom* t = aFrame->GetType();
   return t == nsGkAtoms::flexContainerFrame ||
          t == nsGkAtoms::gridContainerFrame;
 }
 
+// Returns true IFF the given nsIFrame is a nsFlexContainerFrame and
+// represents a -webkit-{inline-}box container. The frame's GetType() result is
+// passed as a separate argument, since in most cases, the caller will have
+// looked it up already, and we'd rather not make redundant calls to the
+// virtual GetType() method.
+static inline bool
+IsFlexContainerForLegacyBox(const nsIFrame* aFrame,
+                            const nsIAtom* aFrameType)
+{
+  MOZ_ASSERT(aFrame->GetType() == aFrameType, "wrong aFrameType was passed");
+  return aFrameType == nsGkAtoms::flexContainerFrame &&
+    aFrame->HasAnyStateBits(NS_STATE_FLEX_IS_LEGACY_WEBKIT_BOX);
+}
+
 #if DEBUG
 static void
 AssertAnonymousFlexOrGridItemParent(const nsIFrame* aChild,
                                     const nsIFrame* aParent)
 {
   MOZ_ASSERT(IsAnonymousFlexOrGridItem(aChild),
              "expected an anonymous flex or grid item child frame");
   MOZ_ASSERT(aParent, "expected a parent frame");
@@ -9916,23 +9930,27 @@ nsCSSFrameConstructor::sPseudoParentData
 };
 
 void
 nsCSSFrameConstructor::CreateNeededAnonFlexOrGridItems(
   nsFrameConstructorState& aState,
   FrameConstructionItemList& aItems,
   nsIFrame* aParentFrame)
 {
-  if (aItems.IsEmpty() ||
-      !::IsFlexOrGridContainer(aParentFrame)) {
+  if (aItems.IsEmpty()) {
     return;
   }
-
-  const bool isWebkitBox = nsFlexContainerFrame::IsLegacyBox(aParentFrame);
-
+  const nsIAtom* parentType = aParentFrame->GetType();
+  if (parentType != nsGkAtoms::flexContainerFrame &&
+      parentType != nsGkAtoms::gridContainerFrame) {
+    return;
+  }
+
+  const bool isWebkitBox = IsFlexContainerForLegacyBox(aParentFrame,
+                                                       parentType);
   FCItemIterator iter(aItems);
   do {
     // Advance iter past children that don't want to be wrapped
     if (iter.SkipItemsThatDontNeedAnonFlexOrGridItem(aState, isWebkitBox)) {
       // Hit the end of the items without finding any remaining children that
       // need to be wrapped. We're finished!
       return;
     }
@@ -12214,22 +12232,24 @@ nsCSSFrameConstructor::WipeContainingBlo
     return true;
   }
 
   nsIFrame* nextSibling = ::GetInsertNextSibling(aFrame, aPrevSibling);
 
   // Situation #2 is a flex or grid container frame into which we're inserting
   // new inline non-replaced children, adjacent to an existing anonymous
   // flex or grid item.
-  if (::IsFlexOrGridContainer(aFrame)) {
+  nsIAtom* frameType = aFrame->GetType();
+  if (frameType == nsGkAtoms::flexContainerFrame ||
+      frameType == nsGkAtoms::gridContainerFrame) {
     FCItemIterator iter(aItems);
 
     // Check if we're adding to-be-wrapped content right *after* an existing
     // anonymous flex or grid item (which would need to absorb this content).
-    const bool isWebkitBox = nsFlexContainerFrame::IsLegacyBox(aFrame);
+    const bool isWebkitBox = IsFlexContainerForLegacyBox(aFrame, frameType);
     if (aPrevSibling && IsAnonymousFlexOrGridItem(aPrevSibling) &&
         iter.item().NeedsAnonFlexOrGridItem(aState, isWebkitBox)) {
       RecreateFramesForContent(aFrame->GetContent(), true,
                                REMOVE_FOR_RECONSTRUCTION, nullptr);
       return true;
     }
 
     // Check if we're adding to-be-wrapped content right *before* an existing
@@ -12259,17 +12279,18 @@ nsCSSFrameConstructor::WipeContainingBlo
     // _flex/grid container_ as its parent in the content tree.
     nsFrameConstructorSaveState floatSaveState;
     aState.PushFloatContainingBlock(nullptr, floatSaveState);
 
     FCItemIterator iter(aItems);
     // Skip over things that _do_ need an anonymous flex item, because
     // they're perfectly happy to go here -- they won't cause a reframe.
     nsIFrame* containerFrame = aFrame->GetParent();
-    const bool isWebkitBox = nsFlexContainerFrame::IsLegacyBox(containerFrame);
+    const bool isWebkitBox =
+      IsFlexContainerForLegacyBox(containerFrame, containerFrame->GetType());
     if (!iter.SkipItemsThatNeedAnonFlexOrGridItem(aState,
                                                   isWebkitBox)) {
       // We hit something that _doesn't_ need an anonymous flex item!
       // Rebuild the flex container to bust it out.
       RecreateFramesForContent(containerFrame->GetContent(), true,
                                REMOVE_FOR_RECONSTRUCTION, nullptr);
       return true;
     }
@@ -12283,17 +12304,16 @@ nsCSSFrameConstructor::WipeContainingBlo
   // spaces. It containes these two special cases apart from tables:
   // 1) There are effectively three types of white spaces in ruby frames
   //    we handle differently: leading/tailing/inter-level space,
   //    inter-base/inter-annotation space, and inter-segment space.
   //    These three types of spaces can be converted to each other when
   //    their sibling changes.
   // 2) The first effective child of a ruby frame must always be a ruby
   //    base container. It should be created or destroyed accordingly.
-  nsIAtom* frameType = aFrame->GetType();
   if (IsRubyPseudo(aFrame) ||
       frameType == nsGkAtoms::rubyFrame ||
       RubyUtils::IsRubyContainerBox(frameType)) {
     // We want to optimize it better, and avoid reframing as much as
     // possible. But given the cases above, and the fact that a ruby
     // usually won't be very large, it should be fine to reframe it.
     RecreateFramesForContent(aFrame->GetContent(), true,
                              REMOVE_FOR_RECONSTRUCTION, nullptr);
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -82,20 +82,32 @@ kAxisOrientationToSidesMap[eNumAxisOrien
 // Returns true iff the given nsStyleDisplay has display:-webkit-{inline-}-box.
 static inline bool
 IsDisplayValueLegacyBox(const nsStyleDisplay* aStyleDisp)
 {
   return aStyleDisp->mDisplay == mozilla::StyleDisplay::WebkitBox ||
     aStyleDisp->mDisplay == mozilla::StyleDisplay::WebkitInlineBox;
 }
 
+// Returns true if aFlexContainer is the frame for an element with
+// "display:-webkit-box" or "display:-webkit-inline-box". aFlexContainer is
+// expected to be an instance of nsFlexContainerFrame (enforced with an assert);
+// otherwise, this function's state-bit-check here is bogus.
+static bool
+IsLegacyBox(const nsIFrame* aFlexContainer)
+{
+  MOZ_ASSERT(aFlexContainer->GetType() == nsGkAtoms::flexContainerFrame,
+             "only flex containers may be passed to this function");
+  return aFlexContainer->HasAnyStateBits(NS_STATE_FLEX_IS_LEGACY_WEBKIT_BOX);
+}
+
 // XXXdholbert This will be merged into Init(), in a later patch in this series
 // (after all callers have been converted to check frame state bit).
-/* static */ bool
-nsFlexContainerFrame::IsLegacyBox(const nsIFrame* aFrame)
+static bool
+IsLegacyBoxFOLD_ME(const nsIFrame* aFrame)
 {
   nsStyleContext* styleContext = aFrame->StyleContext();
   const nsStyleDisplay* styleDisp = styleContext->StyleDisplay();
 
   // Trivial case: just check "display" directly.
   bool isLegacyBox = IsDisplayValueLegacyBox(styleDisp);
 
   // If this frame is for a scrollable element, then it will actually have
@@ -1128,17 +1140,17 @@ IsOrderLEQWithDOMFallback(nsIFrame* aFra
   if (aFrame1->GetType() == nsGkAtoms::placeholderFrame ||
       aFrame2->GetType() == nsGkAtoms::placeholderFrame) {
     // Treat placeholders (for abspos/fixedpos frames) as LEQ everything.  This
     // ensures we don't reorder them w.r.t. one another, which is sufficient to
     // prevent them from noticeably participating in "order" reordering.
     return true;
   }
 
-  bool isInLegacyBox = nsFlexContainerFrame::IsLegacyBox(aFrame1->GetParent());
+  const bool isInLegacyBox = IsLegacyBox(aFrame1->GetParent());
 
   int32_t order1 = GetOrderOrBoxOrdinalGroup(aFrame1, isInLegacyBox);
   int32_t order2 = GetOrderOrBoxOrdinalGroup(aFrame2, isInLegacyBox);
 
   if (order1 != order2) {
     return order1 < order2;
   }
 
@@ -1206,17 +1218,17 @@ IsOrderLEQ(nsIFrame* aFrame1,
   if (aFrame1->GetType() == nsGkAtoms::placeholderFrame ||
       aFrame2->GetType() == nsGkAtoms::placeholderFrame) {
     // Treat placeholders (for abspos/fixedpos frames) as LEQ everything.  This
     // ensures we don't reorder them w.r.t. one another, which is sufficient to
     // prevent them from noticeably participating in "order" reordering.
     return true;
   }
 
-  bool isInLegacyBox = nsFlexContainerFrame::IsLegacyBox(aFrame1->GetParent());
+  const bool isInLegacyBox = IsLegacyBox(aFrame1->GetParent());
 
   int32_t order1 = GetOrderOrBoxOrdinalGroup(aFrame1, isInLegacyBox);
   int32_t order2 = GetOrderOrBoxOrdinalGroup(aFrame2, isInLegacyBox);
 
   return order1 <= order2;
 }
 
 uint8_t
@@ -2279,17 +2291,17 @@ nsFlexContainerFrame::~nsFlexContainerFr
 /* virtual */
 void
 nsFlexContainerFrame::Init(nsIContent*       aContent,
                            nsContainerFrame* aParent,
                            nsIFrame*         aPrevInFlow)
 {
   nsContainerFrame::Init(aContent, aParent, aPrevInFlow);
 
-  if (nsFlexContainerFrame::IsLegacyBox(this)) {
+  if (IsLegacyBoxFOLD_ME(this)) {
     // Toggle frame state bit to indicate that this frame represents a
     // legacy -webkit-{inline-}box container:
     AddStateBits(NS_STATE_FLEX_IS_LEGACY_WEBKIT_BOX);
   }
 }
 
 template<bool IsLessThanOrEqual(nsIFrame*, nsIFrame*)>
 /* static */ bool
--- a/layout/generic/nsFlexContainerFrame.h
+++ b/layout/generic/nsFlexContainerFrame.h
@@ -106,22 +106,16 @@ public:
     *                                     space to be divided up.
     */
   static void CalculatePackingSpace(uint32_t aNumThingsToPack,
                                     uint8_t aAlignVal,
                                     nscoord* aFirstSubjectOffset,
                                     uint32_t* aNumPackingSpacesRemaining,
                                     nscoord* aPackingSpaceRemaining);
 
-  /**
-   * Returns true if aFrame is the frame for an element with
-   * "display:-webkit-box" or "display:-webkit-inline-box".
-   */
-  static bool IsLegacyBox(const nsIFrame* aFrame);
-
 protected:
   // Protected constructor & destructor
   explicit nsFlexContainerFrame(nsStyleContext* aContext)
     : nsContainerFrame(aContext)
     , mBaselineFromLastReflow(NS_INTRINSIC_WIDTH_UNKNOWN)
   {}
   virtual ~nsFlexContainerFrame();