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
--- 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();