Title: [156355] trunk/Source/WebCore
Revision
156355
Author
an...@apple.com
Date
2013-09-24 13:47:01 -0700 (Tue, 24 Sep 2013)

Log Message

Tighten table rendering code
https://bugs.webkit.org/show_bug.cgi?id=121860

Reviewed by Andreas Kling.

Hide firstChild/lastChild/nextSibling/previousSibling in table renderers, 
expose correctly typed firstRow/nextCell etc instead.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (156354 => 156355)


--- trunk/Source/WebCore/ChangeLog	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/ChangeLog	2013-09-24 20:47:01 UTC (rev 156355)
@@ -1,3 +1,13 @@
+2013-09-24  Antti Koivisto  <an...@apple.com>
+
+        Tighten table rendering code
+        https://bugs.webkit.org/show_bug.cgi?id=121860
+
+        Reviewed by Andreas Kling.
+
+        Hide firstChild/lastChild/nextSibling/previousSibling in table renderers, 
+        expose correctly typed firstRow/nextCell etc instead.
+
 2013-09-24  Dean Jackson  <d...@apple.com>
 
         Implement symbol name hashing for WebGL shaders

Modified: trunk/Source/WebCore/rendering/FixedTableLayout.cpp (156354 => 156355)


--- trunk/Source/WebCore/rendering/FixedTableLayout.cpp	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/rendering/FixedTableLayout.cpp	2013-09-24 20:47:01 UTC (rev 156355)
@@ -136,13 +136,8 @@
 
     unsigned currentColumn = 0;
 
