Bug 1009214 part 1 - [css-grid] Don't wrap placeholders in an anonymous grid item. r=dholbert
authorMats Palmgren <matspal@gmail.com>
Tue, 05 May 2015 21:53:22 +0000
changeset 273849 55d911f8cb6a61621866feab66afd0ee59dfd3ce
parent 273848 6ff073aa7f130450ac8ede04cd88231c57bcc216
child 273850 4eca79afce17755cf6833d0d340c88bf076f8b3d
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1009214
milestone40.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 1009214 part 1 - [css-grid] Don't wrap placeholders in an anonymous grid item. r=dholbert
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/generic/nsGridContainerFrame.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -9574,17 +9574,17 @@ nsCSSFrameConstructor::CreateNeededAnonF
       !::IsFlexOrGridContainer(aParentFrame)) {
     return;
   }
 
   nsIAtom* containerType = aParentFrame->GetType();
   FCItemIterator iter(aItems);
   do {
     // Advance iter past children that don't want to be wrapped
-    if (iter.SkipItemsThatDontNeedAnonFlexOrGridItem(aState)) {
+    if (iter.SkipItemsThatDontNeedAnonFlexOrGridItem(aState, containerType)) {
       // Hit the end of the items without finding any remaining children that
       // need to be wrapped. We're finished!
       return;
     }
 
     // If our next potentially-wrappable child is whitespace, then see if
     // there's anything wrappable immediately after it. If not, we just drop
     // the whitespace and move on. (We're not supposed to create any anonymous
@@ -9596,41 +9596,42 @@ nsCSSFrameConstructor::CreateNeededAnonF
     // entirely whitespace*, then we technically should still skip over it, per
     // the CSS grid & flexbox specs. I'm not bothering with that at this point,
     // since it's a pretty extreme edge case.
     if (!aParentFrame->IsGeneratedContentFrame() &&
         iter.item().IsWhitespace(aState)) {
       FCItemIterator afterWhitespaceIter(iter);
       bool hitEnd = afterWhitespaceIter.SkipWhitespace(aState);
       bool nextChildNeedsAnonItem =
-        !hitEnd && afterWhitespaceIter.item().NeedsAnonFlexOrGridItem(aState);
+        !hitEnd && afterWhitespaceIter.item().NeedsAnonFlexOrGridItem(aState,
+                                                containerType);
 
       if (!nextChildNeedsAnonItem) {
         // There's nothing after the whitespace that we need to wrap, so we
         // just drop this run of whitespace.
         iter.DeleteItemsTo(afterWhitespaceIter);
         if (hitEnd) {
           // Nothing left to do -- we're finished!
           return;
         }
         // else, we have a next child and it does not want to be wrapped.  So,
         // we jump back to the beginning of the loop to skip over that child
         // (and anything else non-wrappable after it)
         MOZ_ASSERT(!iter.IsDone() &&
-                   !iter.item().NeedsAnonFlexOrGridItem(aState),
+                   !iter.item().NeedsAnonFlexOrGridItem(aState, containerType),
                    "hitEnd and/or nextChildNeedsAnonItem lied");
         continue;
       }
     }
 
     // Now |iter| points to the first child that needs to be wrapped in an
     // anonymous flex/grid item. Now we see how many children after it also want
     // to be wrapped in an anonymous flex/grid item.
     FCItemIterator endIter(iter); // iterator to find the end of the group
-    endIter.SkipItemsThatNeedAnonFlexOrGridItem(aState);
+    endIter.SkipItemsThatNeedAnonFlexOrGridItem(aState, containerType);
 
     NS_ASSERTION(iter != endIter,
                  "Should've had at least one wrappable child to seek past");
 
     // Now, we create the anonymous flex or grid item to contain the children
     // between |iter| and |endIter|.
     nsIAtom* pseudoType = containerType == nsGkAtoms::flexContainerFrame ?
       nsCSSAnonBoxes::anonymousFlexItem : nsCSSAnonBoxes::anonymousGridItem;
@@ -11826,30 +11827,31 @@ nsCSSFrameConstructor::WipeContainingBlo
   // 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)) {
     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).
+    nsIAtom* containerType = aFrame->GetType();
     if (aPrevSibling && IsAnonymousFlexOrGridItem(aPrevSibling) &&
-        iter.item().NeedsAnonFlexOrGridItem(aState)) {
+        iter.item().NeedsAnonFlexOrGridItem(aState, containerType)) {
       RecreateFramesForContent(aFrame->GetContent(), true,
                                REMOVE_FOR_RECONSTRUCTION, nullptr);
       return true;
     }
 
     // Check if we're adding to-be-wrapped content right *before* an existing
     // anonymous flex or grid item (which would need to absorb this content).
     if (nextSibling && IsAnonymousFlexOrGridItem(nextSibling)) {
       // Jump to the last entry in the list
       iter.SetToEnd();
       iter.Prev();
-      if (iter.item().NeedsAnonFlexOrGridItem(aState)) {
+      if (iter.item().NeedsAnonFlexOrGridItem(aState, containerType)) {
         RecreateFramesForContent(aFrame->GetContent(), true,
                                  REMOVE_FOR_RECONSTRUCTION, nullptr);
         return true;
       }
     }
   }
 
   // Situation #3 is an anonymous flex or grid item that's getting new children
@@ -11864,20 +11866,21 @@ nsCSSFrameConstructor::WipeContainingBlo
     // We're not honoring floats on this content because it has the
     // _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.
-    if (!iter.SkipItemsThatNeedAnonFlexOrGridItem(aState)) {
+    nsIFrame* containerFrame = aFrame->GetParent();
+    if (!iter.SkipItemsThatNeedAnonFlexOrGridItem(aState,
+                                                  containerFrame->GetType())) {
       // We hit something that _doesn't_ need an anonymous flex item!
       // Rebuild the flex container to bust it out.
-      nsIFrame* containerFrame = aFrame->GetParent();
       RecreateFramesForContent(containerFrame->GetContent(), true,
                                REMOVE_FOR_RECONSTRUCTION, nullptr);
       return true;
     }
 
     // If we get here, then everything in |aItems| needs to be wrapped in
     // an anonymous flex or grid item.  That's where it's already going - good!
   }
@@ -12319,57 +12322,62 @@ Iterator::SkipItemsNotWantingParentType(
       return true;
     }
   }
   return false;
 }
 
 bool
 nsCSSFrameConstructor::FrameConstructionItem::
