Absolute positioned objects should not be added to anonymous block lists
authorkenrb@chromium.org <kenrb@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Jun 2012 22:34:55 +0000 (22:34 +0000)
committerkenrb@chromium.org <kenrb@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Jun 2012 22:34:55 +0000 (22:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=87768

Reviewed by Abhishek Arya.

Source/WebCore:

containingBlock() was returning an anonymous block for absolute
positioned objects under a relative positioned inline in the case
that the inline is split and the object is underneath the block
continuation. Anonymous blocks should never have anything in their
positioned object lists because they can be destroyed at any time
for a different reasons such as anonymous block merging, which is
a problem for layout if they have m_posChildNeedsLayout set.

This patch adds a generic check for anonymous blocks in
containingBlock() to correct this problem.

* rendering/RenderObject.cpp:
(WebCore::RenderObject::containingBlock):

LayoutTests:

Test crashing condition in bug 87768.

* fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash-expected.txt: Added
* fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash.html: Added
* fast/block/positioning/insert-positioned-in-anonymous-crash-expected.txt: Added
* fast/block/positioning/insert-positioned-in-anonymous-crash.html: Added

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

LayoutTests/ChangeLog
LayoutTests/fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash.html [new file with mode: 0644]
LayoutTests/fast/block/positioning/insert-positioned-in-anonymous-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/block/positioning/insert-positioned-in-anonymous-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderObject.cpp

index 54818c5..813e291 100644 (file)
@@ -1,3 +1,17 @@
+2012-06-19  Ken Buchanan  <kenrb@chromium.org>
+
+        Absolute positioned objects should not be added to anonymous block lists
+        https://bugs.webkit.org/show_bug.cgi?id=87768
+
+        Reviewed by Abhishek Arya.
+
+        Test crashing condition in bug 87768.
+
+        * fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash-expected.txt: Added
+        * fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash.html: Added
+        * fast/block/positioning/insert-positioned-in-anonymous-crash-expected.txt: Added
+        * fast/block/positioning/insert-positioned-in-anonymous-crash.html: Added
+
 2012-06-19  Jan Keromnes  <janx@linux.com>
 
         Web Inspector: extensionPanel.onSearch `action` strings should be enumerated
diff --git a/LayoutTests/fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash-expected.txt b/LayoutTests/fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash-expected.txt
new file mode 100644 (file)
index 0000000..2afa0bf
--- /dev/null
@@ -0,0 +1 @@
+PASS. WebKit didn't crash.
diff --git a/LayoutTests/fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash.html b/LayoutTests/fast/block/positioning/abspositioned-object-under-split-relpositioned-inline-crash.html
new file mode 100644 (file)
index 0000000..d9523e7
--- /dev/null
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+.class1 { position: relative; }
+.class2 { position: absolute; }
+</style>
+<script>
+window.onload = function() {
+    document.designMode="on";
+    document.execCommand("SelectAll");
+    document.execCommand("Strikethrough");
+    document.body.offsetTop;
+    document.body.innerHTML = "PASS. WebKit didn't crash.";
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+}
+</script>
+<i class="class1">
+<span class="class2"></span>
+<div>
+<xmp class="class2">
+A
+</xmp>
+</div>
+</i>
+</body>
+</html>
diff --git a/LayoutTests/fast/block/positioning/insert-positioned-in-anonymous-crash-expected.txt b/LayoutTests/fast/block/positioning/insert-positioned-in-anonymous-crash-expected.txt
new file mode 100644 (file)
index 0000000..53cdf1e
--- /dev/null
@@ -0,0 +1 @@
+PASSED
diff --git a/LayoutTests/fast/block/positioning/insert-positioned-in-anonymous-crash.html b/LayoutTests/fast/block/positioning/insert-positioned-in-anonymous-crash.html
new file mode 100644 (file)
index 0000000..6f5add8
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <style>
+      .inlineContainer {
+        position: relative;
+        display: inline;
+      }
+      #positioned {
+        position: absolute;
+      }
+    </style>
+    <script>
+      if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+      function runTest() {
+        document.getElementById('positioned').innerHTML = 'PASSED';
+      }
+    </script>
+  </head>
+  <body onload="runTest()">
+    <div class='inlineContainer'>
+      <div>
+        <div id='positioned'>FAILED</div>
+      </div>
+    </div>
+  </body>
+</html>
index 609879f..ad271c9 100644 (file)
@@ -1,3 +1,24 @@
+2012-06-19  Ken Buchanan  <kenrb@chromium.org>
+
+        Absolute positioned objects should not be added to anonymous block lists
+        https://bugs.webkit.org/show_bug.cgi?id=87768
+
+        Reviewed by Abhishek Arya.
+
+        containingBlock() was returning an anonymous block for absolute
+        positioned objects under a relative positioned inline in the case
+        that the inline is split and the object is underneath the block
+        continuation. Anonymous blocks should never have anything in their
+        positioned object lists because they can be destroyed at any time
+        for a different reasons such as anonymous block merging, which is
+        a problem for layout if they have m_posChildNeedsLayout set.
+
+        This patch adds a generic check for anonymous blocks in
+        containingBlock() to correct this problem.
+
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::containingBlock):
+
 2012-06-19  Jan Keromnes  <janx@linux.com>
 
         Web Inspector: extensionPanel.onSearch `action` strings should be
