InlineBox should not hold a reference to RenderObject

https://bugs.webkit.org/show_bug.cgi?id=224506

Reviewed by Antti Koivisto.

The InlineBox/RenderObject relationship is somewhat error-prone (see RenderBox::setInlineBoxWrapper vs. InlineElementBox::extractLine) and it's rather easy to delete one without letting the other know.

* rendering/InlineBox.cpp:
(WebCore::InlineBox::showNodeTreeForThis const):
(WebCore::InlineBox::showLineTreeForThis const):
(WebCore::InlineBox::caretMinOffset const):
(WebCore::InlineBox::caretMaxOffset const):
(WebCore::InlineBox::adjustPosition):
(WebCore::InlineBox::selectionState):
(WebCore::InlineBox::canAccommodateEllipsis const):
(WebCore::InlineBox::locationIncludingFlipping const):
(WebCore::InlineBox::flipForWritingMode const):
* rendering/InlineBox.h:
(WebCore::InlineBox::renderer const):
(WebCore::InlineBox::boxModelObject const):
(WebCore::InlineBox::InlineBox):


Canonical link: https://commits.webkit.org/236505@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275943 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,27 @@
+2021-04-14  Zalan Bujtas  <zalan@apple.com>
+
+        InlineBox should not hold a reference to RenderObject
+        https://bugs.webkit.org/show_bug.cgi?id=224506
+
+        Reviewed by Antti Koivisto.
+
+        The InlineBox/RenderObject relationship is somewhat error-prone (see RenderBox::setInlineBoxWrapper vs. InlineElementBox::extractLine) and it's rather easy to delete one without letting the other know.
+
+        * rendering/InlineBox.cpp:
+        (WebCore::InlineBox::showNodeTreeForThis const):
+        (WebCore::InlineBox::showLineTreeForThis const):
+        (WebCore::InlineBox::caretMinOffset const):
+        (WebCore::InlineBox::caretMaxOffset const):
+        (WebCore::InlineBox::adjustPosition):
+        (WebCore::InlineBox::selectionState):
+        (WebCore::InlineBox::canAccommodateEllipsis const):
+        (WebCore::InlineBox::locationIncludingFlipping const):
+        (WebCore::InlineBox::flipForWritingMode const):
+        * rendering/InlineBox.h:
+        (WebCore::InlineBox::renderer const):
+        (WebCore::InlineBox::boxModelObject const):
+        (WebCore::InlineBox::InlineBox):
+
 2021-04-14  Aditya Keerthi  <akeerthi@apple.com>
 
         [iOS][FCR] Indeterminate progress bars should update at 60fps

Source/WebCore/rendering/InlineBox.cpp

