Changeset 287744 in webkit

Timestamp:

Jan 7, 2022, 7:08:00 AM (5 years ago)

Author:

Alan Bujtas

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:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Element.cpp (modified) (1 diff)
• rendering/LogicalSelectionOffsetCaches.h (modified) (1 diff)
• rendering/RenderElement.cpp (modified) (1 diff)
• rendering/RenderElement.h (modified) (1 diff)
• rendering/RenderInline.cpp (modified) (1 diff)
• rendering/RenderLineBreak.cpp (modified) (1 diff)
• rendering/RenderObject.cpp (modified) (2 diffs)
• rendering/RenderObject.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-01-07  Alan Bujtas  <zalan@apple.com>
+
+        [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  <nzimmermann@igalia.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -1586 +1586 @@
 
     // 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) {

trunk/Source/WebCore/rendering/LogicalSelectionOffsetCaches.h

@@ -90 +90 @@
 #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);
     }
 

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -635 +635 @@
 }
 
-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)
 {

trunk/Source/WebCore/rendering/RenderElement.h

@@ -235 +235 @@
     WEBCORE_EXPORT void resetTextAutosizing();
 #endif
-    RenderBlock* containingBlockForFixedPosition() const;
-    RenderBlock* containingBlockForAbsolutePosition() const;
 
     WEBCORE_EXPORT ImageOrientation imageOrientation() const;

trunk/Source/WebCore/rendering/RenderInline.cpp

@@ -160 +160 @@
     // 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);

trunk/Source/WebCore/rendering/RenderLineBreak.cpp

@@ -200 +200 @@
     }
 
-    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);

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -657 +657 @@
 }
 
+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)) {
@@ -678 +723 @@
     }
     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);
 }
 

trunk/Source/WebCore/rendering/RenderObject.h

@@ -539 +539 @@
     // 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.