Changeset 200953 in webkit

Timestamp:

May 16, 2016, 11:44:02 AM (9 years ago)

Author:

Alan Bujtas

Message:

containingBlockFor*Position functions should take the renderer instead of the parent.
​https://bugs.webkit.org/show_bug.cgi?id=157659

Reviewed by Simon Fraser.

containingBlockForFixedPosition, containingBlockForAbsolutePosition and containingBlockForObjectInFlow functions
expect the renderer's parent to be passed in (unless it is a RenderInline!). It is rather misleading and highly error-prone.
We should call them with the renderer itself instead.

• dom/Element.cpp:

(WebCore::layoutOverflowRectContainsAllDescendants): This expects ancestor containing block.

• rendering/LogicalSelectionOffsetCaches.h:

(WebCore::LogicalSelectionOffsetCaches::LogicalSelectionOffsetCaches):

• rendering/RenderElement.cpp:

(WebCore::containingBlockForFixedPosition):
(WebCore::containingBlockForAbsolutePosition):
(WebCore::containingBlockForObjectInFlow):

• rendering/RenderElement.h:
• rendering/RenderInline.cpp:

(WebCore::RenderInline::styleWillChange):

• rendering/RenderLineBreak.cpp:

(WebCore::RenderLineBreak::collectSelectionRects): Not a behaviour change.

• rendering/RenderObject.cpp:

(WebCore::RenderObject::containingBlock): RenderScrollbarPart renderer now returns
the containing block based on its owning renderer's style.

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) (1 diff)
• rendering/RenderObject.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-05-16  Zalan Bujtas  <zalan@apple.com>
+
+        containingBlockFor*Position functions should take the renderer instead of the parent.
+        https://bugs.webkit.org/show_bug.cgi?id=157659
+
+        Reviewed by Simon Fraser.
+
+        containingBlockForFixedPosition, containingBlockForAbsolutePosition and containingBlockForObjectInFlow functions
+        expect the renderer's parent to be passed in (unless it is a RenderInline!). It is rather misleading and highly error-prone.
+        We should call them with the renderer itself instead.
+
+        * dom/Element.cpp:
+        (WebCore::layoutOverflowRectContainsAllDescendants): This expects ancestor containing block.
+        * rendering/LogicalSelectionOffsetCaches.h:
+        (WebCore::LogicalSelectionOffsetCaches::LogicalSelectionOffsetCaches):
+        * rendering/RenderElement.cpp:
+        (WebCore::containingBlockForFixedPosition):
+        (WebCore::containingBlockForAbsolutePosition):
+        (WebCore::containingBlockForObjectInFlow):
+        * rendering/RenderElement.h:
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::styleWillChange):
+        * rendering/RenderLineBreak.cpp:
+        (WebCore::RenderLineBreak::collectSelectionRects): Not a behaviour change.
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::containingBlock): RenderScrollbarPart renderer now returns
+        the containing block based on its owning renderer's style.
+
 2016-05-16  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -977 +977 @@
 
     // This renderer may have positioned descendants whose containing block is some ancestor.
-    if (auto containingBlock = containingBlockForAbsolutePosition(&renderer)) {
+    if (auto containingBlock = renderer.containingBlockForAbsolutePosition()) {
         if (auto positionedObjects = containingBlock->positionedObjects()) {
             for (RenderBox* it : *positionedObjects) {

trunk/Source/WebCore/rendering/LogicalSelectionOffsetCaches.h

@@ -89 +89 @@
         ASSERT(rootBlock.isSelectionRoot());
 #endif
-        auto parent = rootBlock.parent();
-
         // LogicalSelectionOffsetCaches should not be used on an orphaned tree.
-        m_containingBlockForFixedPosition.setBlock(containingBlockForFixedPosition(parent), nullptr);
-        m_containingBlockForAbsolutePosition.setBlock(containingBlockForAbsolutePosition(parent), nullptr);
-        m_containingBlockForInflowPosition.setBlock(containingBlockForObjectInFlow(parent), nullptr);
+        m_containingBlockForFixedPosition.setBlock(rootBlock.containingBlockForFixedPosition(), nullptr);
+        m_containingBlockForAbsolutePosition.setBlock(rootBlock.containingBlockForAbsolutePosition(), nullptr);
+        m_containingBlockForInflowPosition.setBlock(rootBlock.containingBlockForObjectInFlow(), nullptr);
     }
 

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -2244 +2244 @@
 #endif // ENABLE(IOS_TEXT_AUTOSIZING)
 
-RenderBlock* containingBlockForFixedPosition(const RenderElement* element)
-{
-    const auto* object = element;
-    while (object && !object->canContainFixedPositionObjects())
-        object = object->parent();
-
-    ASSERT(!object || !object->isAnonymousBlock());
-    return const_cast<RenderBlock*>(downcast<RenderBlock>(object));
-}
-
-RenderBlock* containingBlockForAbsolutePosition(const RenderElement* element)
-{
-    const auto* object = element;
-    while (object && !object->canContainAbsolutelyPositionedObjects())
-        object = object->parent();
-
-    // For a relatively positioned inline, return its nearest non-anonymous containing block,
-    // not the inline itself, to avoid having a positioned objects list in all RenderInlines
-    // and use RenderBlock* as RenderElement::containingBlock's return type.
-    // Use RenderBlock::container() to obtain the inline.
-    if (object && !is<RenderBlock>(*object))
-        object = object->containingBlock();
-
-    while (object && object->isAnonymousBlock())
-        object = object->containingBlock();
-
-    return const_cast<RenderBlock*>(downcast<RenderBlock>(object));
-}
-
-RenderBlock* containingBlockForObjectInFlow(const RenderElement* element)
-{
-    const auto* object = element;
-    while (object && ((object->isInline() && !object->isReplaced()) || !object->isRenderBlock()))
-        object = object->parent();
-    return const_cast<RenderBlock*>(downcast<RenderBlock>(object));
-}
-
-}
+}

trunk/Source/WebCore/rendering/RenderElement.h

@@ -489 +489 @@
 }
 
