- Revision
- 287744
- Author
- [email protected]
- Date
- 2022-01-07 07:08:00 -0800 (Fri, 07 Jan 2022)
Log Message
[Cleanup] RenderElement::containingBlockFor*(fixed/absolute/inflow)Position is slightly confusing
https://bugs.webkit.org/show_bug.cgi?id=234939
Reviewed by Antti Koivisto.
These 3 helper functions (containingBlockForFixedPosition/containingBlockForAbsolutePosition/containingBlockForObjectInFlow)
are expected to return an ancestor renderer which would act as the containing block if the renderer was
fixed/absolute/inflow positioned (in their current form they should read as containingBlockIfTheRendererWasFixedPositioned..)
These functions were introduced as part of LogicalSelectionOffsetCaches where we cache all 3 types of
containing blocks (fixed/absolute/inflow) to save containingBlock() calls as the cached object gets propagated
to ancestor renderers (so we really have a "what if" type of use case).
After some refactoring (and moving them out of LogicalSelectionOffsetCaches), we started introducing more and more
callsites of these functions where the "what if" question made less sense.
This patch replaces these 3 functions with a static helper:
static RenderBlock* containingBlockForPositionType(PositionType, const RenderObject&);
While it does not make the callsites look much cleaner, it helps to stop the spread of the special containing block handling for top-layer/backdrop boxes.
* dom/Element.cpp:
(WebCore::layoutOverflowRectContainsAllDescendants):
* rendering/LogicalSelectionOffsetCaches.h:
(WebCore::LogicalSelectionOffsetCaches::LogicalSelectionOffsetCaches):
* rendering/RenderElement.cpp:
(WebCore::nearestNonAnonymousContainingBlockIncludingSelf): Deleted.
(WebCore::RenderElement::containingBlockForFixedPosition const): Deleted.
(WebCore::RenderElement::containingBlockForAbsolutePosition const): Deleted.
* rendering/RenderElement.h:
* rendering/RenderInline.cpp:
(WebCore::RenderInline::styleWillChange):
* rendering/RenderLineBreak.cpp:
(WebCore::RenderLineBreak::collectSelectionGeometries):
* rendering/RenderObject.cpp:
(WebCore::nearestNonAnonymousContainingBlockIncludingSelf):
(WebCore::RenderObject::containingBlockForPositionType):
(WebCore::RenderObject::containingBlock const):
(WebCore::RenderObject::containingBlockForObjectInFlow const): Deleted.
* rendering/RenderObject.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (287743 => 287744)
--- trunk/Source/WebCore/ChangeLog 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/ChangeLog 2022-01-07 15:08:00 UTC (rev 287744)
@@ -1,3 +1,45 @@
+2022-01-07 Alan Bujtas <[email protected]>
+
+ [Cleanup] RenderElement::containingBlockFor*(fixed/absolute/inflow)Position is slightly confusing
+ https://bugs.webkit.org/show_bug.cgi?id=234939
+
+ Reviewed by Antti Koivisto.
+
+ These 3 helper functions (containingBlockForFixedPosition/containingBlockForAbsolutePosition/containingBlockForObjectInFlow)
+ are expected to return an ancestor renderer which would act as the containing block if the renderer was
+ fixed/absolute/inflow positioned (in their current form they should read as containingBlockIfTheRendererWasFixedPositioned..)
+
+ These functions were introduced as part of LogicalSelectionOffsetCaches where we cache all 3 types of
+ containing blocks (fixed/absolute/inflow) to save containingBlock() calls as the cached object gets propagated
+ to ancestor renderers (so we really have a "what if" type of use case).
+
+ After some refactoring (and moving them out of LogicalSelectionOffsetCaches), we started introducing more and more
+ callsites of these functions where the "what if" question made less sense.
+
+ This patch replaces these 3 functions with a static helper:
+ static RenderBlock* containingBlockForPositionType(PositionType, const RenderObject&);
+ While it does not make the callsites look much cleaner, it helps to stop the spread of the special containing block handling for top-layer/backdrop boxes.
+
+ * dom/Element.cpp:
+ (WebCore::layoutOverflowRectContainsAllDescendants):
+ * rendering/LogicalSelectionOffsetCaches.h:
+ (WebCore::LogicalSelectionOffsetCaches::LogicalSelectionOffsetCaches):
+ * rendering/RenderElement.cpp:
+ (WebCore::nearestNonAnonymousContainingBlockIncludingSelf): Deleted.
+ (WebCore::RenderElement::containingBlockForFixedPosition const): Deleted.
+ (WebCore::RenderElement::containingBlockForAbsolutePosition const): Deleted.
+ * rendering/RenderElement.h:
+ * rendering/RenderInline.cpp:
+ (WebCore::RenderInline::styleWillChange):
+ * rendering/RenderLineBreak.cpp:
+ (WebCore::RenderLineBreak::collectSelectionGeometries):
+ * rendering/RenderObject.cpp:
+ (WebCore::nearestNonAnonymousContainingBlockIncludingSelf):
+ (WebCore::RenderObject::containingBlockForPositionType):
+ (WebCore::RenderObject::containingBlock const):
+ (WebCore::RenderObject::containingBlockForObjectInFlow const): Deleted.
+ * rendering/RenderObject.h:
+
2022-01-06 Nikolas Zimmermann <[email protected]>
[LBSE] Rename RenderSVGRect -> LegacyRenderSVGRect
Modified: trunk/Source/WebCore/dom/Element.cpp (287743 => 287744)
--- trunk/Source/WebCore/dom/Element.cpp 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/dom/Element.cpp 2022-01-07 15:08:00 UTC (rev 287744)
@@ -1585,7 +1585,7 @@
}
// This renderer may have positioned descendants whose containing block is some ancestor.
- if (auto* containingBlock = renderBox.containingBlockForAbsolutePosition()) {
+ if (auto* containingBlock = RenderObject::containingBlockForPositionType(PositionType::Absolute, renderBox)) {
if (auto* positionedObjects = containingBlock->positionedObjects()) {
for (auto* positionedBox : *positionedObjects) {
if (positionedBox == &renderBox)
Modified: trunk/Source/WebCore/rendering/LogicalSelectionOffsetCaches.h (287743 => 287744)
--- trunk/Source/WebCore/rendering/LogicalSelectionOffsetCaches.h 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/rendering/LogicalSelectionOffsetCaches.h 2022-01-07 15:08:00 UTC (rev 287744)
@@ -89,9 +89,9 @@
ASSERT(rootBlock.isSelectionRoot());
#endif
// LogicalSelectionOffsetCaches should not be used on an orphaned tree.
- m_containingBlockForFixedPosition.setBlock(rootBlock.containingBlockForFixedPosition(), nullptr);
- m_containingBlockForAbsolutePosition.setBlock(rootBlock.containingBlockForAbsolutePosition(), nullptr);
- m_containingBlockForInflowPosition.setBlock(rootBlock.containingBlockForObjectInFlow(), nullptr);
+ m_containingBlockForFixedPosition.setBlock(RenderObject::containingBlockForPositionType(PositionType::Fixed, rootBlock), nullptr);
+ m_containingBlockForAbsolutePosition.setBlock(RenderObject::containingBlockForPositionType(PositionType::Absolute, rootBlock), nullptr);
+ m_containingBlockForInflowPosition.setBlock(RenderObject::containingBlockForPositionType(PositionType::Static, rootBlock), nullptr);
}
LogicalSelectionOffsetCaches(RenderBlock& block, const LogicalSelectionOffsetCaches& cache)
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (287743 => 287744)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2022-01-07 15:08:00 UTC (rev 287744)
@@ -634,35 +634,6 @@
return RenderPtr<RenderObject>(&renderer);
}
-static inline RenderBlock* nearestNonAnonymousContainingBlockIncludingSelf(RenderElement* renderer)
-{
- while (renderer && (!is<RenderBlock>(*renderer) || renderer->isAnonymousBlock()))
- renderer = renderer->containingBlock();
- return downcast<RenderBlock>(renderer);
-}
-
-RenderBlock* RenderElement::containingBlockForFixedPosition() const
-{
- auto* ancestor = parent();
- while (ancestor && !ancestor->canContainFixedPositionObjects())
- ancestor = ancestor->parent();
- return nearestNonAnonymousContainingBlockIncludingSelf(ancestor);
-}
-
-RenderBlock* RenderElement::containingBlockForAbsolutePosition() const
-{
- if (is<RenderInline>(*this) && style().position() == PositionType::Relative) {
- // A relatively positioned RenderInline forwards its absolute positioned descendants to
- // its nearest non-anonymous containing block (to avoid having positioned objects list in RenderInlines).
- return nearestNonAnonymousContainingBlockIncludingSelf(parent());
- }
- auto* ancestor = parent();
- while (ancestor && !ancestor->canContainAbsolutelyPositionedObjects())
- ancestor = ancestor->parent();
- // Make sure we only return non-anonymous RenderBlock as containing block.
- return nearestNonAnonymousContainingBlockIncludingSelf(ancestor);
-}
-
static void addLayers(RenderElement& renderer, RenderLayer* parentLayer, RenderElement*& newObject, RenderLayer*& beforeChild)
{
if (renderer.hasLayer()) {
Modified: trunk/Source/WebCore/rendering/RenderElement.h (287743 => 287744)
--- trunk/Source/WebCore/rendering/RenderElement.h 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/rendering/RenderElement.h 2022-01-07 15:08:00 UTC (rev 287744)
@@ -234,8 +234,6 @@
void adjustComputedFontSizesOnBlocks(float size, float visibleWidth);
WEBCORE_EXPORT void resetTextAutosizing();
#endif
- RenderBlock* containingBlockForFixedPosition() const;
- RenderBlock* containingBlockForAbsolutePosition() const;
WEBCORE_EXPORT ImageOrientation imageOrientation() const;
Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (287743 => 287744)
--- trunk/Source/WebCore/rendering/RenderInline.cpp 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp 2022-01-07 15:08:00 UTC (rev 287744)
@@ -159,7 +159,7 @@
// RenderInlines forward their absolute positioned descendants to their (non-anonymous) containing block.
// Check if this non-anonymous containing block can hold the absolute positioned elements when the inline is no longer positioned.
if (canContainAbsolutelyPositionedObjects() && newStyle.position() == PositionType::Static) {
- auto* container = containingBlockForAbsolutePosition();
+ auto* container = RenderObject::containingBlockForPositionType(PositionType::Absolute, *this);
if (container && !container->canContainAbsolutelyPositionedObjects())
container->removePositionedObjects(nullptr, NewContainingBlock);
}
Modified: trunk/Source/WebCore/rendering/RenderLineBreak.cpp (287743 => 287744)
--- trunk/Source/WebCore/rendering/RenderLineBreak.cpp 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/rendering/RenderLineBreak.cpp 2022-01-07 15:08:00 UTC (rev 287744)
@@ -199,7 +199,8 @@
rect.shiftXEdgeTo(line->lineBoxTop());
}
- auto* containingBlock = containingBlockForObjectInFlow();
+ // FIXME: Out-of-flow positioned line breaks do not follow normal containing block chain.
+ auto* containingBlock = RenderObject::containingBlockForPositionType(PositionType::Static, *this);
// Map rect, extended left to leftOffset, and right to rightOffset, through transforms to get minX and maxX.
LogicalSelectionOffsetCaches cache(*containingBlock);
LayoutUnit leftOffset = containingBlock->logicalLeftSelectionOffset(*containingBlock, LayoutUnit(run->logicalTop()), cache);
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (287743 => 287744)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2022-01-07 15:08:00 UTC (rev 287744)
@@ -656,21 +656,66 @@
downcast<RenderLayerModelObject>(*this).layer()->setRepaintStatus(NeedsFullRepaintForPositionedMovementLayout);
}
+static inline RenderBlock* nearestNonAnonymousContainingBlockIncludingSelf(RenderElement* renderer)
+{
+ while (renderer && (!is<RenderBlock>(*renderer) || renderer->isAnonymousBlock()))
+ renderer = renderer->containingBlock();
+ return downcast<RenderBlock>(renderer);
+}
+
+RenderBlock* RenderObject::containingBlockForPositionType(PositionType positionType, const RenderObject& renderer)
+{
+ if (positionType == PositionType::Static || positionType == PositionType::Relative || positionType == PositionType::Sticky) {
+ auto containingBlockForObjectInFlow = [&] {
+ auto* ancestor = renderer.parent();
+ while (ancestor && ((ancestor->isInline() && !ancestor->isReplaced()) || !ancestor->isRenderBlock()))
+ ancestor = ancestor->parent();
+ return downcast<RenderBlock>(ancestor);
+ };
+ return containingBlockForObjectInFlow();
+ }
+
+ if (positionType == PositionType::Absolute) {
+ auto containingBlockForAbsolutePosition = [&] {
+ if (is<RenderInline>(renderer) && renderer.style().position() == PositionType::Relative) {
+ // A relatively positioned RenderInline forwards its absolute positioned descendants to
+ // its nearest non-anonymous containing block (to avoid having positioned objects list in RenderInlines).
+ return nearestNonAnonymousContainingBlockIncludingSelf(renderer.parent());
+ }
+ auto* ancestor = renderer.parent();
+ while (ancestor && !ancestor->canContainAbsolutelyPositionedObjects())
+ ancestor = ancestor->parent();
+ // Make sure we only return non-anonymous RenderBlock as containing block.
+ return nearestNonAnonymousContainingBlockIncludingSelf(ancestor);
+ };
+ return containingBlockForAbsolutePosition();
+ }
+
+ if (positionType == PositionType::Fixed) {
+ auto containingBlockForFixedPosition = [&] {
+ auto* ancestor = renderer.parent();
+ while (ancestor && !ancestor->canContainFixedPositionObjects())
+ ancestor = ancestor->parent();
+ return nearestNonAnonymousContainingBlockIncludingSelf(ancestor);
+ };
+ return containingBlockForFixedPosition();
+ }
+
+ ASSERT_NOT_REACHED();
+ return nullptr;
+}
+
RenderBlock* RenderObject::containingBlock() const
{
+ if (is<RenderText>(*this))
+ return containingBlockForPositionType(PositionType::Static, *this);
+
auto containingBlockForRenderer = [](const auto& renderer) -> RenderBlock* {
if (isInTopLayerOrBackdrop(renderer.style(), renderer.element()))
return &renderer.view();
- if (renderer.isAbsolutelyPositioned())
- return renderer.containingBlockForAbsolutePosition();
- if (renderer.isFixedPositioned())
- return renderer.containingBlockForFixedPosition();
- return renderer.containingBlockForObjectInFlow();
+ return containingBlockForPositionType(renderer.style().position(), renderer);
};
- if (is<RenderText>(*this))
- return containingBlockForObjectInFlow();
-
if (!parent() && is<RenderScrollbarPart>(*this)) {
if (auto* scrollbarPart = downcast<RenderScrollbarPart>(*this).rendererOwningScrollbar())
return containingBlockForRenderer(*scrollbarPart);
@@ -679,14 +724,6 @@
return containingBlockForRenderer(downcast<RenderElement>(*this));
}
-RenderBlock* RenderObject::containingBlockForObjectInFlow() const
-{
- auto* renderer = parent();
- while (renderer && ((renderer->isInline() && !renderer->isReplaced()) || !renderer->isRenderBlock()))
- renderer = renderer->parent();
- return downcast<RenderBlock>(renderer);
-}
-
void RenderObject::addPDFURLRect(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
{
Vector<LayoutRect> focusRingRects;
Modified: trunk/Source/WebCore/rendering/RenderObject.h (287743 => 287744)
--- trunk/Source/WebCore/rendering/RenderObject.h 2022-01-07 13:02:32 UTC (rev 287743)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2022-01-07 15:08:00 UTC (rev 287744)
@@ -538,7 +538,7 @@
// Returns the containing block level element for this element.
WEBCORE_EXPORT RenderBlock* containingBlock() const;
- RenderBlock* containingBlockForObjectInFlow() const;
+ static RenderBlock* containingBlockForPositionType(PositionType, const RenderObject&);
// Convert the given local point to absolute coordinates. If OptionSet<MapCoordinatesMode> includes UseTransforms, take transforms into account.
WEBCORE_EXPORT FloatPoint localToAbsolute(const FloatPoint& localPoint = FloatPoint(), OptionSet<MapCoordinatesMode> = { }, bool* wasFixed = nullptr) const;
