Crash in RenderTableSection::paintCell.
authorinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 26 May 2012 00:28:23 +0000 (00:28 +0000)
committerinferno@chromium.org <inferno@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 26 May 2012 00:28:23 +0000 (00:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=87445

Reviewed by Eric Seidel and Julien Chaffraix.

Source/WebCore:

Fix the crash by preventing table parts from being set
as layout root. This prevents us from accessing removed
table cells which can happen if RenderTableSection::layout
is called directly without calling RenderTable::layout first
(in case of cell recalc).

Add ASSERTs to RenderTableSection::layout to prevent
layout to happen when we are already pending cell recalc
or our table is pending section recalc. In those cases,
RenderTable::layout should be called first to relayout
the entire table.

Test: tables/table-section-overflow-clip-crash.html

* rendering/RenderObject.cpp:
(WebCore::objectIsRelayoutBoundary):
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::layout):

LayoutTests:

* tables/table-section-overflow-clip-crash-expected.txt: Added.
* tables/table-section-overflow-clip-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/tables/table-section-overflow-clip-crash-expected.txt [new file with mode: 0644]
LayoutTests/tables/table-section-overflow-clip-crash.html [new file with mode: 0755]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderTableSection.cpp

index 7df96a5..b90301a 100644 (file)
@@ -1,3 +1,13 @@
+2012-05-25  Abhishek Arya  <inferno@chromium.org>
+
+        Crash in RenderTableSection::paintCell.
+        https://bugs.webkit.org/show_bug.cgi?id=87445
+
+        Reviewed by Eric Seidel and Julien Chaffraix.
+
+        * tables/table-section-overflow-clip-crash-expected.txt: Added.
+        * tables/table-section-overflow-clip-crash.html: Added.
+
 2012-05-25  Jessie Berlin  <jberlin@apple.com>
 
         Implement spinbutton support in RenderThemeSafari
diff --git a/LayoutTests/tables/table-section-overflow-clip-crash-expected.txt b/LayoutTests/tables/table-section-overflow-clip-crash-expected.txt
new file mode 100644 (file)
index 0000000..5ace1f1
--- /dev/null
@@ -0,0 +1,2 @@
+WebKit Bug 87445 - RenderTableSection::paintCell.
+Test passes if it does not crash.
diff --git a/LayoutTests/tables/table-section-overflow-clip-crash.html b/LayoutTests/tables/table-section-overflow-clip-crash.html
new file mode 100755 (executable)
index 0000000..0cb478a
--- /dev/null
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#test0 {
+    counter-reset: c;
+}
+#test0::after {
+    content: counter(c);
+    counter-reset: c;
+}
+#test1::after {
+    content: counter(c);
+    counter-reset: c;
+}
+#test2 {
+    counter-reset: c;
+    height: 1px;
+    width: 1px;
+    overflow-x: scroll;
+    -webkit-perspective: 1;
+}
+#test3 {
+    content: counter(c);
+    -webkit-animation-name: a;
+    -webkit-animation-duration: 0.01s;
+}
+</style>
+<script>
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+function finish() {
+    document.body.innerHTML = "WebKit Bug 87445 - RenderTableSection::paintCell.<br/>Test passes if it does not crash.";
+    if (window.layoutTestController)
+        layoutTestController.notifyDone();
+}
+
+onload = function() {
+    test0 = document.createElement('div');
+    test0.setAttribute('id', 'test0');
+    document.body.appendChild(test0);
+    test1 = document.createElement('div');
+    test1.setAttribute('id', 'test1');
+    test0.appendChild(test1);
+    test2 = document.createElement('div');
+    test2.setAttribute('id', 'test2');
+    test1.appendChild(test2);
+    test3 = document.createElement('div');
+    test3.setAttribute('id', 'test3');
+    test2.appendChild(test3);
+    test2.style.display = 'table-footer-group';
+    document.body.offsetTop;
+    setTimeout("finish()", 10);
+}
+</script>
+</head>
+<body>
+</body>
+</html>
index e2c513c..3612eec 100644 (file)
@@ -1,3 +1,29 @@
+2012-05-25  Abhishek Arya  <inferno@chromium.org>
+
+        Crash in RenderTableSection::paintCell.
+        https://bugs.webkit.org/show_bug.cgi?id=87445
+
+        Reviewed by Eric Seidel and Julien Chaffraix.
+
+        Fix the crash by preventing table parts from being set
+        as layout root. This prevents us from accessing removed
+        table cells which can happen if RenderTableSection::layout
+        is called directly without calling RenderTable::layout first
+        (in case of cell recalc).
+
+        Add ASSERTs to RenderTableSection::layout to prevent
+        layout to happen when we are already pending cell recalc
+        or our table is pending section recalc. In those cases,
+        RenderTable::layout should be called first to relayout
+        the entire table.
+
+        Test: tables/table-section-overflow-clip-crash.html
+
+        * rendering/RenderObject.cpp:
+        (WebCore::objectIsRelayoutBoundary):
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::layout):
+
 2012-05-25  Philip Rogers  <pdr@google.com>
 
         Fix for self-closing <use> tags
index 56b00c5..1cd6d36 100755 (executable)
@@ -595,14 +595,26 @@ RenderBlock* RenderObject::firstLineBlock() const
 
 static inline bool objectIsRelayoutBoundary(const RenderObject* object)
 {
-    // FIXME: In future it may be possible to broaden this condition in order to improve performance.
-    // Table cells are excluded because even when their CSS height is fixed, their height()
-    // may depend on their contents.
-    return object->isTextControl()
+    // FIXME: In future it may be possible to broaden these conditions in order to improve performance.
+    if (object->isTextControl())
+        return true;
+
 #if ENABLE(SVG)
-        || object->isSVGRoot()
+    if (object->isSVGRoot())
+        return true;
 #endif
-        || (object->hasOverflowClip() && !object->style()->width().isIntrinsicOrAuto() && !object->style()->height().isIntrinsicOrAuto() && !object->style()->height().isPercent() && !object->isTableCell());
+
+    if (!object->hasOverflowClip())
+        return false;
+
+    if (object->style()->width().isIntrinsicOrAuto() || object->style()->height().isIntrinsicOrAuto() || object->style()->height().isPercent())
+        return false;
+
+    // Table parts can't be relayout roots since the table is responsible for layouting all the parts.
+    if (object->isTablePart())
+        return false;
+
+    return true;
 }
 
 void RenderObject::markContainingBlocksForLayout(bool scheduleRelayout, RenderObject* newRoot)
index 7f44a75..db25e81 100644 (file)
@@ -403,6 +403,8 @@ int RenderTableSection::calcRowLogicalHeight()
 void RenderTableSection::layout()
 {
     ASSERT(needsLayout());
+    ASSERT(!needsCellRecalc());
+    ASSERT(!table()->needsSectionRecalc());
 
     LayoutStateMaintainer statePusher(view(), this, locationOffset(), style()->isFlippedBlocksWritingMode());
     for (RenderObject* child = children()->firstChild(); child; child = child->nextSibling()) {