Use-after-free of continuation in RenderBlock::paintContinuationOutlines()
authorrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Mar 2012 21:31:46 +0000 (21:31 +0000)
committerrobert@webkit.org <robert@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 20 Mar 2012 21:31:46 +0000 (21:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=81276

Reviewed by David Hyatt.

Source/WebCore:

Test: fast/css/relative-positioned-block-crash.html

https://trac.webkit.org/changeset/108185/ allowed anonymous blocks to get their own layer (when they're
relatively positioned). This broke the dependency in addContinuationWithOutline() on the owner of the continuation
table and the renderer getting added to it always being in the same layer. When they're not in the same layer
there's no guarantee that the owner of the continuation table will get painted again and so avoid any stale pointers
in its continuation table should any of the renderers in there get destroyed.

Fix this for now by only adding renderers to the containing block's continuation table if we don't have our own layer.
This fix causes fast/inline/continuation-outlines-with-layers.html to regress as it uses blocks inside relatively positioned
inlines, so skip it on all platforms pending a medium-term fix.

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

LayoutTests:

* fast/css/relative-positioned-block-crash-expected.txt: Added.
* fast/css/relative-positioned-block-crash.html: Added.
* platform/chromium/test_expectations.txt: Skip fast/inline/continuation-outlines-with-layers.html for now.
* platform/gtk/Skipped: ditto
* platform/mac/Skipped: ditto
* platform/qt/Skipped: ditto
* platform/win/Skipped: ditto

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

LayoutTests/ChangeLog
LayoutTests/fast/css/relative-positioned-block-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/relative-positioned-block-crash.html [new file with mode: 0644]
LayoutTests/platform/chromium/test_expectations.txt
LayoutTests/platform/gtk/Skipped
LayoutTests/platform/mac/Skipped
LayoutTests/platform/qt/Skipped
LayoutTests/platform/win/Skipped
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp

index fec4ffc..340596e 100644 (file)
@@ -1,3 +1,18 @@
+2012-03-20  Robert Hogan  <robert@webkit.org>
+
+        Use-after-free of continuation in RenderBlock::paintContinuationOutlines()
+        https://bugs.webkit.org/show_bug.cgi?id=81276
+
+        Reviewed by David Hyatt.
+
+        * fast/css/relative-positioned-block-crash-expected.txt: Added.
+        * fast/css/relative-positioned-block-crash.html: Added.
+        * platform/chromium/test_expectations.txt: Skip fast/inline/continuation-outlines-with-layers.html for now.
+        * platform/gtk/Skipped: ditto
+        * platform/mac/Skipped: ditto
+        * platform/qt/Skipped: ditto
+        * platform/win/Skipped: ditto
+
 2012-03-20  Dan Bernstein  <mitz@apple.com>
 
         Work around the inconsistency in test results caused by <http://webkit.org/b/81696> in a few
diff --git a/LayoutTests/fast/css/relative-positioned-block-crash-expected.txt b/LayoutTests/fast/css/relative-positioned-block-crash-expected.txt
new file mode 100644 (file)
index 0000000..7de8812
--- /dev/null
@@ -0,0 +1,4 @@
+PASSED: Didn't crash!
+
+A
+
diff --git a/LayoutTests/fast/css/relative-positioned-block-crash.html b/LayoutTests/fast/css/relative-positioned-block-crash.html
new file mode 100644 (file)
index 0000000..9ba7916
--- /dev/null
@@ -0,0 +1,57 @@
+<html>
+  <head>
+    <style>
+      #el0 { position: relative; }
+      #el2 { outline-style: dashed; }
+    </style>
+    <script>
+        if (window.layoutTestController) {
+            layoutTestController.display();
+            layoutTestController.waitUntilDone();
+        }
+
+        function log(message) {
+        var console = document.getElementById("console");
+        console.appendChild(document.createTextNode(message + "\n"));
+        }
+
+        function test() {
+           document.execCommand('removeformat');
+           setTimeout("finish()", 100);
+        }
+        function finish() {
+            if (window.layoutTestController) {
+                layoutTestController.dumpAsText();
+                log("PASSED: Didn't crash!\n");
+                layoutTestController.notifyDone();
+            }
+        }
+
+//         This tests a crash caused by stale pointers to continuations in a block's continuations table.
+//         https://bugs.webkit.org/show_bug.cgi?id=81276
+
+        function runTest() {
+            el0=document.createElement('span')
+            el0.setAttribute('id','el0')
+            document.body.appendChild(el0)
+            el1=document.createElement('div')
+            el0.appendChild(el1)
+            el1.appendChild(document.createElement('input'))
+            el0.appendChild(document.createTextNode('A'))
+            el2=document.createElement('q')
+            el2.setAttribute('id','el2')
+            document.body.appendChild(el2)
+            el2.appendChild(document.createElement('div'))
+            el2.appendChild(document.createElement('input'))
+            document.designMode='on'
+            document.execCommand('selectall')
+            document.execCommand('FormatBlock', false, '<'+'pre>')
+            document.execCommand('Undo')
+            setTimeout("test();",10)
+         }
+    </script>
+  </head>
+  <body onload="runTest();">
+  <div id="console"></div>
+  </body>
+</html>
\ No newline at end of file
index 5f9b71f..8135083 100644 (file)
@@ -3949,3 +3949,6 @@ BUGWK81537 : fast/events/touch/gesture/pad-gesture-fling.html = TEXT
 BUGWK81544 : fast/text/international/font-fallback-to-common-script.html = IMAGE PASS
 
 BUGWK81638 SNOWLEOPARD DEBUG : editing/selection/iframe.html = IMAGE PASS
