Crashes when percent calculation of row's height goes very high.

third_party/WebKit/Source/core/layout/LayoutTableSection.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, 2013 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 285 matching lines @@
         if (!spanningRowsHeight.rowHeight[row])
             spanningRowsHeight.isAnyRowWithOnlySpanningCells |= rowHasOnlySpanningCells(actualRow);
 
         spanningRowsHeight.totalRowsHeight += spanningRowsHeight.rowHeight[row];
         spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing -= borderSpacingForRow(actualRow);
     }
     // We don't span the following row so its border-spacing (if any) should be included.
     spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing += borderSpacingForRow(rowIndex + rowSpan - 1);
 }
 
-void LayoutTableSection::distributeExtraRowSpanHeightToPercentRows(LayoutTableCell* cell, int totalPercent, int& extraRowSpanningHeight, Vector<int>& rowsHeight)
+void LayoutTableSection::distributeExtraRowSpanHeightToPercentRows(
+    LayoutTableCell* cell, float totalPercent, int& extraRowSpanningHeight,

[mstensho (USE GERRIT), 2015/11/02 10:46:42]

Please put this back on one line.

[a.suchit2, 2016/01/12 06:46:35]

Done.

+    Vector<int>& rowsHeight)
 {
     if (!extraRowSpanningHeight || !totalPercent)
         return;
 
     const unsigned rowSpan = cell->rowSpan();
     const unsigned rowIndex = cell->rowIndex();
-    int percent = std::min(totalPercent, 100);
+    float percent = std::min(totalPercent, 100.0f);
     const int tableHeight = m_rowPos[m_grid.size()] + extraRowSpanningHeight;
 
     // Our algorithm matches Firefox. Extra spanning height would be distributed Only in first percent height rows
     // those total percent is 100. Other percent rows would be uneffected even extra spanning height is remain.
     int accumulatedPositionIncrease = 0;
     for (unsigned row = rowIndex; row < (rowIndex + rowSpan); row++) {
         if (percent > 0 && extraRowSpanningHeight > 0) {
             // TODO(alancutter): Make this work correctly for calc lengths.
             if (m_grid[row].logicalHeight.hasPercent()) {
-                int toAdd = (tableHeight * m_grid[row].logicalHeight.percent() / 100) - rowsHeight[row - rowIndex];
-                // FIXME: Note that this is wrong if we have a percentage above 100% and may make us grow
-                // above the available space.
+                int toAdd = (tableHeight *
+    std::min(m_grid[row].logicalHeight.percent(), percent) / 100)

[mstensho (USE GERRIT), 2015/11/02 10:46:42]

OK, so what you want to avoid here is integer overflow. How about using an
intermediary LayoutUnit here, which does saturated arithmetic? We certainly
don't want toAdd to be 0, which is the result of your changes.

I'm talking about changing toAdd from int to LayoutUnit.

Alternatively, change it from int to float.

[a.suchit2, 2016/01/12 06:46:35]

I am avoiding high value of percent which is not acceptable and trying to take
min. value which can be adjusted here.
In terms of the overflow, it may happen for int/float/double for all.
I did not get here that, how will LayoutUnit/float be helpful for changing toAdd
?

[mstensho (USE GERRIT), 2016/01/18 14:37:03]

Because LayoutUnit cannot overflow, since it uses saturated arithmetic.

The original code is:
int toAdd = (tableHeight * m_grid[row].logicalHeight.percent() / 100) -
rowsHeight[row - rowIndex];

Just wrapping a LayoutUnit around the right-hand side here will make the
assertion go away:

int toAdd = LayoutUnit((tableHeight * m_grid[row].logicalHeight.percent() / 100)
- rowsHeight[row - rowIndex]);

[a.suchit2, 2016/01/19 13:02:16]

Replied on latest patch set 5.

+    - rowsHeight[row - rowIndex];
 
                 toAdd = std::max(std::min(toAdd, extraRowSpanningHeight), 0);
                 accumulatedPositionIncrease += toAdd;
                 extraRowSpanningHeight -= toAdd;
                 percent -= m_grid[row].logicalHeight.percent();
             }
         }
         m_rowPos[row + 1] += accumulatedPositionIncrease;
     }
 }
@@ skipping 312 matching lines @@
             if (m_grid[row].logicalHeight.hasPercent()) {
                 totalPercent += m_grid[row].logicalHeight.percent();
                 totalRemainingRowsHeight -= spanningRowsHeight.rowHeight[row - rowIndex];
             } else if (m_grid[row].logicalHeight.isAuto()) {
                 totalAutoRowsHeight += spanningRowsHeight.rowHeight[row - rowIndex];
             }
         }
 
         int extraRowSpanningHeight = spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing - spanningRowsHeight.totalRowsHeight;
 
         if (totalPercent < 100 && !totalAutoRowsHeight && !totalRemainingRowsHeight) {

[a.suchit2, 2015/10/30 01:43:50]

Yes, when auto height and remaining height are 0 then divide by |totalPercent|
should be used.
So changes are required here. Condition should be :
if (totalPercent && !totalAutoRowsHeight && !totalRemainingRowsHeight) {

[mstensho (USE GERRIT), 2015/11/02 10:46:42]

I guess that's for a separate CL to sort out, right?

             // Distributing whole extra rowspanning height in percent row when only non-percent rows height is 0.
             distributeWholeExtraRowSpanHeightToPercentRows(cell, totalPercent, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
         } else {
             distributeExtraRowSpanHeightToPercentRows(cell, totalPercent, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
             distributeExtraRowSpanHeightToAutoRows(cell, totalAutoRowsHeight, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
             distributeExtraRowSpanHeightToRemainingRows(cell, totalRemainingRowsHeight, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
         }
 
         ASSERT(!extraRowSpanningHeight);
 
@@ skipping 975 matching lines @@
     // FIXME: The table's direction should determine our row's direction, not the section's (see bug 96691).
     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);
 }
 
 } // namespace blink