Title: [286866] trunk
- Revision
- 286866
- Author
- [email protected]
- Date
- 2021-12-10 11:37:38 -0800 (Fri, 10 Dec 2021)
Log Message
nullptr deref in ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded
https://bugs.webkit.org/show_bug.cgi?id=234018
Patch by Gabriel Nava Marino <[email protected]> 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (286865 => 286866)
--- trunk/LayoutTests/ChangeLog 2021-12-10 19:34:01 UTC (rev 286865)
+++ trunk/LayoutTests/ChangeLog 2021-12-10 19:37:38 UTC (rev 286866)
@@ -1,3 +1,13 @@
+2021-12-10 Gabriel Nava Marino <[email protected]>
+
+ 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-10 Commit Queue <[email protected]>
Unreviewed, reverting r286836.
Added: trunk/LayoutTests/fast/rendering/floating-object-renderer-crash-expected.txt (0 => 286866)
--- trunk/LayoutTests/fast/rendering/floating-object-renderer-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/rendering/floating-object-renderer-crash-expected.txt 2021-12-10 19:37:38 UTC (rev 286866)
@@ -0,0 +1,3 @@
+PASS if this doesn't crash
+
+
Added: trunk/LayoutTests/fast/rendering/floating-object-renderer-crash.html (0 => 286866)
--- trunk/LayoutTests/fast/rendering/floating-object-renderer-crash.html (rev 0)
+++ trunk/LayoutTests/fast/rendering/floating-object-renderer-crash.html 2021-12-10 19:37:38 UTC (rev 286866)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<style>
+ video {
+ block-size: 400px;
+ }
+ input {
+ float: left;
+ content: url();
+ }
+</style>
+<script>
+ _onload_ = () => {
+ document.documentElement.prepend(document.createElement('span'));
+ document.body.append(document.createElement('div'));
+ document.body.append(document.createElement('span'));
+ document.documentElement.append(document.createElement('div'));
+ let html2 = document.createElement('html');
+ html2.style.blockSize = '0';
+ document.body.appendChild(html2);
+ html2.appendChild(document.createElement('q'));
+ html2.appendChild(document.createElement('input'));
+ html2.appendChild(document.createElement('video'));
+ document.body.offsetTop;
+ document.styleSheets[0].insertRule(`html, body, video { float: left; }`);
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ };
+</script>
+PASS if this doesn't crash
Modified: trunk/Source/WebCore/ChangeLog (286865 => 286866)
--- trunk/Source/WebCore/ChangeLog 2021-12-10 19:34:01 UTC (rev 286865)
+++ trunk/Source/WebCore/ChangeLog 2021-12-10 19:37:38 UTC (rev 286866)
@@ -1,3 +1,31 @@
+2021-12-10 Gabriel Nava Marino <[email protected]>
+
+ 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-10 Said Abou-Hallawa <[email protected]>
[GPU Process] [Filters] Make FilterEffectVector a Vector of Ref<FilterEffect>
Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.cpp (286865 => 286866)
--- trunk/Source/WebCore/rendering/RenderBlockFlow.cpp 2021-12-10 19:34:01 UTC (rev 286865)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.cpp 2021-12-10 19:37:38 UTC (rev 286866)
@@ -2109,6 +2109,30 @@
return m_floatingObjects && m_floatingObjects->set().contains<FloatingObjectHashTranslator>(renderer);
}
+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)
{
RenderBlock::styleDidChange(diff, oldStyle);
@@ -2878,7 +2902,7 @@
continue;
}
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);
}
}
Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.h (286865 => 286866)
--- trunk/Source/WebCore/rendering/RenderBlockFlow.h 2021-12-10 19:34:01 UTC (rev 286865)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.h 2021-12-10 19:37:38 UTC (rev 286866)
@@ -278,6 +278,8 @@
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;
void computeOverflow(LayoutUnit oldClientAfterEdge, bool recomputeFloats = false) override;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
