[webkit-changes] [185572] trunk

Mon, 15 Jun 2015 16:57:07 -0700

Title: [185572] trunk
Revision
185572
Author
[email protected]
Date
2015-06-15 16:56:48 -0700 (Mon, 15 Jun 2015)

Log Message

RootInlineBox::m_lineBreakObj becomes invalid when a child renderer is removed and the line does not get marked dirty.
https://bugs.webkit.org/show_bug.cgi?id=145988
rdar://problem/20959137

Reviewed by David Hyatt.

This patch ensures that we find the right first inline box so that we can dirty the
the appropriate line boxes.
With marking the right line boxes dirty, now we can update RootInlineBox::m_lineBreakObj at the next layout.

Source/WebCore:

Test: fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html

* rendering/RenderInline.cpp:
(WebCore::RenderInline::culledInlineFirstLineBox):
(WebCore::RenderInline::culledInlineLastLineBox):
* rendering/RootInlineBox.cpp:
(WebCore::RootInlineBox::setLineBreakInfo): Deleted. Remove misleading assert and comment.

LayoutTests:

* fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt: Added.
* fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (185571 => 185572)


--- trunk/LayoutTests/ChangeLog	2015-06-15 23:32:45 UTC (rev 185571)
+++ trunk/LayoutTests/ChangeLog	2015-06-15 23:56:48 UTC (rev 185572)
@@ -1,3 +1,18 @@
+2015-06-15  Zalan Bujtas  <[email protected]>
+
+        RootInlineBox::m_lineBreakObj becomes invalid when a child renderer is removed and the line does not get marked dirty.
+        https://bugs.webkit.org/show_bug.cgi?id=145988
+        rdar://problem/20959137
+
+        Reviewed by David Hyatt.
+
+        This patch ensures that we find the right first inline box so that we can dirty the
+        the appropriate line boxes.
+        With marking the right line boxes dirty, now we can update RootInlineBox::m_lineBreakObj at the next layout.
+
+        * fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt: Added.
+        * fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html: Added.
+
 2015-06-15  Darin Adler  <[email protected]>
 
         REGRESSION (r182215): Reproducible crash at drawsvg.org due to reentrant layout

Added: trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt (0 => 185572)


--- trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt	2015-06-15 23:56:48 UTC (rev 185572)
@@ -0,0 +1,4 @@
+Pass if no crash or assert in Debug.   bar
+
+
+

Added: trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html (0 => 185572)


--- trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html	2015-06-15 23:56:48 UTC (rev 185572)
@@ -0,0 +1,22 @@
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+window._onload_ = function()
+{
+    document.body.offsetTop;
+    b.lastChild.parentNode.removeChild(b.lastChild);
+    document.body.offsetTop;
+    a.firstChild.parentNode.removeChild(a.firstChild);
+}
+</script>
+<body>
+<div id="a">foo</div><div></div>
+<div>Pass if no crash or assert in Debug.    
+<output>
+<o&#x0195;tput>bar</output>
+<span id="b">
+<span>
+<div style="display:inline-block"></div>
+<br><br><br>
+</span>
+</body>

Modified: trunk/Source/WebCore/ChangeLog (185571 => 185572)


--- trunk/Source/WebCore/ChangeLog	2015-06-15 23:32:45 UTC (rev 185571)
+++ trunk/Source/WebCore/ChangeLog	2015-06-15 23:56:48 UTC (rev 185572)
@@ -1,3 +1,23 @@
+2015-06-15  Zalan Bujtas  <[email protected]>
+
+        RootInlineBox::m_lineBreakObj becomes invalid when a child renderer is removed and the line does not get marked dirty.
+        https://bugs.webkit.org/show_bug.cgi?id=145988
+        rdar://problem/20959137
+
+        Reviewed by David Hyatt.
+
+        This patch ensures that we find the right first inline box so that we can dirty the
+        the appropriate line boxes.
+        With marking the right line boxes dirty, now we can update RootInlineBox::m_lineBreakObj at the next layout.
+
+        Test: fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html
+
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::culledInlineFirstLineBox):
+        (WebCore::RenderInline::culledInlineLastLineBox):
+        * rendering/RootInlineBox.cpp:
+        (WebCore::RootInlineBox::setLineBreakInfo): Deleted. Remove misleading assert and comment.
+
 2015-06-15  Matt Rajca  <[email protected]>
 
         Media Session: Improve the safety of playback toggling

Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (185571 => 185572)


--- trunk/Source/WebCore/rendering/RenderInline.cpp	2015-06-15 23:32:45 UTC (rev 185571)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp	2015-06-15 23:56:48 UTC (rev 185572)
@@ -990,9 +990,11 @@
             
         // We want to get the margin box in the inline direction, and then use our font ascent/descent in the block
         // direction (aligned to the root box's baseline).
-        if (is<RenderBox>(*current))
-            return downcast<RenderBox>(*current).inlineBoxWrapper();
-        if (is<RenderLineBreak>(*current)) {
+        if (is<RenderBox>(*current)) {
+            const auto& renderBox = downcast<RenderBox>(*current);
+            if (renderBox.inlineBoxWrapper())
+                return renderBox.inlineBoxWrapper();
+        } else if (is<RenderLineBreak>(*current)) {
             RenderLineBreak& renderBR = downcast<RenderLineBreak>(*current);
             if (renderBR.inlineBoxWrapper())
                 return renderBR.inlineBoxWrapper();
@@ -1017,9 +1019,11 @@
             
         // We want to get the margin box in the inline direction, and then use our font ascent/descent in the block
         // direction (aligned to the root box's baseline).
-        if (is<RenderBox>(*current))
-            return downcast<RenderBox>(*current).inlineBoxWrapper();
-        if (is<RenderLineBreak>(*current)) {
+        if (is<RenderBox>(*current)) {
+            const auto& renderBox = downcast<RenderBox>(*current);
+            if (renderBox.inlineBoxWrapper())
+                return renderBox.inlineBoxWrapper();
+        } else if (is<RenderLineBreak>(*current)) {
             RenderLineBreak& renderBR = downcast<RenderLineBreak>(*current);
             if (renderBR.inlineBoxWrapper())
                 return renderBR.inlineBoxWrapper();

Modified: trunk/Source/WebCore/rendering/RootInlineBox.cpp (185571 => 185572)


--- trunk/Source/WebCore/rendering/RootInlineBox.cpp	2015-06-15 23:32:45 UTC (rev 185571)
+++ trunk/Source/WebCore/rendering/RootInlineBox.cpp	2015-06-15 23:56:48 UTC (rev 185572)
@@ -794,13 +794,6 @@
 
 void RootInlineBox::setLineBreakInfo(RenderObject* obj, unsigned breakPos, const BidiStatus& status)
 {
-    // When setting lineBreakObj, the RenderObject must not be a RenderInline
-    // with no line boxes, otherwise all sorts of invariants are broken later.
-    // This has security implications because if the RenderObject does not
-    // point to at least one line box, then that RenderInline can be deleted
-    // later without resetting the lineBreakObj, leading to use-after-free.
-    ASSERT_WITH_SECURITY_IMPLICATION(!obj || is<RenderText>(*obj) || !(is<RenderInline>(*obj) && is<RenderBox>(*obj) && !downcast<RenderBox>(*obj).inlineBoxWrapper()));
-
     m_lineBreakObj = obj;
     m_lineBreakPos = breakPos;
     m_lineBreakBidiStatusEor = status.eor;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to