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

Source/core/layout/TableLayoutAlgorithmAuto.cpp

 /*
  * Copyright (C) 2002 Lars Knoll (knoll@kde.org)
  *           (C) 2002 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2003, 2006, 2008, 2010 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License.
  *
@@ skipping 572 matching lines @@
     if (available > 0) {
         for (size_t i = 0; i < nEffCols; ++i) {
             Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
             if (logicalWidth.isFixed() && logicalWidth.value() > m_layoutStruct[i].computedLogicalWidth) {
                 available += m_layoutStruct[i].computedLogicalWidth - logicalWidth.value();
                 m_layoutStruct[i].computedLogicalWidth = logicalWidth.value();
             }
         }
     }
 
-    // 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) {
         m_table->setColumnPosition(i, pos);
         pos += m_layoutStruct[i].computedLogicalWidth + m_table->hBorderSpacing();
     }
     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;
     for (unsigned i = nEffCols; i; ) {
         --i;
         Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
         if (logicalWidth.type() == lengthType)
             logicalWidthBeyondMin += m_layoutStruct[i].computedLogicalWidth - m_layoutStruct[i].effectiveMinLogicalWidth;
     }
 
     for (unsigned i = nEffCols; i && logicalWidthBeyondMin > 0; ) {
         --i;
         Length& logicalWidth = m_layoutStruct[i].effectiveLogicalWidth;
         if (logicalWidth.type() == lengthType) {
             int minMaxDiff = m_layoutStruct[i].computedLogicalWidth - m_layoutStruct[i].effectiveMinLogicalWidth;
             int reduce = available * minMaxDiff / logicalWidthBeyondMin;
             m_layoutStruct[i].computedLogicalWidth += reduce;
             available -= reduce;
             logicalWidthBeyondMin -= minMaxDiff;
             if (available >= 0)
                 break;
         }
     }
 }
 }