Refactor the code distributing width to columns in auto layout tables.

Source/core/layout/TableLayoutAlgorithmAuto.cpp

Index: Source/core/layout/TableLayoutAlgorithmAuto.cpp
diff --git a/Source/core/layout/TableLayoutAlgorithmAuto.cpp b/Source/core/layout/TableLayoutAlgorithmAuto.cpp
index d6dad14d1737a4bd1ae03338129b28002585d630..9f0757964a76a3bac431217f29403f4d402cd715 100644
--- a/Source/core/layout/TableLayoutAlgorithmAuto.cpp
+++ b/Source/core/layout/TableLayoutAlgorithmAuto.cpp
@@ -590,74 +590,31 @@ void TableLayoutAlgorithmAuto::layout()
         }
     }
 
-    // now satisfy variable

[mstensho (USE GERRIT), 2015/04/28 21:53:12]

I think I'd like to keep these comments.

     if (available > 0 && numAuto) {
-        available += allocAuto; // this gets redistributed
-        for (size_t i = 0; i < nEffCols; ++i) {
-            Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
-            if (logicalWidth.isAuto() && totalAuto && !m_layoutStruct[i].emptyCellsOnly) {
-                int cellLogicalWidth = std::max<int>(m_layoutStruct[i].computedLogicalWidth, static_cast<int>(available * static_cast<float>(m_layoutStruct[i].effectiveMaxLogicalWidth) / totalAuto));
-                available -= cellLogicalWidth;
-                totalAuto -= m_layoutStruct[i].effectiveMaxLogicalWidth;
-                m_layoutStruct[i].computedLogicalWidth = cellLogicalWidth;
-            }
-        }
+        available += allocAuto;
+        distributeWidthToColumns(Auto, available, totalAuto, NonEmptyCells, InitialWidth);
     }
 
-    // spread over fixed columns
-    if (available > 0 && numFixed) {
-        for (size_t i = 0; i < nEffCols; ++i) {
-            Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
-            if (logicalWidth.isFixed()) {
-                int cellLogicalWidth = static_cast<int>(available * static_cast<float>(m_layoutStruct[i].effectiveMaxLogicalWidth) / totalFixed);
-                available -= cellLogicalWidth;
-                totalFixed -= m_layoutStruct[i].effectiveMaxLogicalWidth;
-                m_layoutStruct[i].computedLogicalWidth += cellLogicalWidth;
-            }
-        }
-    }
+    if (available > 0 && numFixed)
+        distributeWidthToColumns(Fixed, available, totalFixed);
 
-    // spread over percent colums
-    if (available > 0 && m_hasPercent && totalPercent < 100) {
-        for (size_t i = 0; i < nEffCols; ++i) {
-            Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
-            if (logicalWidth.isPercent()) {
-                int cellLogicalWidth = available * logicalWidth.percent() / totalPercent;
-                available -= cellLogicalWidth;
-                totalPercent -= logicalWidth.percent();
-                m_layoutStruct[i].computedLogicalWidth += cellLogicalWidth;
-                if (!available || !totalPercent)
-                    break;
-            }
-        }
-    }
+    if (available > 0 && m_hasPercent && totalPercent < 100)
+        distributeWidthToColumns(Percent, available, totalPercent);
 
-    // spread over the rest
     if (available > 0 && nEffCols > numAutoEmptyCellsOnly) {
         unsigned total = nEffCols - numAutoEmptyCellsOnly;

[mstensho (USE GERRIT), 2015/04/28 21:53:13]

Could be signed int now.

-        // still have some width to spread
-        for (unsigned i = nEffCols; i; ) {
-            --i;
-            // variable columns with empty cells only don't get any width
-            if (m_layoutStruct[i].effectiveLogicalWidth.isAuto() && m_layoutStruct[i].emptyCellsOnly)
-                continue;
-            int cellLogicalWidth = available / total;
-            available -= cellLogicalWidth;
-            total--;
-            m_layoutStruct[i].computedLogicalWidth += cellLogicalWidth;
-        }
+        distributeWidthToColumns(Auto, available, total, NonEmptyCells, LeftOverWidth, EndToStart);
     }
 
     // If we have overallocated, reduce every cell according to the difference between desired width and minwidth
     // this seems to produce to the pixel exact results with IE. Wonder is some of this also holds for width distributing.