index 92dfe28..8037760 100755 (executable)
@@ -724,7 +724,11 @@ RenderBlock* RenderObject::containingBlock() const
     if (!o && isRenderScrollbarPart())
         o = toRenderScrollbarPart(this)->rendererOwningScrollbar();
     if (!isText() && m_style->position() == FixedPosition) {
-        while (o && !o->isRenderView() && !(o->hasTransform() && o->isRenderBlock())) {
+        while (o) {
+            if (o->isRenderView())
+                break;
+            if (o->hasTransform() && o->isRenderBlock())
+                break;
 #if ENABLE(SVG)
             // foreignObject is the containing block for its contents.
             if (o->isSVGForeignObject())
@@ -732,18 +736,24 @@ RenderBlock* RenderObject::containingBlock() const
 #endif
             o = o->parent();
         }
+        ASSERT(!o->isAnonymousBlock());
     } else if (!isText() && m_style->position() == AbsolutePosition) {
-        while (o && (o->style()->position() == StaticPosition || (o->isInline() && !o->isReplaced())) && !o->isRenderView() && !(o->hasTransform() && o->isRenderBlock())) {
+        while (o) {
             // For relpositioned inlines, we return the nearest non-anonymous enclosing block. We don't try
             // to return the inline itself.  This allows us to avoid having a positioned objects
             // list in all RenderInlines and lets us return a strongly-typed RenderBlock* result
             // from this method.  The container() method can actually be used to obtain the
             // inline directly.
+            if (!o->style()->position() == StaticPosition && !(o->isInline() && !o->isReplaced()))
+                break;
+            if (o->isRenderView())
+                break;
+            if (o->hasTransform() && o->isRenderBlock())
+                break;
+
             if (o->style()->position() == RelativePosition && o->isInline() && !o->isReplaced()) {
-                RenderBlock* relPositionedInlineContainingBlock = o->containingBlock();
-                while (relPositionedInlineContainingBlock->isAnonymousBlock())
-                    relPositionedInlineContainingBlock = relPositionedInlineContainingBlock->containingBlock();
-                return relPositionedInlineContainingBlock;
+                o = o->containingBlock();
+                break;
             }
 #if ENABLE(SVG)
             if (o->isSVGForeignObject()) //foreignObject is the containing block for contents inside it
@@ -752,6 +762,9 @@ RenderBlock* RenderObject::containingBlock() const
 
             o = o->parent();
         }
+
+        while (o && o->isAnonymousBlock())
+            o = o->containingBlock();
     } else {
         while (o && ((o->isInline() && !o->isReplaced()) || !o->isRenderBlock()))
             o = o->parent();