Bug 1321698 part 2: Use the new frame state bit to check for -webkit-box containers. r=mats
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 02 Dec 2016 10:32:31 -0800
changeset 325152 e38ccf5210624c1a4906f893fc2f7932d4e76a5e
parent 325151 15f43157a6e1241e6203ac47871e4b1e59ed2585
child 325153 eeb34d6bc82cbc44e9dec0005faa6dedb3d1b318
push id84610
push userphilringnalda@gmail.com
push dateSat, 03 Dec 2016 06:28:13 +0000
treeherdermozilla-inbound@1b2237e0b5e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1321698
milestone53.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 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();