Diff
Modified: trunk/LayoutTests/ChangeLog (99211 => 99212)
--- trunk/LayoutTests/ChangeLog 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/LayoutTests/ChangeLog 2011-11-03 17:20:01 UTC (rev 99212)
@@ -1,3 +1,20 @@
+2011-11-03 Julien Chaffraix <[email protected]>
+
+ Stop abusing RenderTableSection::needsRecalcCells logic
+ https://bugs.webkit.org/show_bug.cgi?id=71420
+
+ Reviewed by Darin Adler.
+
+ * platform/chromium-linux/fast/repaint/table-extra-bottom-grow-expected.png:
+ Update this test as this is a progression: we are not over-repainting the table
+ anymore.
+
+ * platform/chromium/test_expectations.txt:
+ * platform/efl/Skipped:
+ * platform/mac/Skipped:
+ * platform/qt/test_expectations.txt:
+ Skipped the test here as it needs a rebaseline.
+
2011-11-03 Kentaro Hara <[email protected]>
Unreviewed rebaselining. Unskipped progress-event-constructor.html in Qt-Arm
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (99211 => 99212)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-11-03 17:20:01 UTC (rev 99212)
@@ -3886,4 +3886,6 @@
BUGWK71451 SLOW : fast/frames/sandboxed-iframe-navigation-windowopen.html = PASS
BUGWK71451 LINUX : http/tests/security/contentSecurityPolicy/policy-does-not-affect-child.html = TEXT PASS
BUGWK71451 LINUX : http/tests/security/contentSecurityPolicy/object-src-none-allowed.html = TEXT PASS
-
+
+// This just needs a rebaseline.
+BUGWK71420 WIN : fast/repaint/table-extra-bottom-grow.html = IMAGE
Modified: trunk/LayoutTests/platform/chromium-linux/fast/repaint/table-extra-bottom-grow-expected.png
(Binary files differ)
Modified: trunk/LayoutTests/platform/efl/Skipped (99211 => 99212)
--- trunk/LayoutTests/platform/efl/Skipped 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/LayoutTests/platform/efl/Skipped 2011-11-03 17:20:01 UTC (rev 99212)
@@ -1923,3 +1923,6 @@
# Tests for MediaSource API. Feature is not yet functional.
# https://bugs.webkit.org/show_bug.cgi?id=64731
http/tests/media/media-source/
+
+# Needs a rebaseline
+fast/repaint/table-extra-bottom-grow.html
Modified: trunk/LayoutTests/platform/mac/Skipped (99211 => 99212)
--- trunk/LayoutTests/platform/mac/Skipped 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/LayoutTests/platform/mac/Skipped 2011-11-03 17:20:01 UTC (rev 99212)
@@ -467,3 +467,7 @@
# https://bugs.webkit.org/show_bug.cgi?id=70989
fast/canvas/webgl/attrib-location-length-limits.html
fast/canvas/webgl/uniform-location-length-limits.html
+
+# https://bugs.webkit.org/show_bug.cgi?id=71420
+# It needs a rebaseline
+fast/repaint/table-extra-bottom-grow.html
Modified: trunk/LayoutTests/platform/qt/test_expectations.txt (99211 => 99212)
--- trunk/LayoutTests/platform/qt/test_expectations.txt 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/LayoutTests/platform/qt/test_expectations.txt 2011-11-03 17:20:01 UTC (rev 99212)
@@ -25,3 +25,6 @@
BUGWK67007 DEBUG : fast/ruby/generated-before-and-after-counter-doesnt-crash.html = CRASH
BUGWK62662 DEBUG : inspector/cookie-parser.html = CRASH PASS
+
+// Needs a rebaseline
+BUGWK71420 : fast/repaint/table-extra-bottom-grow.html = IMAGE
Modified: trunk/Source/WebCore/ChangeLog (99211 => 99212)
--- trunk/Source/WebCore/ChangeLog 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/Source/WebCore/ChangeLog 2011-11-03 17:20:01 UTC (rev 99212)
@@ -1,3 +1,41 @@
+2011-11-03 Julien Chaffraix <[email protected]>
+
+ Stop abusing RenderTableSection::needsRecalcCells logic
+ https://bugs.webkit.org/show_bug.cgi?id=71420
+
+ Reviewed by Darin Adler.
+
+ Change covered by existing tests like fast/repaint/table-extra-bottom-grow.html
+ and fast/table/row-height-recalc* (among others).
+
+ Cell recalculation is very expensive and should only be called when the section's structure
+ changed in a way that requires a safe update to its structure (like removing a row as our
+ column split may not be appropriate anymore).
+
+ The current code would abuse cell recalculation to actually reset the logical height on the
+ RowStruct. This change makes it do the right thing.
+
+ * rendering/RenderTableCell.h:
+ * rendering/RenderTableRow.h:
+ Removed styleWillChange override as it was unneeded.
+
+ * rendering/RenderTableCell.cpp:
+ (WebCore::RenderTableCell::styleDidChange):
+ * rendering/RenderTableRow.cpp:
+ (WebCore::RenderTableRow::styleDidChange):
+ Move the code from styleWillChange to styleDidChange.
+
+ * rendering/RenderTableSection.cpp:
+ (WebCore::RenderTableSection::rowLogicalHeightChanged):
+ This function just reset the height on the |RowStruct| which is the
+ only part of recalcCells that we would need.
+
+ (WebCore::RenderTableSection::rowIndexForRenderer):
+ Added this function to find out which index a column has (strangely
+ RenderTableRow does not have this information).
+
+ * rendering/RenderTableSection.h: Added the 2 previous functions.
+
2011-11-03 Andreas Kling <[email protected]>
CSSRuleList: Move rule orphaning from deleteRule() out to callers.
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (99211 => 99212)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2011-11-03 17:20:01 UTC (rev 99212)
@@ -310,21 +310,16 @@
return paddingBefore() + borderBefore() + contentLogicalHeight();
}
-void RenderTableCell::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
-{
- if (parent() && section() && style() && style()->height() != newStyle->height())
- section()->setNeedsCellRecalc();
-
- ASSERT(newStyle->display() == TABLE_CELL);
-
- RenderBlock::styleWillChange(diff, newStyle);
-}
-
void RenderTableCell::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
{
+ ASSERT(style()->display() == TABLE_CELL);
+
RenderBlock::styleDidChange(diff, oldStyle);
setHasBoxDecorations(true);
+ if (parent() && section() && oldStyle && style()->height() != oldStyle->height())
+ section()->rowLogicalHeightChanged(row());
+
// If border was changed, notify table.
if (parent()) {
RenderTable* table = this->table();
Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (99211 => 99212)
--- trunk/Source/WebCore/rendering/RenderTableCell.h 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h 2011-11-03 17:20:01 UTC (rev 99212)
@@ -153,7 +153,6 @@
void setCellWidthChanged(bool b = true) { m_cellWidthChanged = b; }
protected:
- virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle);
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
private:
Modified: trunk/Source/WebCore/rendering/RenderTableRow.cpp (99211 => 99212)
--- trunk/Source/WebCore/rendering/RenderTableRow.cpp 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/Source/WebCore/rendering/RenderTableRow.cpp 2011-11-03 17:20:01 UTC (rev 99212)
@@ -53,16 +53,6 @@
recalcSection->setNeedsCellRecalc();
}
-void RenderTableRow::styleWillChange(StyleDifference diff, const RenderStyle* newStyle)
-{
- if (section() && style() && style()->logicalHeight() != newStyle->logicalHeight())
- section()->setNeedsCellRecalc();
-
- ASSERT(newStyle->display() == TABLE_ROW);
-
- RenderBox::styleWillChange(diff, newStyle);
-}
-
void RenderTableRow::updateBeforeAndAfterContent()
{
if (!isAnonymous() && document()->usesBeforeAfterRules()) {
@@ -73,12 +63,17 @@
void RenderTableRow::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
{
+ ASSERT(style()->display() == TABLE_ROW);
+
RenderBox::styleDidChange(diff, oldStyle);
propagateStyleToAnonymousChildren();
if (parent())
updateBeforeAndAfterContent();
+ if (section() && oldStyle && style()->logicalHeight() != oldStyle->logicalHeight())
+ section()->rowLogicalHeightChanged(section()->rowIndexForRenderer(this));
+
// If border was changed, notify table.
if (parent()) {
RenderTable* table = this->table();
Modified: trunk/Source/WebCore/rendering/RenderTableRow.h (99211 => 99212)
--- trunk/Source/WebCore/rendering/RenderTableRow.h 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/Source/WebCore/rendering/RenderTableRow.h 2011-11-03 17:20:01 UTC (rev 99212)
@@ -63,7 +63,6 @@
virtual void imageChanged(WrappedImagePtr, const IntRect* = 0);
- virtual void styleWillChange(StyleDifference, const RenderStyle* newStyle);
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
RenderObjectChildList m_children;
Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (99211 => 99212)
--- trunk/Source/WebCore/rendering/RenderTableSection.cpp 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp 2011-11-03 17:20:01 UTC (rev 99212)
@@ -1158,6 +1158,11 @@
setNeedsLayout(true);
}
+void RenderTableSection::rowLogicalHeightChanged(unsigned rowIndex)
+{
+ setRowLogicalHeightToRowStyleLogicalHeightIfNotRelative(m_grid[rowIndex]);
+}
+
void RenderTableSection::setNeedsCellRecalc()
{
m_needsCellRecalc = true;
@@ -1300,4 +1305,14 @@
}
+unsigned RenderTableSection::rowIndexForRenderer(const RenderTableRow* row) const
+{
+ for (size_t i = 0; i < m_grid.size(); ++i) {
+ if (m_grid[i].rowRenderer == row)
+ return i;
+ }
+ ASSERT_NOT_REACHED();
+ return 0;
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderTableSection.h (99211 => 99212)
--- trunk/Source/WebCore/rendering/RenderTableSection.h 2011-11-03 17:16:58 UTC (rev 99211)
+++ trunk/Source/WebCore/rendering/RenderTableSection.h 2011-11-03 17:20:01 UTC (rev 99212)
@@ -118,6 +118,10 @@
LayoutUnit getBaseline(int row) { return m_grid[row].baseline; }
+ void rowLogicalHeightChanged(unsigned rowIndex);
+
+ unsigned rowIndexForRenderer(const RenderTableRow*) const;
+
protected:
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