-  NeedsAnonFlexOrGridItem(const nsFrameConstructorState& aState)
+  NeedsAnonFlexOrGridItem(const nsFrameConstructorState& aState,
+                          nsIAtom* aContainerType)
 {
   if (mFCData->mBits & FCDATA_IS_LINE_PARTICIPANT) {
     // This will be an inline non-replaced box.
     return true;
   }
 
-  if (!(mFCData->mBits & FCDATA_DISALLOW_OUT_OF_FLOW) &&
+  // Bug 874718: Flex containers still wrap placeholders; Grid containers don't.
+  if (aContainerType == nsGkAtoms::flexContainerFrame &&
+      !(mFCData->mBits & FCDATA_DISALLOW_OUT_OF_FLOW) &&
       aState.GetGeometricParent(mStyleContext->StyleDisplay(), nullptr)) {
     // We're abspos or fixedpos, which means we'll spawn a placeholder which
     // we'll need to wrap in an anonymous flex item.  So, we just treat
     // _this_ frame as if _it_ needs to be wrapped in an anonymous flex item,
     // and then when we spawn the placeholder, it'll end up in the right spot.
     return true;
   }
 
   return false;
 }
 
 inline bool
 nsCSSFrameConstructor::FrameConstructionItemList::
 Iterator::SkipItemsThatNeedAnonFlexOrGridItem(
-  const nsFrameConstructorState& aState)
+  const nsFrameConstructorState& aState,
+  nsIAtom* aContainerType)
 {
   NS_PRECONDITION(!IsDone(), "Shouldn't be done yet");
-  while (item().NeedsAnonFlexOrGridItem(aState)) {
+  while (item().NeedsAnonFlexOrGridItem(aState, aContainerType)) {
     Next();
     if (IsDone()) {
       return true;
     }
   }
   return false;
 }
 
 inline bool
 nsCSSFrameConstructor::FrameConstructionItemList::
 Iterator::SkipItemsThatDontNeedAnonFlexOrGridItem(
-  const nsFrameConstructorState& aState)
+  const nsFrameConstructorState& aState,
+  nsIAtom* aContainerType)
 {
   NS_PRECONDITION(!IsDone(), "Shouldn't be done yet");
-  while (!(item().NeedsAnonFlexOrGridItem(aState))) {
+  while (!(item().NeedsAnonFlexOrGridItem(aState, aContainerType))) {
     Next();
     if (IsDone()) {
       return true;
     }
   }
   return false;
 }
 
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -926,23 +926,23 @@ private:
       // one.  Return whether the iterator is done after doing that.  The
       // iterator must not be done when this is called.
       inline bool SkipItemsNotWantingParentType(ParentType aParentType);
 
       // Skip over non-replaced inline frames and positioned frames.
       // Return whether the iterator is done after doing that.
       // The iterator must not be done when this is called.
       inline bool SkipItemsThatNeedAnonFlexOrGridItem(
-        const nsFrameConstructorState& aState);
+        const nsFrameConstructorState& aState, nsIAtom* aContainerType);
 
       // Skip to the first frame that is a non-replaced inline or is
       // positioned.  Return whether the iterator is done after doing that.
       // The iterator must not be done when this is called.
       inline bool SkipItemsThatDontNeedAnonFlexOrGridItem(
-        const nsFrameConstructorState& aState);
+        const nsFrameConstructorState& aState, nsIAtom* aContainerType);
 
       // Skip over all items that do not want a ruby parent.  Return whether
       // the iterator is done after doing that.  The iterator must not be done
       // when this is called.
       inline bool SkipItemsNotWantingRubyParent();
 
       // Skip over whitespace.  Return whether the iterator is done after doing
       // that.  The iterator must not be done, and must be pointing to a
