Crash in RenderBlock::addChildIgnoringAnonymousColumnBlocks.
authorinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Feb 2012 05:36:11 +0000 (05:36 +0000)
committerinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Feb 2012 05:36:11 +0000 (05:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79043

Reviewed by Julien Chaffraix.

Source/WebCore:

Tests: fast/runin/runin-div-before-child.html
       fast/runin/runin-table-before-child.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::addChildIgnoringAnonymousColumnBlocks): handle
the case of run-in elements and strengthen code to handle cases where
beforeChild is incorrectly set.
* rendering/RenderObject.h: remove anonymousContainer function since
the new logic in RenderBlock does not need it.

LayoutTests:

* fast/runin/runin-div-before-child-expected.png: Added.
* fast/runin/runin-div-before-child-expected.txt: Added.
* fast/runin/runin-div-before-child.html: Added.
* fast/runin/runin-table-before-child-expected.png: Added.
* fast/runin/runin-table-before-child-expected.txt: Added.
* fast/runin/runin-table-before-child.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/runin/runin-div-before-child-expected.png [new file with mode: 0644]
LayoutTests/fast/runin/runin-div-before-child-expected.txt [new file with mode: 0644]
LayoutTests/fast/runin/runin-div-before-child.html [new file with mode: 0755]
LayoutTests/fast/runin/runin-table-before-child-expected.png [new file with mode: 0644]
LayoutTests/fast/runin/runin-table-before-child-expected.txt [new file with mode: 0644]
LayoutTests/fast/runin/runin-table-before-child.html [new file with mode: 0755]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderObject.h

index 3073756..57c6d20 100644 (file)
@@ -1,3 +1,17 @@
+2012-02-22  Abhishek Arya  <inferno@chromium.org>
+
+        Crash in RenderBlock::addChildIgnoringAnonymousColumnBlocks.
+        https://bugs.webkit.org/show_bug.cgi?id=79043
+
+        Reviewed by Julien Chaffraix.
+
+        * fast/runin/runin-div-before-child-expected.png: Added.
+        * fast/runin/runin-div-before-child-expected.txt: Added.
+        * fast/runin/runin-div-before-child.html: Added.
+        * fast/runin/runin-table-before-child-expected.png: Added.
+        * fast/runin/runin-table-before-child-expected.txt: Added.
+        * fast/runin/runin-table-before-child.html: Added.
+
 2012-02-22  Wei James  <james.wei@intel.com>
 
         Add multi channels support in AudioBus and AudioBufferSourceNode