-    RenderObject* firstRow = section->firstChild();
-    for (RenderObject* child = firstRow->firstChildSlow(); child; child = child->nextSibling()) {
-        if (!child->isTableCell())
-            continue;
-
-        RenderTableCell* cell = toRenderTableCell(child);
-
+    RenderTableRow* firstRow = section->firstRow();
+    for (RenderTableCell* cell = firstRow->firstCell(); cell; cell = cell->nextCell()) {
         Length logicalWidth = cell->styleOrColLogicalWidth();
         unsigned span = cell->colSpan();
         int fixedBorderBoxLogicalWidth = 0;

Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (156354 => 156355)


--- trunk/Source/WebCore/rendering/RenderTable.cpp	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp	2013-09-24 20:47:01 UTC (rev 156355)
@@ -186,7 +186,7 @@
     if (lastBox && lastBox->isAnonymous() && !isAfterContent(lastBox)) {
         RenderTableSection* section = toRenderTableSection(lastBox);
         if (beforeChild == section)
-            beforeChild = section->firstChild();
+            beforeChild = section->firstRow();
         section->addChild(child, beforeChild);
         return;
     }
@@ -569,14 +569,10 @@
     for (RenderObject* section = firstChild(); section; section = section->nextSibling()) {
         if (!section->isTableSection())
             continue;
-        for (RenderObject* row = toRenderTableSection(section)->firstChild(); row; row = row->nextSibling()) {
-            if (!row->isTableRow())
-                continue;
-            for (RenderObject* cell = toRenderTableRow(row)->firstChild(); cell; cell = cell->nextSibling()) {
-                if (!cell->isTableCell())
-                    continue;
-                ASSERT(toRenderTableCell(cell)->table() == this);
-                toRenderTableCell(cell)->collectBorderValues(m_collapsedBorders);
+        for (RenderTableRow* row = toRenderTableSection(section)->firstRow(); row; row = row->nextRow()) {
+            for (RenderTableCell* cell = row->firstCell(); cell; cell = cell->nextCell()) {
+                ASSERT(cell->table() == this);
+                cell->collectBorderValues(m_collapsedBorders);
             }
         }
     }

Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (156354 => 156355)


--- trunk/Source/WebCore/rendering/RenderTableCell.cpp	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp	2013-09-24 20:47:01 UTC (rev 156355)
@@ -739,7 +739,7 @@
         if (prevCell->section() == section())
             prevRow = parent()->previousSibling();
         else
-            prevRow = prevCell->section()->lastChild();
+            prevRow = prevCell->section()->lastRow();
     
         if (prevRow) {
             result = chooseBorder(CollapsedBorderValue(prevRow->style()->borderAfter(), includeColor ? prevRow->style()->visitedDependentColor(afterColorProperty) : Color(), BROW), result);

Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (156354 => 156355)


--- trunk/Source/WebCore/rendering/RenderTableCell.h	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h	2013-09-24 20:47:01 UTC (rev 156355)
@@ -70,6 +70,9 @@
         return m_column;
     }
 
+    RenderTableCell* nextCell() const;
+    RenderTableCell* previousCell() const;
+
     RenderTableRow* row() const { return toRenderTableRow(parent()); }
     RenderTableSection* section() const { return toRenderTableSection(parent()->parent()); }
     RenderTable* table() const { return toRenderTable(parent()->parent()->parent()); }
@@ -207,6 +210,8 @@
         return style()->borderEnd();
     }
 
+    using RenderBlockFlow::nodeAtPoint;
+
 #ifndef NDEBUG
     bool isFirstOrLastCellInRow() const
     {
@@ -279,6 +284,9 @@
     unsigned parseRowSpanFromDOM() const;
     unsigned parseColSpanFromDOM() const;
 
+    void nextSibling() const WTF_DELETED_FUNCTION;
+    void previousSibling() const WTF_DELETED_FUNCTION;
+
     // Note MSVC will only pack members if they have identical types, hence we use unsigned instead of bool here.
     unsigned m_column : 29;
     unsigned m_cellWidthChanged : 1;
@@ -303,6 +311,26 @@
 // This will catch anyone doing an unnecessary cast.
 void toRenderTableCell(const RenderTableCell*);
 
+inline RenderTableCell* RenderTableCell::nextCell() const
+{
+    return toRenderTableCell(RenderBlockFlow::nextSibling());
+}
+
+inline RenderTableCell* RenderTableCell::previousCell() const
+{
+    return toRenderTableCell(RenderBlockFlow::previousSibling());
+}
+
+inline RenderTableCell* RenderTableRow::firstCell() const
+{
+    return toRenderTableCell(RenderBox::firstChild());
+}
+
+inline RenderTableCell* RenderTableRow::lastCell() const
+{
+    return toRenderTableCell(RenderBox::lastChild());
+}
+
 } // namespace WebCore
 
 #endif // RenderTableCell_h

Modified: trunk/Source/WebCore/rendering/RenderTableRow.cpp (156354 => 156355)


--- trunk/Source/WebCore/rendering/RenderTableRow.cpp	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/rendering/RenderTableRow.cpp	2013-09-24 20:47:01 UTC (rev 156355)
@@ -109,7 +109,7 @@
     if (!child->isTableCell()) {
         RenderObject* last = beforeChild;
         if (!last)
-            last = lastChild();
+            last = lastCell();
         if (last && last->isAnonymous() && last->isTableCell() && !last->isBeforeOrAfterContent()) {
             RenderTableCell* cell = toRenderTableCell(last);
             if (beforeChild == cell)
@@ -150,7 +150,7 @@
     ASSERT(!beforeChild || beforeChild->isTableCell());
     RenderBox::addChild(cell, beforeChild);
 
-    if (beforeChild || nextSibling())
+    if (beforeChild || nextRow())
         section()->setNeedsCellRecalc();
 }
 
@@ -164,16 +164,13 @@
 
     bool paginated = view().layoutState()->isPaginated();
                 
-    for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
-        if (child->isTableCell()) {
-            RenderTableCell* cell = toRenderTableCell(child);
-            if (!cell->needsLayout() && paginated && view().layoutState()->pageLogicalHeight() && view().layoutState()->pageLogicalOffset(cell, cell->logicalTop()) != cell->pageLogicalOffset())
-                cell->setChildNeedsLayout(true, MarkOnlyThis);
+    for (RenderTableCell* cell = firstCell(); cell; cell = cell->nextCell()) {
+        if (!cell->needsLayout() && paginated && view().layoutState()->pageLogicalHeight() && view().layoutState()->pageLogicalOffset(cell, cell->logicalTop()) != cell->pageLogicalOffset())
+            cell->setChildNeedsLayout(true, MarkOnlyThis);
 
-            if (child->needsLayout()) {
-                cell->computeAndSetBlockDirectionMargins(table());
-                cell->layout();
-            }
+        if (cell->needsLayout()) {
+            cell->computeAndSetBlockDirectionMargins(table());
+            cell->layout();
         }
     }
 
@@ -183,10 +180,8 @@
     // We cannot call repaint() because our clippedOverflowRectForRepaint() is taken from the
     // parent table, and being mid-layout, that is invalid. Instead, we repaint our cells.
     if (selfNeedsLayout() && checkForRepaintDuringLayout()) {
-        for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
-            if (child->isTableCell())
-                child->repaint();
-        }
+        for (RenderTableCell* cell = firstCell(); cell; cell = cell->nextCell())
+            cell->repaint();
     }
 
     statePusher.pop();
@@ -216,14 +211,14 @@
 {
     // Table rows cannot ever be hit tested.  Effectively they do not exist.
     // Just forward to our children always.
-    for (RenderObject* child = lastChild(); child; child = child->previousSibling()) {
+    for (RenderTableCell* cell = lastCell(); cell; cell = cell->previousCell()) {
         // FIXME: We have to skip over inline flows, since they can show up inside table rows
         // at the moment (a demoted inline <form> for example). If we ever implement a
         // table-specific hit-test method (which we should do for performance reasons anyway),
         // then we can remove this check.
-        if (child->isTableCell() && !toRenderBox(child)->hasSelfPaintingLayer()) {
-            LayoutPoint cellPoint = flipForWritingModeForChild(toRenderTableCell(child), accumulatedOffset);
-            if (child->nodeAtPoint(request, result, locationInContainer, cellPoint, action)) {
+        if (cell->hasSelfPaintingLayer()) {
+            LayoutPoint cellPoint = flipForWritingModeForChild(cell, accumulatedOffset);
+            if (cell->nodeAtPoint(request, result, locationInContainer, cellPoint, action)) {
                 updateHitTestResult(result, locationInContainer.point() - toLayoutSize(cellPoint));
                 return true;
             }
@@ -246,16 +241,12 @@
     ASSERT(hasSelfPaintingLayer());
 
     paintOutlineForRowIfNeeded(paintInfo, paintOffset);
-    for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
-        if (child->isTableCell()) {
-            // Paint the row background behind the cell.
-            if (paintInfo.phase == PaintPhaseBlockBackground || paintInfo.phase == PaintPhaseChildBlockBackground) {
-                RenderTableCell* cell = toRenderTableCell(child);
-                cell->paintBackgroundsBehindCell(paintInfo, paintOffset, this);
-            }
-            if (!toRenderBox(child)->hasSelfPaintingLayer())
-                child->paint(paintInfo, paintOffset);
-        }
+    for (RenderTableCell* cell = firstCell(); cell; cell = cell->nextCell()) {
+        // Paint the row background behind the cell.
+        if (paintInfo.phase == PaintPhaseBlockBackground || paintInfo.phase == PaintPhaseChildBlockBackground)
+            cell->paintBackgroundsBehindCell(paintInfo, paintOffset, this);
+        if (!cell->hasSelfPaintingLayer())
+            cell->paint(paintInfo, paintOffset);
     }
 }
 

Modified: trunk/Source/WebCore/rendering/RenderTableRow.h (156354 => 156355)


--- trunk/Source/WebCore/rendering/RenderTableRow.h	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/rendering/RenderTableRow.h	2013-09-24 20:47:01 UTC (rev 156355)
@@ -36,6 +36,12 @@
 public:
     explicit RenderTableRow(Element*);
 
+    RenderTableRow* nextRow() const;
+    RenderTableRow* previousRow() const;
+
+    RenderTableCell* firstCell() const;
+    RenderTableCell* lastCell() const;
+
     RenderTableSection* section() const { return toRenderTableSection(parent()); }
     RenderTable* table() const { return toRenderTable(parent()->parent()); }
 
@@ -84,6 +90,8 @@
 
     virtual void addChild(RenderObject* child, RenderObject* beforeChild = 0) OVERRIDE;
 
+    virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) OVERRIDE;
+
 private:
     virtual const char* renderName() const OVERRIDE { return (isAnonymous() || isPseudoElement()) ? "RenderTableRow (anonymous)" : "RenderTableRow"; }
 
@@ -94,7 +102,6 @@
 
     virtual void layout() OVERRIDE;
     virtual LayoutRect clippedOverflowRectForRepaint(const RenderLayerModelObject* repaintContainer) const OVERRIDE;
-    virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction) OVERRIDE;
 
     virtual bool requiresLayer() const OVERRIDE { return hasOverflowClip() || hasTransform() || hasHiddenBackface() || hasClipPath() || createsGroup(); }
 
@@ -104,6 +111,11 @@
 
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) OVERRIDE;
 
+    void firstChild() const WTF_DELETED_FUNCTION;
+    void lastChild() const WTF_DELETED_FUNCTION;
+    void nextSibling() const WTF_DELETED_FUNCTION;
+    void previousSibling() const WTF_DELETED_FUNCTION;
+
     unsigned m_rowIndex : 31;
 };
 
@@ -122,6 +134,26 @@
 // This will catch anyone doing an unnecessary cast.
 void toRenderTableRow(const RenderTableRow*);
 
+inline RenderTableRow* RenderTableSection::firstRow() const
+{
+    return toRenderTableRow(RenderBox::firstChild());
+}
+
+inline RenderTableRow* RenderTableSection::lastRow() const
+{
+    return toRenderTableRow(RenderBox::lastChild());
+}
+
+inline RenderTableRow* RenderTableRow::nextRow() const
+{
+    return toRenderTableRow(RenderBox::nextSibling());
+}
+
+inline RenderTableRow* RenderTableRow::previousRow() const
+{
+    return toRenderTableRow(RenderBox::previousSibling());
+}
+
 } // namespace WebCore
 
 #endif // RenderTableRow_h

Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (156354 => 156355)


--- trunk/Source/WebCore/rendering/RenderTableSection.cpp	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp	2013-09-24 20:47:01 UTC (rev 156355)
@@ -128,11 +128,11 @@
     if (!child->isTableRow()) {
         RenderObject* last = beforeChild;
         if (!last)
-            last = lastChild();
+            last = lastRow();
         if (last && last->isAnonymous() && !last->isBeforeOrAfterContent()) {
             RenderTableRow* row = toRenderTableRow(last);
             if (beforeChild == row)
-                beforeChild = row->firstChild();
+                beforeChild = row->firstCell();
             row->addChild(child, beforeChild);
             return;
         }
@@ -719,7 +719,7 @@
     if (sb.style() > BHIDDEN)
         borderWidth = sb.width();
 
-    const BorderValue& rb = firstChild()->style()->borderBefore();
+    const BorderValue& rb = firstRow()->style()->borderBefore();
     if (rb.style() == BHIDDEN)
         return -1;
     if (rb.style() > BHIDDEN && rb.width() > borderWidth)
@@ -770,7 +770,7 @@
     if (sb.style() > BHIDDEN)
         borderWidth = sb.width();
 
-    const BorderValue& rb = lastChild()->style()->borderAfter();
+    const BorderValue& rb = lastRow()->style()->borderAfter();
     if (rb.style() == BHIDDEN)
         return -1;
     if (rb.style() > BHIDDEN && rb.width() > borderWidth)
@@ -1223,26 +1223,18 @@
     m_cRow = 0;
     m_grid.clear();
 
-    for (RenderObject* row = firstChild(); row; row = row->nextSibling()) {
-        if (row->isTableRow()) {
-            unsigned insertionRow = m_cRow;
-            m_cRow++;
-            m_cCol = 0;
-            ensureRows(m_cRow);
+    for (RenderTableRow* row = firstRow(); row; row = row->nextRow()) {
+        unsigned insertionRow = m_cRow;
+        m_cRow++;
+        m_cCol = 0;
+        ensureRows(m_cRow);
 
-            RenderTableRow* tableRow = toRenderTableRow(row);
-            m_grid[insertionRow].rowRenderer = tableRow;
-            tableRow->setRowIndex(insertionRow);
-            setRowLogicalHeightToRowStyleLogicalHeightIfNotRelative(m_grid[insertionRow]);
+        m_grid[insertionRow].rowRenderer = row;
+        row->setRowIndex(insertionRow);
+        setRowLogicalHeightToRowStyleLogicalHeightIfNotRelative(m_grid[insertionRow]);
 
-            for (RenderObject* cell = tableRow->firstChild(); cell; cell = cell->nextSibling()) {
-                if (!cell->isTableCell())
-                    continue;
-
-                RenderTableCell* tableCell = toRenderTableCell(cell);
-                addCell(tableCell, tableRow);
-            }
-        }
+        for (RenderTableCell* cell = row->firstCell(); cell; cell = cell->nextCell())
+            addCell(cell, row);
     }
 
     m_grid.shrinkToFit();
@@ -1257,12 +1249,8 @@
 
     setRowLogicalHeightToRowStyleLogicalHeightIfNotRelative(m_grid[rowIndex]);
 
-    for (RenderObject* cell = m_grid[rowIndex].rowRenderer->firstChild(); cell; cell = cell->nextSibling()) {
-        if (!cell->isTableCell())
-            continue;
-
-        updateLogicalHeightForCell(m_grid[rowIndex], toRenderTableCell(cell));
-    }
+    for (RenderTableCell* cell = m_grid[rowIndex].rowRenderer->firstCell(); cell; cell = cell->nextCell())
+        updateLogicalHeightForCell(m_grid[rowIndex], cell);
 }
 
 void RenderTableSection::setNeedsCellRecalc()
@@ -1348,7 +1336,7 @@
 bool RenderTableSection::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction action)
 {
     // If we have no children then we have nothing to do.
-    if (!firstChild())
+    if (!firstRow())
         return false;
 
     // Table sections cannot ever be hit tested.  Effectively they do not exist.
@@ -1359,14 +1347,14 @@
         return false;
 
     if (hasOverflowingCell()) {
-        for (RenderObject* child = lastChild(); child; child = child->previousSibling()) {
+        for (RenderTableRow* row = lastRow(); row; row = row->previousRow()) {
             // FIXME: We have to skip over inline flows, since they can show up inside table rows
             // at the moment (a demoted inline <form> for example). If we ever implement a
             // table-specific hit-test method (which we should do for performance reasons anyway),
             // then we can remove this check.
-            if (child->isBox() && !toRenderBox(child)->hasSelfPaintingLayer()) {
-                LayoutPoint childPoint = flipForWritingModeForChild(toRenderBox(child), adjustedLocation);
-                if (child->nodeAtPoint(request, result, locationInContainer, childPoint, action)) {
+            if (row->hasSelfPaintingLayer()) {
+                LayoutPoint childPoint = flipForWritingModeForChild(row, adjustedLocation);
+                if (row->nodeAtPoint(request, result, locationInContainer, childPoint, action)) {
                     updateHitTestResult(result, toLayoutPoint(locationInContainer.point() - childPoint));
                     return true;
                 }

Modified: trunk/Source/WebCore/rendering/RenderTableSection.h (156354 => 156355)


--- trunk/Source/WebCore/rendering/RenderTableSection.h	2013-09-24 20:41:34 UTC (rev 156354)
+++ trunk/Source/WebCore/rendering/RenderTableSection.h	2013-09-24 20:47:01 UTC (rev 156355)
@@ -30,6 +30,8 @@
 
 namespace WebCore {
 
+class RenderTableRow;
+
 enum CollapsedBorderSide {
     CBSBefore,
     CBSAfter,
@@ -65,6 +67,9 @@
     explicit RenderTableSection(Element*);
     virtual ~RenderTableSection();
 
+    RenderTableRow* firstRow() const;
+    RenderTableRow* lastRow() const;
+
     virtual void addChild(RenderObject* child, RenderObject* beforeChild = 0) OVERRIDE;
 
     virtual int firstLineBoxBaseline() const OVERRIDE;
@@ -238,6 +243,9 @@
 
     void setLogicalPositionForCell(RenderTableCell*, unsigned effectiveColumn) const;
 
+    void firstChild() const WTF_DELETED_FUNCTION;
+    void lastChild() const WTF_DELETED_FUNCTION;
+
     Vector<RowStruct> m_grid;
     Vector<int> m_rowPos;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to