@@ -40,7 +40,8 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(InlineBox);
 
 struct SameSizeAsInlineBox {
     virtual ~SameSizeAsInlineBox() = default;
-    void* a[4];
+    void* a[3];
+    WeakPtr<RenderObject> r;
     FloatPoint b;
     float c[2];
     unsigned d : 23;
@@ -96,12 +97,12 @@ const char* InlineBox::boxName() const
 
 void InlineBox::showNodeTreeForThis() const
 {
-    m_renderer.showNodeTreeForThis();
+    renderer().showNodeTreeForThis();
 }
 
 void InlineBox::showLineTreeForThis() const
 {
-    m_renderer.containingBlock()->showLineTreeForThis();
+    renderer().containingBlock()->showLineTreeForThis();
 }
 
 void InlineBox::outputLineTreeAndMark(TextStream& stream, const InlineBox* markedBox, int depth) const
@@ -157,12 +158,12 @@ LayoutUnit InlineBox::lineHeight() const
 
 int InlineBox::caretMinOffset() const 
 { 
-    return m_renderer.caretMinOffset();
+    return renderer().caretMinOffset();
 }
 
 int InlineBox::caretMaxOffset() const 
 { 
-    return m_renderer.caretMaxOffset();
+    return renderer().caretMaxOffset();
 }
 
 void InlineBox::dirtyLineBoxes()
@@ -176,10 +177,10 @@ void InlineBox::adjustPosition(float dx, float dy)
 {
     m_topLeft.move(dx, dy);
 
-    if (m_renderer.isOutOfFlowPositioned())
+    if (renderer().isOutOfFlowPositioned())
         return;
 
-    if (m_renderer.isReplaced())
+    if (renderer().isReplaced())
         downcast<RenderBox>(renderer()).move(LayoutUnit(dx), LayoutUnit(dy));
 }
 
@@ -259,13 +260,13 @@ InlineBox* InlineBox::previousLeafOnLineIgnoringLineBreak() const
 
 RenderObject::HighlightState InlineBox::selectionState()
 {
-    return m_renderer.selectionState();
+    return renderer().selectionState();
 }
 
 bool InlineBox::canAccommodateEllipsis(bool ltr, int blockEdge, int ellipsisWidth) const
 {
     // Non-replaced elements can always accommodate an ellipsis.
-    if (!m_renderer.isReplaced())
+    if (!renderer().isReplaced())
         return true;
 
     IntRect boxRect(left(), 0, m_logicalWidth, 10);
@@ -289,7 +290,7 @@ void InlineBox::clearKnownToHaveNoOverflow()
 
 FloatPoint InlineBox::locationIncludingFlipping() const
 {
-    if (!m_renderer.style().isFlippedBlocksWritingMode())
+    if (!renderer().style().isFlippedBlocksWritingMode())
         return topLeft();
     RenderBlockFlow& block = root().blockFlow();
     if (block.style().isHorizontalWritingMode())
@@ -299,28 +300,28 @@ FloatPoint InlineBox::locationIncludingFlipping() const
 
 void InlineBox::flipForWritingMode(FloatRect& rect) const
 {
-    if (!m_renderer.style().isFlippedBlocksWritingMode())
+    if (!renderer().style().isFlippedBlocksWritingMode())
         return;
     root().blockFlow().flipForWritingMode(rect);
 }
 
 FloatPoint InlineBox::flipForWritingMode(const FloatPoint& point) const
 {
-    if (!m_renderer.style().isFlippedBlocksWritingMode())
+    if (!renderer().style().isFlippedBlocksWritingMode())
         return point;
     return root().blockFlow().flipForWritingMode(point);
 }
 
 void InlineBox::flipForWritingMode(LayoutRect& rect) const
 {
-    if (!m_renderer.style().isFlippedBlocksWritingMode())
+    if (!renderer().style().isFlippedBlocksWritingMode())
         return;
     root().blockFlow().flipForWritingMode(rect);
 }
 
 LayoutPoint InlineBox::flipForWritingMode(const LayoutPoint& point) const
 {
-    if (!m_renderer.style().isFlippedBlocksWritingMode())
+    if (!renderer().style().isFlippedBlocksWritingMode())
         return point;
     return root().blockFlow().flipForWritingMode(point);
 }

Source/WebCore/rendering/InlineBox.h

@@ -25,6 +25,7 @@
 #include "TextFlags.h"
 #include <wtf/IsoMalloc.h>
 #include <wtf/TypeCasts.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -141,7 +142,7 @@ class InlineBox {
     InlineBox* previousLeafOnLineIgnoringLineBreak() const;
 
     // FIXME: Hide this once all callers are using tighter types.
-    RenderObject& renderer() const { return m_renderer; }
+    RenderObject& renderer() const { return *m_renderer; }
 
     InlineFlowBox* parent() const
     {
@@ -244,8 +245,8 @@ class InlineBox {
     // Use with caution! The type is not checked!
     RenderBoxModelObject* boxModelObject() const
     { 
-        if (!is<RenderText>(m_renderer))
-            return &downcast<RenderBoxModelObject>(m_renderer);
+        if (!is<RenderText>(renderer()))
+            return &downcast<RenderBoxModelObject>(renderer());
         return nullptr;
     }
 
@@ -286,7 +287,7 @@ class InlineBox {
 
     InlineFlowBox* m_parent { nullptr }; // The box that contains us.
 
-    RenderObject& m_renderer;
+    WeakPtr<RenderObject> m_renderer;
 
 private:
     float m_logicalWidth { 0 };
@@ -373,15 +374,15 @@ class InlineBox {
 
 protected:
     explicit InlineBox(RenderObject& renderer)
-        : m_renderer(renderer)
+        : m_renderer(makeWeakPtr(renderer))
     {
     }
 
     InlineBox(RenderObject& renderer, FloatPoint topLeft, float logicalWidth, bool firstLine, bool constructed, bool dirty, bool extracted, bool isHorizontal, InlineBox* next, InlineBox* previous, InlineFlowBox* parent)
         : m_nextOnLine(next)
         , m_previousOnLine(previous)
         , m_parent(parent)
-        , m_renderer(renderer)
+        , m_renderer(makeWeakPtr(renderer))
         , m_logicalWidth(logicalWidth)
         , m_topLeft(topLeft)
         , m_bitfields(firstLine, constructed, dirty, extracted, isHorizontal)