Changeset 288280 in webkit

Timestamp:

Jan 20, 2022 5:35:50 AM (2 years ago)

Author:

Adrian Perez de Castro

Message:

Merge r286866 - nullptr deref in ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded
​https://bugs.webkit.org/show_bug.cgi?id=234018

Patch by Gabriel Nava Marino <​gnavamarino@apple.com> on 2021-12-10
Reviewed by Alan Bujtas.

Source/WebCore:

Test: fast/rendering/floating-object-renderer-crash.html

When destroying a given renderer, we first remove floats and out-of-flow positioned objects
from their containing block before detaching the renderer from the tree. We do this by obtaining
the renderer’s outermost block containing a floating object and recursively marking all siblings
and descendants for layout.

The criteria for continuing down the list of children require the current block to contain floats
or be able to shrink to avoid floats. However, we can have a scenario where the current child block
doesn’t have a float, but one of its descendants does. In this case, although we should continue to
that descendant and remove the float, we do not.

The proposal in this patch will instead check whether the child block contains a float, or any of its descendants do.
If so we should continue traversing towards that descendant.

• rendering/RenderBlockFlow.cpp:

(WebCore::RenderBlockFlow::subtreeContainsFloat const):
(WebCore::RenderBlockFlow::subtreeContainsFloats const):
(WebCore::RenderBlockFlow::markAllDescendantsWithFloatsForLayout):

• rendering/RenderBlockFlow.h:

LayoutTests:

• fast/rendering/floating-object-renderer-crash-expected.txt: Added.
• fast/rendering/floating-object-renderer-crash.html: Added.

Location:

releases/WebKitGTK/webkit-2.34

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/rendering/floating-object-renderer-crash-expected.txt (added)
• LayoutTests/fast/rendering/floating-object-renderer-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockFlow.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderBlockFlow.h (modified) (1 diff)

releases/WebKitGTK/webkit-2.34/LayoutTests/ChangeLog

@@ +1 @@
+2021-12-10  Gabriel Nava Marino  <gnavamarino@apple.com>
+
+        nullptr deref in ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=234018
+
+        Reviewed by Alan Bujtas.
+
+        * fast/rendering/floating-object-renderer-crash-expected.txt: Added.
+        * fast/rendering/floating-object-renderer-crash.html: Added.
+
 2021-12-09  Cathie Chen  <cathiechen@igalia.com>
 

releases/WebKitGTK/webkit-2.34/Source/WebCore/ChangeLog

@@ +1 @@
+2021-12-10  Gabriel Nava Marino  <gnavamarino@apple.com>
+
+        nullptr deref in ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded
+        https://bugs.webkit.org/show_bug.cgi?id=234018
+
+        Reviewed by Alan Bujtas.
+
+        Test: fast/rendering/floating-object-renderer-crash.html
+
+        When destroying a given renderer, we first remove floats and out-of-flow positioned objects
+        from their containing block before detaching the renderer from the tree. We do this by obtaining
+        the renderer’s outermost block containing a floating object and recursively marking all siblings
+        and descendants for layout.
+
+        The criteria for continuing down the list of children require the current block to contain floats
+        or be able to shrink to avoid floats. However, we can have a scenario where the current child block
+        doesn’t have a float, but one of its descendants does. In this case, although we should continue to
+        that descendant and remove the float, we do not.
+
+        The proposal in this patch will instead check whether the child block contains a float, or any of its descendants do.
+        If so we should continue traversing towards that descendant.
+
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::subtreeContainsFloat const):
+        (WebCore::RenderBlockFlow::subtreeContainsFloats const):
+        (WebCore::RenderBlockFlow::markAllDescendantsWithFloatsForLayout):
+        * rendering/RenderBlockFlow.h:
+
 2021-12-09  Cathie Chen  <cathiechen@igalia.com>
 

releases/WebKitGTK/webkit-2.34/Source/WebCore/rendering/RenderBlockFlow.cpp

@@ -2100 +2100 @@
 }
 
+bool RenderBlockFlow::subtreeContainsFloat(RenderBox& renderer) const
+{
+    bool contains = m_floatingObjects && m_floatingObjects->set().contains<FloatingObjectHashTranslator>(renderer);
+    for (auto& block : childrenOfType<RenderBlock>(*this)) {
+        if (!is<RenderBlockFlow>(block))
+            continue;
+        auto& blockFlow = downcast<RenderBlockFlow>(block);
+        contains |= blockFlow.subtreeContainsFloat(renderer);
+    }
+    return contains;
+}
+
+bool RenderBlockFlow::subtreeContainsFloats() const
+{
+    bool contains = m_floatingObjects && !m_floatingObjects->set().isEmpty();
+    for (auto& block : childrenOfType<RenderBlock>(*this)) {
+        if (!is<RenderBlockFlow>(block))
+            continue;
+        auto& blockFlow = downcast<RenderBlockFlow>(block);
+        contains |= blockFlow.subtreeContainsFloats();
+    }
+    return contains;
+}
+
 void RenderBlockFlow::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
@@ -2869 +2893 @@
         }
         auto& blockFlow = downcast<RenderBlockFlow>(block);
-        if ((floatToRemove ? blockFlow.containsFloat(*floatToRemove) : blockFlow.containsFloats()) || blockFlow.shrinkToAvoidFloats())
+        if ((floatToRemove ? blockFlow.subtreeContainsFloat(*floatToRemove) : blockFlow.subtreeContainsFloats()) || blockFlow.shrinkToAvoidFloats())
             blockFlow.markAllDescendantsWithFloatsForLayout(floatToRemove, inLayout);
     }

releases/WebKitGTK/webkit-2.34/Source/WebCore/rendering/RenderBlockFlow.h

@@ -279 +279 @@
     bool containsFloats() const override { return m_floatingObjects && !m_floatingObjects->set().isEmpty(); }
     bool containsFloat(RenderBox&) const;
+    bool subtreeContainsFloats() const;
+    bool subtreeContainsFloat(RenderBox&) const;
 
     void deleteLines() override;