diff --git a/LayoutTests/fast/runin/runin-div-before-child-expected.png b/LayoutTests/fast/runin/runin-div-before-child-expected.png
new file mode 100644 (file)
index 0000000..a32edcf
Binary files /dev/null and b/LayoutTests/fast/runin/runin-div-before-child-expected.png differ
diff --git a/LayoutTests/fast/runin/runin-div-before-child-expected.txt b/LayoutTests/fast/runin/runin-div-before-child-expected.txt
new file mode 100644 (file)
index 0000000..7a3ff7d
--- /dev/null
@@ -0,0 +1,14 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x116
+  RenderBlock {HTML} at (0,0) size 800x116 [color=#FFFFFF]
+    RenderBody {BODY} at (8,8) size 784x100
+      RenderBlock {DIV} at (0,0) size 784x50
+        RenderText {#text} at (0,0) size 50x50
+          text run at (0,0) width 50: "A"
+      RenderBlock {DIV} at (0,50) size 784x50
+        RenderInline (run-in) {DIV} at (0,0) size 50x50
+          RenderText {#text} at (0,0) size 50x50
+            text run at (0,0) width 50: "B"
+        RenderText {#text} at (50,0) size 50x50
+          text run at (50,0) width 50: "C"
diff --git a/LayoutTests/fast/runin/runin-div-before-child.html b/LayoutTests/fast/runin/runin-div-before-child.html
new file mode 100755 (executable)
index 0000000..de68a0a
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>\r
+<html style="font-family: ahem; font-size: 50px; -webkit-font-smoothing: none; color: white;">\r
+<body>\r
+<div id="runin" style="display: run-in">B</div>\r
+<div>C</div>\r
+<script>\r
+document.body.offsetTop;\r
+var div1 = document.createElement('div');\r
+div1.appendChild(document.createTextNode('A'));\r
+document.body.insertBefore(div1, document.getElementById('runin'));\r
+</script>\r
+</body>\r
+</html>\r
diff --git a/LayoutTests/fast/runin/runin-table-before-child-expected.png b/LayoutTests/fast/runin/runin-table-before-child-expected.png
new file mode 100644 (file)
index 0000000..a32edcf
Binary files /dev/null and b/LayoutTests/fast/runin/runin-table-before-child-expected.png differ
diff --git a/LayoutTests/fast/runin/runin-table-before-child-expected.txt b/LayoutTests/fast/runin/runin-table-before-child-expected.txt
new file mode 100644 (file)
index 0000000..1b87ccb
--- /dev/null
@@ -0,0 +1,17 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x116
+  RenderBlock {HTML} at (0,0) size 800x116 [color=#FFFFFF]
+    RenderBody {BODY} at (8,8) size 784x100
+      RenderTable at (0,0) size 50x50
+        RenderTableSection (anonymous) at (0,0) size 50x50
+          RenderTableRow {TR} at (0,0) size 50x50
+            RenderTableCell (anonymous) at (0,0) size 50x50 [r=0 c=0 rs=1 cs=1]
+              RenderText {#text} at (0,0) size 50x50
+                text run at (0,0) width 50: "A"
+      RenderBlock {DIV} at (0,50) size 784x50
+        RenderInline (run-in) {DIV} at (0,0) size 50x50
+          RenderText {#text} at (0,0) size 50x50
+            text run at (0,0) width 50: "B"
+        RenderText {#text} at (50,0) size 50x50
+          text run at (50,0) width 50: "C"
diff --git a/LayoutTests/fast/runin/runin-table-before-child.html b/LayoutTests/fast/runin/runin-table-before-child.html
new file mode 100755 (executable)
index 0000000..1464d13
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>\r
+<html style="font-family: ahem; font-size: 50px; -webkit-font-smoothing: none; color: white;">\r
+<body>\r
+<div id="runin" style="display: run-in">B</div>\r
+<div>C</div>\r
+<script>\r
+document.body.offsetTop;\r
+var tr1 = document.createElement('tr');\r
+tr1.appendChild(document.createTextNode('A'));\r
+document.body.insertBefore(tr1, document.getElementById('runin'));\r
+</script>\r
+</body>\r
+</html>\r
index e5b511d..aabda48 100644 (file)
@@ -1,3 +1,20 @@
+2012-02-22  Abhishek Arya  <inferno@chromium.org>
+
+        Crash in RenderBlock::addChildIgnoringAnonymousColumnBlocks.
+        https://bugs.webkit.org/show_bug.cgi?id=79043
+
+        Reviewed by Julien Chaffraix.
+
+        Tests: fast/runin/runin-div-before-child.html
+               fast/runin/runin-table-before-child.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::addChildIgnoringAnonymousColumnBlocks): handle
+        the case of run-in elements and strengthen code to handle cases where
+        beforeChild is incorrectly set.
+        * rendering/RenderObject.h: remove anonymousContainer function since
+        the new logic in RenderBlock does not need it.
+
 2012-02-22  Hayato Ito  <hayato@chromium.org>
 
         Make ShadowRootList manage a node distribution.
index c5e94c1..3591657 100755 (executable)
@@ -781,34 +781,48 @@ void RenderBlock::addChildIgnoringAnonymousColumnBlocks(RenderObject* newChild,
     if (!beforeChild)
         beforeChild = afterPseudoElementRenderer();
 
-    // If the requested beforeChild is not one of our children, then this is because
-    // there is an anonymous container within this object that contains the beforeChild.
     if (beforeChild && beforeChild->parent() != this) {
-        RenderObject* beforeChildAnonymousContainer = anonymousContainer(beforeChild);
-        ASSERT(beforeChildAnonymousContainer);
-        ASSERT(beforeChildAnonymousContainer->isAnonymous());
-
-        if (beforeChildAnonymousContainer->isAnonymousBlock()) {
-            // Insert the child into the anonymous block box instead of here.
-            if (newChild->isInline() || beforeChild->parent()->firstChild() != beforeChild)
-                beforeChild->parent()->addChild(newChild, beforeChild);
-            else
-                addChild(newChild, beforeChild->parent());
-            return;
-        }
+        RenderObject* beforeChildContainer = beforeChild->parent();
+        while (beforeChildContainer->parent() != this)
+            beforeChildContainer = beforeChildContainer->parent();
+        ASSERT(beforeChildContainer);
+
+        if (beforeChildContainer->isAnonymous()) {
+            // If the requested beforeChild is not one of our children, then this is because
+            // there is an anonymous container within this object that contains the beforeChild.
+            RenderObject* beforeChildAnonymousContainer = beforeChildContainer;
+            if (beforeChildAnonymousContainer->isAnonymousBlock()) {
+                // Insert the child into the anonymous block box instead of here.
+                if (newChild->isInline() || beforeChild->parent()->firstChild() != beforeChild)
+                    beforeChild->parent()->addChild(newChild, beforeChild);
+                else
+                    addChild(newChild, beforeChild->parent());
+                return;
+            }
 
-        ASSERT(beforeChildAnonymousContainer->isTable());
-        if ((newChild->isTableCol() && newChild->style()->display() == TABLE_COLUMN_GROUP)
-                || (newChild->isTableCaption())
-                || newChild->isTableSection()
-                || newChild->isTableRow()
-                || newChild->isTableCell()) {
-            // Insert into the anonymous table.
-            beforeChildAnonymousContainer->addChild(newChild, beforeChild);
-            return;
-        }
+            ASSERT(beforeChildAnonymousContainer->isTable());
+            if (newChild->isTablePart()) {
+                // Insert into the anonymous table.
+                beforeChildAnonymousContainer->addChild(newChild, beforeChild);
+                return;
+            }
 
-        beforeChild = splitTablePartsAroundChild(beforeChild);
+            beforeChild = splitTablePartsAroundChild(beforeChild);
+
+            ASSERT(beforeChild->parent() == this);
+            if (beforeChild->parent() != this) {
+                // We should never reach here. If we do, we need to use the
+                // safe fallback to use the topmost beforeChild container.
+                beforeChild = beforeChildContainer;
+            }
+        } else {
+            // We will reach here when beforeChild is a run-in element.
+            // If run-in element precedes a block-level element, it becomes the
+            // the first inline child of that block level element. The insertion
+            // point will be before that block-level element.
+            ASSERT(beforeChild->isRunIn());
+            beforeChild = beforeChildContainer;
+        }
     }
 
     // Check for a spanning element in columns.
index 9d4ca1f..d5f813d 100644 (file)
@@ -364,16 +364,6 @@ public:
     static inline bool isAfterContent(const RenderObject* obj) { return obj && obj->isAfterContent(); }
     static inline bool isBeforeOrAfterContent(const RenderObject* obj) { return obj && obj->isBeforeOrAfterContent(); }
 
-    inline RenderObject* anonymousContainer(RenderObject* child)
-    {
-         RenderObject* container = child;
-         while (container->parent() != this)
-             container = container->parent();
-
-         ASSERT(container->isAnonymous());
-         return container;
-    }
-
     bool hasCounterNodeMap() const { return m_bitfields.hasCounterNodeMap(); }
     void setHasCounterNodeMap(bool hasCounterNodeMap) { m_bitfields.setHasCounterNodeMap(hasCounterNodeMap); }
     bool everHadLayout() const { return m_bitfields.everHadLayout(); }