+
+// Allowed to regress to fix a crash. 
+BUGWK81276 WIN LINUX: fast/inline/continuation-outlines-with-layers.html = IMAGE
index 35df71a..ea27f07 100644 (file)
@@ -1601,3 +1601,7 @@ editing/selection/move-by-word-visually-wrong-left-right.html
 
 # https://bugs.webkit.org/show_bug.cgi?id=43022
 tables/mozilla_expected_failures/bugs/bug85016.html
+
+# https://bugs.webkit.org/show_bug.cgi?id=81276
+# Allowed to regress to fix a crash. 
+fast/inline/continuation-outlines-with-layers.html
index 9b16b3f..b5fe301 100644 (file)
@@ -603,3 +603,6 @@ media/audio-garbage-collect.html
 # http://bugs.webkit.org/show_bug.cgi?id=81618
 fast/workers/storage/use-same-database-in-page-and-workers.html
 
+# https://bugs.webkit.org/show_bug.cgi?id=81276
+# Allowed to regress to fix a crash. 
+fast/inline/continuation-outlines-with-layers.html
index 34dd485..ddd6d3b 100644 (file)
@@ -2764,3 +2764,7 @@ editing/selection/move-by-word-visually-single-space-inline-element.html
 editing/selection/move-by-word-visually-single-space-one-element.html
 editing/selection/move-by-word-visually-textarea.html
 editing/selection/move-by-word-visually-wrong-left-right.html
+
+# https://bugs.webkit.org/show_bug.cgi?id=81276
+# Allowed to regress to fix a crash. 
+fast/inline/continuation-outlines-with-layers.html
index 3fe7686..a76d884 100644 (file)
@@ -1856,3 +1856,7 @@ svg/custom/delete-text-crash.html
 
 # https://bugs.webkit.org/show_bug.cgi?id=43022
 tables/mozilla_expected_failures/bugs/bug85016.html
+
+# https://bugs.webkit.org/show_bug.cgi?id=81276
+# Allowed to regress to fix a crash. 
+fast/inline/continuation-outlines-with-layers.html
index bfd731a..74656c6 100644 (file)
@@ -1,3 +1,25 @@
+2012-03-20  Robert Hogan  <robert@webkit.org>
+
+        Use-after-free of continuation in RenderBlock::paintContinuationOutlines()
+        https://bugs.webkit.org/show_bug.cgi?id=81276
+
+        Reviewed by David Hyatt.
+
+        Test: fast/css/relative-positioned-block-crash.html
+
+        https://trac.webkit.org/changeset/108185/ allowed anonymous blocks to get their own layer (when they're
+        relatively positioned). This broke the dependency in addContinuationWithOutline() on the owner of the continuation
+        table and the renderer getting added to it always being in the same layer. When they're not in the same layer
+        there's no guarantee that the owner of the continuation table will get painted again and so avoid any stale pointers
+        in its continuation table should any of the renderers in there get destroyed.
+
+        Fix this for now by only adding renderers to the containing block's continuation table if we don't have our own layer.
+        This fix causes fast/inline/continuation-outlines-with-layers.html to regress as it uses blocks inside relatively positioned
+        inlines, so skip it on all platforms pending a medium-term fix.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::paintObject):
+
 2012-03-20  Adele Peterson  <adele@apple.com>
 
         "Attempt to insert nil value " exception when calling attributed string APIs on content with a custom font
index 9a4220c..9c7203d 100755 (executable)
@@ -2925,7 +2925,10 @@ void RenderBlock::paintObject(PaintInfo& paintInfo, const LayoutPoint& paintOffs
     // 6. paint continuation outlines.
     if ((paintPhase == PaintPhaseOutline || paintPhase == PaintPhaseChildOutlines)) {
         RenderInline* inlineCont = inlineElementContinuation();
-        if (inlineCont && inlineCont->hasOutline() && inlineCont->style()->visibility() == VISIBLE) {
+        // FIXME: For now, do not add continuations for outline painting by our containing block if we are a relative positioned
+        // anonymous block (i.e. have our own layer). This is because a block depends on renderers in its continuation table being
+        // in the same layer. 
+        if (inlineCont && inlineCont->hasOutline() && inlineCont->style()->visibility() == VISIBLE && !hasLayer()) {
             RenderInline* inlineRenderer = toRenderInline(inlineCont->node()->renderer());
             RenderBlock* cb = containingBlock();