Crash from positioned generated content under run-in

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

Patch by Ken Buchanan <kenrb@chromium.org> on 2011-11-17
Reviewed by David Hyatt.

Source/WebCore:

Modified handling of run-in children to clear generated children
before removing the parent from the render tree. This caused problems
with absolute positioned children being not properly removed from the
positioned object list of the RenderView.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::handleRunInChild):

LayoutTests:

Layout test for crash condition.

* fast/css-generated-content/positioned-generated-content-under-run-in-crash-expected.html: Added
* fast/css-generated-content/positioned-generated-content-under-run-in-crash.html: Added

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index ba61fe4c6a0315fe453d0d8fa59b891b62874464..fb41e748231a981bce541a2bd35200695e51cbcd 100755 (executable)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2011-11-17  Ken Buchanan <kenrb@chromium.org>
+
+        Crash from positioned generated content under run-in
+        https://bugs.webkit.org/show_bug.cgi?id=70456
+
+        Reviewed by David Hyatt.
+
+        Layout test for crash condition.
+
+        * fast/css-generated-content/positioned-generated-content-under-run-in-crash-expected.html: Added
+        * fast/css-generated-content/positioned-generated-content-under-run-in-crash.html: Added
+
 2011-11-17  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r100652.

diff --git a/LayoutTests/fast/css-generated-content/positioned-generated-content-under-run-in-crash-expected.txt b/LayoutTests/fast/css-generated-content/positioned-generated-content-under-run-in-crash-expected.txt
new file mode 100755 (executable)
index 0000000..a1846ea
--- /dev/null
+++ b/LayoutTests/fast/css-generated-content/positioned-generated-content-under-run-in-crash-expected.txt
@@ -0,0 +1,2 @@
+PASS, if no exceptions or crash observed
+

diff --git a/LayoutTests/fast/css-generated-content/positioned-generated-content-under-run-in-crash.html b/LayoutTests/fast/css-generated-content/positioned-generated-content-under-run-in-crash.html
new file mode 100755 (executable)
index 0000000..60d02a4
--- /dev/null
+++ b/LayoutTests/fast/css-generated-content/positioned-generated-content-under-run-in-crash.html
@@ -0,0 +1,22 @@
+<style>
+.testclass::before { position: absolute; content: ""; }
+.testclass { display: run-in; }
+</style>
+PASS, if no exceptions or crash observed
+<script>
+function runTest() 
+{
+    test1 = document.createElement('div');
+    test1.setAttribute('class', 'testclass');
+    document.documentElement.appendChild(test1);
+    test2 = document.createElement('b');
+    test2.setAttribute('class', 'testclass');
+    document.documentElement.appendChild(test2);
+    test3 = document.createElement('div');
+    document.documentElement.appendChild(test3);
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+}
+window.onload = runTest;
+</script>
+

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
old mode 100644 (file)
new mode 100755 (executable)
index 7dd593b..59432dc
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2011-11-17  Ken Buchanan <kenrb@chromium.org>
+
+        Crash from positioned generated content under run-in
+        https://bugs.webkit.org/show_bug.cgi?id=70456
+
+        Reviewed by David Hyatt.
+
+        Modified handling of run-in children to clear generated children
+        before removing the parent from the render tree. This caused problems
+        with absolute positioned children being not properly removed from the
+        positioned object list of the RenderView.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::handleRunInChild):
+
 2011-11-17  Peter Kasting  <pkasting@google.com>
 
         Unreviewed, rolling out r100572.

diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp
index 46ffbc5f4a5cb2d0e6145f759906274ca7d15239..bae6175bce8b671e51840d875e04a0c669b5dc3d 100755 (executable)
--- a/Source/WebCore/rendering/RenderBlock.cpp
+++ b/Source/WebCore/rendering/RenderBlock.cpp
@@ -1582,6 +1582,16 @@ bool RenderBlock::handleRunInChild(RenderBox* child)
 
     RenderBlock* currBlock = toRenderBlock(curr);
 
+    // First we destroy any :before/:after content. It will be regenerated by the new inline.
+    // Exception is if the run-in itself is generated.
+    if (child->style()->styleType() != BEFORE && child->style()->styleType() != AFTER) {
+        RenderObject* generatedContent;
+        if (child->getCachedPseudoStyle(BEFORE) && (generatedContent = child->beforePseudoElementRenderer()))
+            generatedContent->destroy();
+        if (child->getCachedPseudoStyle(AFTER) && (generatedContent = child->afterPseudoElementRenderer()))
+            generatedContent->destroy();
+    }
+
     // Remove the old child.
     children()->removeChildNode(this, blockRunIn);
 
@@ -1590,16 +1600,11 @@ bool RenderBlock::handleRunInChild(RenderBox* child)
     RenderInline* inlineRunIn = new (renderArena()) RenderInline(runInNode ? runInNode : document());
     inlineRunIn->setStyle(blockRunIn->style());
 
-    bool runInIsGenerated = child->style()->styleType() == BEFORE || child->style()->styleType() == AFTER;
-
-    // Move the nodes from the old child to the new child, but skip any :before/:after content.  It has already
-    // been regenerated by the new inline.
+    // Move the nodes from the old child to the new child
     for (RenderObject* runInChild = blockRunIn->firstChild(); runInChild;) {
         RenderObject* nextSibling = runInChild->nextSibling();
-        if (runInIsGenerated || (runInChild->style()->styleType() != BEFORE && runInChild->style()->styleType() != AFTER)) {
-            blockRunIn->children()->removeChildNode(blockRunIn, runInChild, false);
-            inlineRunIn->addChild(runInChild); // Use addChild instead of appendChildNode since it handles correct placement of the children relative to :after-generated content.
-        }
+        blockRunIn->children()->removeChildNode(blockRunIn, runInChild, false);
+        inlineRunIn->addChild(runInChild); // Use addChild instead of appendChildNode since it handles correct placement of the children relative to :after-generated content.
         runInChild = nextSibling;
     }