RootInlineBox::m_lineBreakObj becomes invalid when a child renderer is removed and...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Jun 2015 23:56:48 +0000 (23:56 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Jun 2015 23:56:48 +0000 (23:56 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185572 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt [new file with mode: 0644]
LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderInline.cpp
Source/WebCore/rendering/RootInlineBox.cpp

index ae3a590..261b033 100644 (file)
@@ -1,3 +1,18 @@
+2015-06-15  Zalan Bujtas  <zalan@apple.com>
+
+        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  <darin@apple.com>
 
         REGRESSION (r182215): Reproducible crash at drawsvg.org due to reentrant layout
diff --git a/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt b/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean-expected.txt
new file mode 100644 (file)
index 0000000..d692f96
--- /dev/null
@@ -0,0 +1,4 @@
+Pass if no crash or assert in Debug.   bar
+
+
+
diff --git a/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html b/LayoutTests/fast/inline/crash-when-child-renderer-is-removed-and-line-stays-clean.html
new file mode 100644 (file)
index 0000000..7f9c16a
--- /dev/null
@@ -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>
index 99011f3..1b68a75 100644 (file)
@@ -1,3 +1,23 @@
+2015-06-15  Zalan Bujtas  <zalan@apple.com>
+
+        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  <mrajca@apple.com>
 
         Media Session: Improve the safety of playback toggling
index 66f2dc6..14833f1 100644 (file)
@@ -990,9 +990,11 @@ InlineBox* RenderInline::culledInlineFirstLineBox() const
             
         // 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 @@ InlineBox* RenderInline::culledInlineLastLineBox() const
             
         // 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();
index 19ceec2..39f979d 100644 (file)
@@ -794,13 +794,6 @@ BidiStatus RootInlineBox::lineBreakBidiStatus() const
 
 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;