Implement justify-self handling in grid

Source/core/rendering/RenderGrid.cpp

Index: Source/core/rendering/RenderGrid.cpp
diff --git a/Source/core/rendering/RenderGrid.cpp b/Source/core/rendering/RenderGrid.cpp
index 36fb8e077ab2fdc938b484214339807b3d9f0225..75035bf25cc575ae87e89d6b44f69e77516f47b0 100644
--- a/Source/core/rendering/RenderGrid.cpp
+++ b/Source/core/rendering/RenderGrid.cpp
@@ -1089,8 +1089,99 @@ LayoutPoint RenderGrid::findChildLogicalPosition(RenderBox* child, const GridSiz
     ASSERT(coordinate.columns.initialPositionIndex < sizingData.columnTracks.size());
     ASSERT(coordinate.rows.initialPositionIndex < sizingData.rowTracks.size());
 
+    LayoutUnit startOfColumn = m_columnPositions[coordinate.columns.initialPositionIndex];
+    LayoutUnit startOfRow = m_rowPositions[coordinate.rows.initialPositionIndex];
     // The grid items should be inside the grid container's border box, that's why they need to be shifted.
-    return LayoutPoint(m_columnPositions[coordinate.columns.initialPositionIndex] + marginStartForChild(child), m_rowPositions[coordinate.rows.initialPositionIndex] + marginBeforeForChild(child));
+    LayoutUnit columnPosition = startOfColumn + marginStartForChild(child);
+    LayoutUnit rowPosition = startOfRow + marginBeforeForChild(child);
+
+    ItemPosition childJustifySelf = child->style()->justifySelf();

[ojan, 2014/01/22 23:43:14]

Maybe move this and the switch statement below into a helper function? Then you
don't need the comment explaining what this switch does since the function name
will make that clear.

+    // This first switch pre-processes the justify-self value following the rules in css-align. It also resolves
+    // all the position into the grid container's logical coordinate for easier computation (and code sharing).
+    switch (childJustifySelf) {
+    case ItemPositionStretch:
+    case ItemPositionBaseline:
+    case ItemPositionCenter:
+    case ItemPositionStart:
+    case ItemPositionEnd:
+        // Nothing to do.
+        break;
+
+    case ItemPositionAuto:
+        // FIXME: We should use 'justify-items' value and convert any auto value here. For now, we ignore it.
+        break;
+    case ItemPositionSelfStart:
+        if (child->style()->direction() != style()->direction())
+            childJustifySelf = ItemPositionEnd;
+        else
+            childJustifySelf = ItemPositionStart;
+        break;
+    case ItemPositionSelfEnd:
+        if (child->style()->direction() != style()->direction())
+            childJustifySelf = ItemPositionStart;
+        else
+            childJustifySelf = ItemPositionEnd;
+        break;
+    case ItemPositionFlexStart:
+    case ItemPositionFlexEnd:

[ojan, 2014/01/22 23:43:14]

It's weird the flex-end is equivalent to start. I'd expect it to be equivalent
to end.

+        // Only used in flex layout, for other layout, it's equivalent to 'start'.
+        childJustifySelf = ItemPositionStart;
+        break;
+

[ojan, 2014/01/22 23:43:14]

Are these line-breaks intentional?

+    case ItemPositionLeft:
+        // If the property's axis is not parallel with the inline axis, this is equivalent to ‘start’.
+        if (!isHorizontalWritingMode())
+            childJustifySelf = ItemPositionStart;
+        else
+            childJustifySelf = style()->isLeftToRightDirection() ? ItemPositionStart : ItemPositionEnd;
+        break;
+    case ItemPositionRight:
+        // If the property's axis is not parallel with the inline axis, this is equivalent to ‘start’.
+        if (!isHorizontalWritingMode())
+            childJustifySelf = ItemPositionStart;
+        else
+            childJustifySelf = style()->isLeftToRightDirection() ? ItemPositionEnd : ItemPositionStart;
+        break;
+    }

[Julien - ping for review, 2014/01/22 19:13:42]

I am not happy with this 2-pass code. I will come up with a better way but in
the meantime, suggestions are very welcome.

[ojan, 2014/01/22 23:43:14]

Seems fine to me if you move the first pass into a helper function.

+
+    switch (childJustifySelf) {
+    case ItemPositionCenter: {
+        LayoutUnit endOfColumn = m_columnPositions[coordinate.columns.finalPositionIndex + 1];
+        columnPosition += std::max<LayoutUnit>(0, endOfColumn - startOfColumn - child->logicalWidth()) / 2;
+        break;
+    }
+    case ItemPositionLeft:
+    case ItemPositionRight:
+    case ItemPositionFlexStart:
+    case ItemPositionFlexEnd:
+    case ItemPositionSelfStart:
+    case ItemPositionSelfEnd:
+        ASSERT_NOT_REACHED();
+        break;
+    case ItemPositionStart:
+        // Move the item to account for the column's direction.

[ojan, 2014/01/22 23:43:14]

Meh. this comment and the one below don't really tell me anything useful.

+        if (!style()->isLeftToRightDirection()) {
+            LayoutUnit endOfColumn = m_columnPositions[coordinate.columns.finalPositionIndex + 1];
+            columnPosition += std::max<LayoutUnit>(0, endOfColumn - startOfColumn - child->logicalWidth());
+        }
+        break;
+    case ItemPositionEnd:
+        // Move the item to account for the column's direction.
+        if (style()->isLeftToRightDirection()) {
+            LayoutUnit endOfColumn = m_columnPositions[coordinate.columns.finalPositionIndex + 1];
+            columnPosition += std::max<LayoutUnit>(0, endOfColumn - startOfColumn - child->logicalWidth());
+        }
+        break;
+    case ItemPositionStretch:
+        // FIXME: Handle this value.
+    case ItemPositionBaseline:
+        // FIXME: This is blocked on implementing the grid container's baseline.
+    case ItemPositionAuto:
+        // FIXME: We should never see this value (per the comment above).

[ojan, 2014/01/22 23:43:14]

Per which comment?

+        break;
+    }
+
+    return LayoutPoint(columnPosition, rowPosition);
 }
 
 static GridSpan dirtiedGridAreas(const Vector<LayoutUnit>& coordinates, LayoutUnit start, LayoutUnit end)