Crash when opening web page http://build.webkit.org/waterfall.

Source/core/rendering/RenderTableSection.cpp

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 354 matching lines @@
         m_rowPos[row + 1] += accumulatedPositionIncrease;
     }
 
     ASSERT(!remainder);
 
     extraRowSpanningHeight -= accumulatedPositionIncrease;
 }
 
 // To avoid unneeded extra height distributions, we apply the following sorting algorithm:
 // 1. We sort by increasing start row but decreasing last row (ie the top-most, shortest cells first).
 // 2. For cells spanning the same rows, we sort by intrinsic size.

[Julien - ping for review, 2013/09/04 20:46:27]

As you inlined the description of what you do, the description of how we sort
could be removed from this comment.

[a.suchit, 2013/09/05 06:35:01]

Done.

-static bool compareRowSpanCellsInHeightDistributionOrder(const RenderTableCell* cell2, const RenderTableCell* cell1)
+static bool compareRowSpanCellsInHeightDistributionOrder(const RenderTableCell* cell1, const RenderTableCell* cell2)
 {
-    unsigned cellRowIndex1 = cell1->rowIndex();
-    unsigned cellRowSpan1 = cell1->rowSpan();
-    unsigned cellRowIndex2 = cell2->rowIndex();
-    unsigned cellRowSpan2 = cell2->rowSpan();
+    unsigned cell1RowIndex = cell1->rowIndex();
+    unsigned cell1RowSpan = cell1->rowSpan();
+    unsigned cell2RowIndex = cell2->rowIndex();
+    unsigned cell2RowSpan = cell2->rowSpan();
 
-    if (cellRowIndex1 == cellRowIndex2 && cellRowSpan1 == cellRowSpan2)
-        return (cell2->logicalHeightForRowSizing() > cell1->logicalHeightForRowSizing());
+    // Sorting bigger height cell first if cells are at same index with same span because we will skip smaller
+    // height cell to distribute it's extra height.
+    if (cell1RowIndex == cell2RowIndex && cell1RowSpan == cell2RowSpan)
+        return (cell1->logicalHeightForRowSizing() > cell2->logicalHeightForRowSizing());
+    // Sorting inner most cell first because if inner spanning cell'e extra height is distributed then outer
+    // spanning cell's extra height will adjust accordingly. In reverse order, there is more chances that outer
+    // spanning cell's height will exceed than defined by user.
+    if (cell1RowIndex >= cell2RowIndex && (cell1RowIndex + cell1RowSpan) <= (cell2RowIndex + cell2RowSpan))

[Julien - ping for review, 2013/09/04 18:35:45]

This should be a function as you use it twice:

bool cellIsFullyIncludedInOtherCell(const RenderTableCell*, cosnt
RenderTableCell*)

[a.suchit, 2013/09/05 06:35:01]

Done.

+        return true;
+    // Sorting lower row index first because first we need to apply the extra height of spanning cell which
+    // comes first in the table so lower rows's position would increment in sequence.
+    if (!(cell2RowIndex >= cell1RowIndex && (cell2RowIndex + cell2RowSpan) <= (cell1RowIndex + cell1RowSpan)))
+        return (cell1RowIndex < cell2RowIndex);
 
-    return (cellRowIndex2 >= cellRowIndex1 && (cellRowIndex2 + cellRowSpan2) <= (cellRowIndex1 + cellRowSpan1));
+    return false;

[Julien - ping for review, 2013/09/04 18:35:45]

I am concerned about always returning false here but I couldn't find a violation
of the weak ordering properties.

 }
 
 // Distribute rowSpan cell height in rows those comes in rowSpan cell based on the ratio of row's height if
 // 1. RowSpan cell height is greater then the total height of rows in rowSpan cell
 void RenderTableSection::distributeRowSpanHeightToRows(SpanningRenderTableCells& rowSpanCells)
 {
     ASSERT(rowSpanCells.size());
 
     // 'rowSpanCells' list is already sorted based on the cells rowIndex in ascending order
     // Arrange row spanning cell in the order in which we need to process first.
@@ skipping 1286 matching lines @@
     if (!style()->isLeftToRightDirection())
         cellLocation.setX(table()->columnPositions()[table()->numEffCols()] - table()->columnPositions()[table()->colToEffCol(cell->col() + cell->colSpan())] + horizontalBorderSpacing);
     else
         cellLocation.setX(table()->columnPositions()[effectiveColumn] + horizontalBorderSpacing);
 
     cell->setLogicalLocation(cellLocation);
     view()->addLayoutDelta(oldCellLocation - cell->location());
 }
 
 } // namespace WebCore