Implement justify-self handling in grid

Implement justify-self handling in grid

This is a partial but good enough support for 'justify-self'.
The remaining cases (auto, baseline and stretch) require more
thoughts and/or other patches.

BUG=249451
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=166002

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

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderGrid.cpp:1145: }
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:13]

lgtm

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderGrid.cpp:1098: ItemPosition childJustifySelf =
child->style()->justifySelf();
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.

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderGrid.cpp:1126: case ItemPositionFlexEnd:
It's weird the flex-end is equivalent to start. I'd expect it to be equivalent
to end.

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderGrid.cpp:1130: 
Are these line-breaks intentional?

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderGrid.cpp:1145: }
On 2014/01/22 19:13:42, Julien Chaffraix - PST wrote:
> 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.

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

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderGrid.cpp:1162: // Move the item to account for the
column's direction.
Meh. this comment and the one below don't really tell me anything useful.

https://codereview.chromium.org/144323002/diff/1/Source/core/rendering/Render...
Source/core/rendering/RenderGrid.cpp:1180: // FIXME: We should never see this
value (per the comment above).
Per which comment?

[ojan, 2014-01-28 21:38:05]

https://codereview.chromium.org/144323002/diff/80001/Source/core/rendering/Re...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/144323002/diff/80001/Source/core/rendering/Re...
Source/core/rendering/RenderGrid.cpp:1106: LayoutUnit
RenderGrid::columnPositionAlignedWithGridContainerEnd(const RenderBox* child)
const
Am I just misreading the code or is the exactly the same as
columnPositionAlignedWithGridContainerStart?

[ojan, 2014-01-28 23:13:07]

lgtm

https://codereview.chromium.org/144323002/diff/80001/Source/core/rendering/Re...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/144323002/diff/80001/Source/core/rendering/Re...
Source/core/rendering/RenderGrid.cpp:1106: LayoutUnit
RenderGrid::columnPositionAlignedWithGridContainerEnd(const RenderBox* child)
const
On 2014/01/28 21:38:05, ojan wrote:
> Am I just misreading the code or is the exactly the same as
> columnPositionAlignedWithGridContainerStart?

Oh, I missed the !. You could avoid the code duplication by having a single
method that takes an enum as to whether it's aligning to the start of the end.

[Julien - ping for review, 2014-01-29 00:11:56]

https://codereview.chromium.org/144323002/diff/80001/Source/core/rendering/Re...
File Source/core/rendering/RenderGrid.cpp (right):

https://codereview.chromium.org/144323002/diff/80001/Source/core/rendering/Re...
Source/core/rendering/RenderGrid.cpp:1106: LayoutUnit
RenderGrid::columnPositionAlignedWithGridContainerEnd(const RenderBox* child)
const
On 2014/01/28 23:13:07, ojan wrote:
> On 2014/01/28 21:38:05, ojan wrote:
> > Am I just misreading the code or is the exactly the same as
> > columnPositionAlignedWithGridContainerStart?
> 
> Oh, I missed the !. You could avoid the code duplication by having a single
> method that takes an enum as to whether it's aligning to the start of the end.

That's a good catch. I think it makes more sense to keep the explicit cases and
have a method for each direction.

I think that matches more closely what the rest of the code does for handling
writing-mode or direction.

[commit-bot: I haz the power, 2014-01-29 01:19:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/144323002/130001

[commit-bot: I haz the power, 2014-01-29 06:14:02]

Change committed as 166002

[commit-bot: I haz the power, 2014-01-29 06:14:09]

CQ bit was unchecked on CL. Ignoring.