-    // Need to reduce cells with the following prioritization:
     // This is basically the reverse of how we grew the cells.
     if (available < 0)
-        shrinkCellWidth(Auto, available);
+        shrinkColumnWidth(Auto, available);
     if (available < 0)
-        shrinkCellWidth(Fixed, available);
+        shrinkColumnWidth(Fixed, available);
     if (available < 0)
-        shrinkCellWidth(Percent, available);
+        shrinkColumnWidth(Percent, available);
 
     int pos = 0;
     for (size_t i = 0; i < nEffCols; ++i) {
@@ -667,7 +624,38 @@ void TableLayoutAlgorithmAuto::layout()
     m_table->setColumnPosition(m_table->columnPositions().size() - 1, pos);
 }
 
-void TableLayoutAlgorithmAuto::shrinkCellWidth(const LengthType& lengthType, int& available)
+

[mstensho (USE GERRIT), 2015/04/28 21:53:13]

Extraneous blank line.

[mstensho (USE GERRIT), 2015/05/04 20:42:47]

Acknowledged.

+void TableLayoutAlgorithmAuto::distributeWidthToColumns(const LengthType& lengthType, int& available, int total, LimitToCells limitToCells, DistributionMode distributionMode, DistributionDirection distributionDirection)
+{
+    int nEffCols = static_cast<int>(m_table->numEffCols());
+    bool startToEnd = distributionDirection == StartToEnd;
+    for (int i = startToEnd ? 0 : nEffCols - 1; startToEnd ? i < nEffCols : i > -1; startToEnd ? ++i : --i) {
+        Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;

[mstensho (USE GERRIT), 2015/04/28 21:53:12]

Please make it a const reference.

[mstensho (USE GERRIT), 2015/05/04 20:42:47]

Acknowledged.

+        if (limitToCells == NonEmptyCells && logicalWidth.isAuto() && m_layoutStruct[i].emptyCellsOnly)
+            continue;
+        if (distributionMode != LeftOverWidth && logicalWidth.type() != lengthType)
+            continue;
+
+        int factor = 1;
+        if (distributionMode == LeftOverWidth)
+            factor = 1;

[mstensho (USE GERRIT), 2015/04/28 21:53:12]

It's already 1 at this point.

[mstensho (USE GERRIT), 2015/05/04 20:42:47]

Acknowledged.

+        else if (lengthType == Percent)
+            factor = logicalWidth.percent();

[mstensho (USE GERRIT), 2015/04/28 21:53:13]

Losing the fraction part of the value here, which would be a regression. Try
placing a column with 1% next to one with 1.5%. I guess factor needs to be
either float or LayoutUnit.

For even more fun, try a column with 0.1% next to one with 0.5%. Division by
zero - SIGFPE. :) That's thanks to |total| being int. Used to be float for
percentages.

[mstensho (USE GERRIT), 2015/05/04 20:42:47]

Not sure if using floats all over the place is right, either. That could affect
both precision and performance, couldn't it?

This entire method looks like it could benefit from a more template-based
approach. A lot of stuff can be determined at compile-time.

How about something like:

    template<typename T, LengthType, CellsToProcess, DistributionMode,
DistributionDirection>
    void distributeWidthToColumns(int& available, T total);

+        else if (lengthType == Auto || lengthType == Fixed)
+            factor = m_layoutStruct[i].effectiveMaxLogicalWidth;
+
+        int newWidth = available * factor / total;
+        int cellLogicalWidth = (distributionMode == InitialWidth) ? max<int>(m_layoutStruct[i].computedLogicalWidth, newWidth) : newWidth;
+        available -= cellLogicalWidth;
+        total -= factor;
+        m_layoutStruct[i].computedLogicalWidth = (distributionMode == InitialWidth) ? cellLogicalWidth : m_layoutStruct[i].computedLogicalWidth + cellLogicalWidth;
+
+        if (available <=0  || total <= 0)

[mstensho (USE GERRIT), 2015/04/28 21:53:12]

"<=0  ||" -> "<= 0 ||"

[mstensho (USE GERRIT), 2015/05/04 20:42:47]

Acknowledged.

+            return;
+    }
+}
+
+void TableLayoutAlgorithmAuto::shrinkColumnWidth(const LengthType& lengthType, int& available)
 {
     size_t nEffCols = m_table->numEffCols();
     int logicalWidthBeyondMin = 0;