-RenderBlock* containingBlockForFixedPosition(const RenderElement*);
-RenderBlock* containingBlockForAbsolutePosition(const RenderElement*);
-RenderBlock* containingBlockForObjectInFlow(const RenderElement*);
 } // namespace WebCore
 

trunk/Source/WebCore/rendering/RenderInline.cpp

@@ -172 +172 @@
     if (canContainAbsolutelyPositionedObjects() && newStyle.position() == StaticPosition) {
         // RenderInlines forward their absolute positioned descendants to their (non-anonymous) containing block.
-        auto* container = containingBlockForAbsolutePosition(this);
+        auto* container = containingBlockForAbsolutePosition();
         if (container && !container->canContainAbsolutelyPositionedObjects())
             container->removePositionedObjects(nullptr, NewContainingBlock);

trunk/Source/WebCore/rendering/RenderLineBreak.cpp

@@ -242 +242 @@
     }
 
-    auto* containingBlock = containingBlockForObjectInFlow(this);
+    auto* containingBlock = containingBlockForObjectInFlow();
     // 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

@@ -623 +623 @@
 RenderBlock* RenderObject::containingBlock() const
 {
-    auto* parent = this->parent();
+    auto containingBlockForRenderer = [](const RenderObject& renderer)
+    {
+        auto& style = renderer.style();
+        if (style.position() == AbsolutePosition)
+            return renderer.containingBlockForAbsolutePosition();
+        if (style.position() == FixedPosition)
+            return renderer.containingBlockForFixedPosition();
+        return renderer.containingBlockForObjectInFlow();
+    };
+
     if (is<RenderText>(*this))
-        return containingBlockForObjectInFlow(parent);
-
-    if (!parent && is<RenderScrollbarPart>(*this))
-        parent = downcast<RenderScrollbarPart>(*this).rendererOwningScrollbar();
-
-    auto& style = this->style();
-    if (style.position() == AbsolutePosition)
-        return containingBlockForAbsolutePosition(parent);
-    if (style.position() == FixedPosition)
-        return containingBlockForFixedPosition(parent);
-    return containingBlockForObjectInFlow(parent);
+        return containingBlockForObjectInFlow();
+
+    if (!parent() && is<RenderScrollbarPart>(*this)) {
+        if (auto* renderer = downcast<RenderScrollbarPart>(*this).rendererOwningScrollbar())
+            return containingBlockForRenderer(*renderer);
+        return nullptr;
+    }
+    return containingBlockForRenderer(*this);
+}
+
+RenderBlock* RenderObject::containingBlockForFixedPosition() const
+{
+    auto* renderer = parent();
+    while (renderer && !renderer->canContainFixedPositionObjects())
+        renderer = renderer->parent();
+
+    ASSERT(!renderer || !renderer->isAnonymousBlock());
+    return downcast<RenderBlock>(renderer);
+}
+
+RenderBlock* RenderObject::containingBlockForAbsolutePosition() const
+{
+    // RenderInlines forward their absolute positioned descendants to the containing block, so
+    // we need to start searching from 'this' and not from 'parent()'.
+    auto* renderer = isRenderInline() ? const_cast<RenderElement*>(downcast<RenderElement>(this)) : parent();
+    while (renderer && !renderer->canContainAbsolutelyPositionedObjects())
+        renderer = renderer->parent();
+
+    // For a relatively positioned inline, return its nearest non-anonymous containing block,
+    // not the inline itself, to avoid having a positioned objects list in all RenderInlines
+    // and use RenderBlock* as RenderElement::containingBlock's return type.
+    // Use RenderBlock::container() to obtain the inline.
+    if (renderer && !is<RenderBlock>(*renderer))
+        renderer = renderer->containingBlock();
+
+    while (renderer && renderer->isAnonymousBlock())
+        renderer = renderer->containingBlock();
+
+    return downcast<RenderBlock>(renderer);
+}
+
+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

@@ -648 +648 @@
     // returns the containing block level element for this element.
     RenderBlock* containingBlock() const;
+    RenderBlock* containingBlockForFixedPosition() const;
+    RenderBlock* containingBlockForAbsolutePosition() const;
+    RenderBlock* containingBlockForObjectInFlow() const;
 
     // Convert the given local point to absolute coordinates. If MapCoordinatesFlags includes UseTransforms, take transforms into account.