@@ -1069,17 +1069,18 @@ private:
     }
 
     ParentType DesiredParentType() {
       return FCDATA_DESIRED_PARENT_TYPE(mFCData->mBits);
     }
 
     // Indicates whether (when in a flex or grid container) this item needs
     // to be wrapped in an anonymous block.
-    bool NeedsAnonFlexOrGridItem(const nsFrameConstructorState& aState);
+    bool NeedsAnonFlexOrGridItem(const nsFrameConstructorState& aState,
+                                 nsIAtom* aContainerType);
 
     // Don't call this unless the frametree really depends on the answer!
     // Especially so for generated content, where we don't want to reframe
     // things.
     bool IsWhitespace(nsFrameConstructorState& aState) const;
 
     bool IsLineBoundary() const {
       return mIsBlock || (mFCData->mBits & FCDATA_IS_LINE_BREAK);
--- a/layout/generic/nsGridContainerFrame.cpp
+++ b/layout/generic/nsGridContainerFrame.cpp
@@ -29,21 +29,24 @@ typedef nsGridContainerFrame::TrackSize 
 const uint32_t nsGridContainerFrame::kAutoLine = 12345U;
 const uint32_t nsGridContainerFrame::kTranslatedMaxLine =
   uint32_t(nsStyleGridLine::kMaxLine - nsStyleGridLine::kMinLine - 1);
 
 class nsGridContainerFrame::GridItemCSSOrderIterator
 {
 public:
   enum OrderState { eUnknownOrder, eKnownOrdered, eKnownUnordered };
+  enum ChildFilter { eSkipPlaceholders, eIncludeAll };
   GridItemCSSOrderIterator(nsIFrame* aGridContainer,
                            nsIFrame::ChildListID aListID,
+                           ChildFilter aFilter = eSkipPlaceholders,
                            OrderState aState = eUnknownOrder)
     : mChildren(aGridContainer->GetChildList(aListID))
     , mArrayIndex(0)
+    , mSkipPlaceholders(aFilter == eSkipPlaceholders)
 #ifdef DEBUG
     , mGridContainer(aGridContainer)
     , mListID(aListID)
 #endif
   {
     size_t count = 0;
     bool isOrdered = aState != eKnownUnordered;
     if (aState == eUnknownOrder) {
@@ -64,27 +67,53 @@ public:
       count *= 2; // XXX somewhat arbitrary estimate for now...
       mArray.emplace(count);
       for (nsFrameList::Enumerator e(mChildren); !e.AtEnd(); e.Next()) {
         mArray->AppendElement(e.get());
       }
       // XXX replace this with nsTArray::StableSort when bug 1147091 is fixed.
       std::stable_sort(mArray->begin(), mArray->end(), IsCSSOrderLessThan);
     }
+
+    if (mSkipPlaceholders) {
+      SkipPlaceholders();
+    }
   }
 
   nsIFrame* operator*() const
   {
     MOZ_ASSERT(!AtEnd());
     if (mEnumerator) {
       return mEnumerator->get();
     }
     return (*mArray)[mArrayIndex];
   }
 
+  /**
+   * Skip over placeholder children.
+   */
+  void SkipPlaceholders()
+  {
+    if (mEnumerator) {
+      for (; !mEnumerator->AtEnd(); mEnumerator->Next()) {
+        nsIFrame* child = mEnumerator->get();
+        if (child->GetType() != nsGkAtoms::placeholderFrame) {
+          return;
+        }
+      }
+    } else {
+      for (; mArrayIndex < mArray->Length(); ++mArrayIndex) {
+        nsIFrame* child = (*mArray)[mArrayIndex];
+        if (child->GetType() != nsGkAtoms::placeholderFrame) {
+          return;
+        }
+      }
+    }
+  }
+
   bool AtEnd() const
   {
     MOZ_ASSERT(mEnumerator || mArrayIndex <= mArray->Length());
     return mEnumerator ? mEnumerator->AtEnd() : mArrayIndex >= mArray->Length();
   }
 
   void Next()
   {
@@ -95,40 +124,49 @@ public:
                "the list of child frames must not change while iterating!");
 #endif
     if (mEnumerator) {
       mEnumerator->Next();
     } else {
       MOZ_ASSERT(mArrayIndex < mArray->Length(), "iterating past end");
       ++mArrayIndex;
     }
+    if (mSkipPlaceholders) {
+      SkipPlaceholders();
+    }
   }
 
-  void Reset()
+  void Reset(ChildFilter aFilter = eSkipPlaceholders)
   {
     if (mEnumerator) {
       mEnumerator.reset();
       mEnumerator.emplace(mChildren);
     } else {
       mArrayIndex = 0;
     }
+    mSkipPlaceholders = aFilter == eSkipPlaceholders;
+    if (mSkipPlaceholders) {
+      SkipPlaceholders();
+    }
   }
 
   bool ItemsAreAlreadyInOrder() const { return mEnumerator.isSome(); }
 
 private:
   static bool IsCSSOrderLessThan(nsIFrame* const& a, nsIFrame* const& b)
     { return a->StylePosition()->mOrder < b->StylePosition()->mOrder; }
 
   nsFrameList mChildren;
   // Used if child list is already in ascending 'order'.
   Maybe<nsFrameList::Enumerator> mEnumerator;
   // Used if child list is *not* in ascending 'order'.
   Maybe<nsTArray<nsIFrame*>> mArray;
   size_t mArrayIndex;
+  // Skip placeholder children in the iteration?
+  bool mSkipPlaceholders;
 #ifdef DEBUG
   nsIFrame* mGridContainer;
   nsIFrame::ChildListID mListID;
 #endif
 };
 
 /**
  * Search for the aNth occurrence of aName in aNameList (forward), starting at
@@ -1208,23 +1246,29 @@ nsGridContainerFrame::ReflowChildren(Gri
 {
   WritingMode wm = aReflowState.GetWritingMode();
   const LogicalPoint gridOrigin(aContentArea.Origin(wm));
   const nscoord containerWidth = aContentArea.Width(wm) +
     aReflowState.ComputedPhysicalBorderPadding().LeftRight();
   nsPresContext* pc = PresContext();
   for (; !aIter.AtEnd(); aIter.Next()) {
     nsIFrame* child = *aIter;
-    GridArea* area = GetGridAreaForChild(child);
-    MOZ_ASSERT(area && area->IsDefinite());
-    LogicalRect cb = ContainingBlockFor(wm, *area, aColSizes, aRowSizes);
-    cb += gridOrigin;
+    const bool isGridItem = child->GetType() != nsGkAtoms::placeholderFrame;
+    LogicalRect cb(wm);
+    if (MOZ_LIKELY(isGridItem)) {
+      GridArea* area = GetGridAreaForChild(child);
+      MOZ_ASSERT(area && area->IsDefinite());
+      cb = ContainingBlockFor(wm, *area, aColSizes, aRowSizes);
+      cb += gridOrigin;
+    } else {
+      cb = aContentArea;
+    }
     nsHTMLReflowState childRS(pc, aReflowState, child, cb.Size(wm));
     const LogicalMargin margin = childRS.ComputedLogicalMargin();
-    if (childRS.ComputedBSize() == NS_AUTOHEIGHT) {
+    if (childRS.ComputedBSize() == NS_AUTOHEIGHT && MOZ_LIKELY(isGridItem)) {
       // XXX the start of an align-self:stretch impl.  Needs min-/max-bsize
       // clamping though, and check the prop value is actually 'stretch'!
       LogicalMargin bp = childRS.ComputedLogicalBorderPadding();
       bp.ApplySkipSides(child->GetLogicalSkipSides());
       nscoord bSize = cb.BSize(wm) - bp.BStartEnd(wm) - margin.BStartEnd(wm);
       childRS.SetComputedBSize(std::max(bSize, 0));
     }
     LogicalPoint childPos = cb.Origin(wm);
@@ -1326,17 +1370,17 @@ nsGridContainerFrame::Reflow(nsPresConte
   bSize = std::max(bSize - GetConsumedBSize(), 0);
   LogicalSize desiredSize(wm, computedISize + bp.IStartEnd(wm),
                           bSize + bp.BStartEnd(wm));
   aDesiredSize.SetSize(wm, desiredSize);
   aDesiredSize.SetOverflowAreasToDesiredBounds();
 
   LogicalRect contentArea(wm, bp.IStart(wm), bp.BStart(wm),
                           computedISize, bSize);
-  normalFlowIter.Reset();
+  normalFlowIter.Reset(GridItemCSSOrderIterator::eIncludeAll);
   ReflowChildren(normalFlowIter, contentArea, colSizes, rowSizes, aDesiredSize,
                  aReflowState, aStatus);
 
   FinishAndStoreOverflow(&aDesiredSize);
   aStatus = NS_FRAME_COMPLETE;
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowState, aDesiredSize);
 }
 
@@ -1363,17 +1407,18 @@ nsGridContainerFrame::BuildDisplayList(n
                               aLists.BlockBorderBackgrounds(),
                               aLists.Floats(),
                               aLists.Content(),
                               &positionedDescendants,
                               aLists.Outlines());
   typedef GridItemCSSOrderIterator::OrderState OrderState;
   OrderState order = mIsNormalFlowInCSSOrder ? OrderState::eKnownOrdered
                                              : OrderState::eKnownUnordered;
-  GridItemCSSOrderIterator iter(this, kPrincipalList, order);
+  GridItemCSSOrderIterator iter(this, kPrincipalList,
+                                GridItemCSSOrderIterator::eIncludeAll, order);
   for (; !iter.AtEnd(); iter.Next()) {
     nsIFrame* child = *iter;
     BuildDisplayListForChild(aBuilder, child, aDirtyRect, childLists,
                              ::GetDisplayFlagsForGridItem(child));
   }
   positionedDescendants.SortByCSSOrder(aBuilder);
   aLists.PositionedDescendants()->AppendToTop(&positionedDescendants);
 }
@@ -1433,18 +1478,17 @@ nsGridContainerFrame::CellMap::Dump() co
   }
 }
 
 static bool
 FrameWantsToBeInAnonymousGridItem(nsIFrame* aFrame)
 {
   // Note: This needs to match the logic in
   // nsCSSFrameConstructor::FrameConstructionItem::NeedsAnonFlexOrGridItem()
-  return (aFrame->IsFrameOfType(nsIFrame::eLineParticipant) ||
-          nsGkAtoms::placeholderFrame == aFrame->GetType());
+  return aFrame->IsFrameOfType(nsIFrame::eLineParticipant);
 }
 
 // Debugging method, to let us assert that our anonymous grid items are
 // set up correctly -- in particular, we assert:
 //  (1) we don't have any inline non-replaced children
 //  (2) we don't have any consecutive anonymous grid items
 //  (3) we don't have any empty anonymous grid items
 //  (4) all children are on the expected child lists