[css-grid] Small refactoring in LayoutGrid::layoutPositionedObjects()

[css-grid] Small refactoring in LayoutGrid::layoutPositionedObjects()

We were passing too many arguments to
LayoutGrid::offsetAndBreadthForPositionedChild().
It seems simpler to move some code there from
LayoutGrid::layoutPositionedObjects().

Committed: https://crrev.com/7c8c5489cd8c2367316d96a5a3d3a0d6d43831c4
Cr-Commit-Position: refs/heads/master@{#351056}

[Manuel Rego, 2015-09-24 10:32:05]

rego@igalia.com changed reviewers:
+ cbiesinger@chromium.org, jfernandez@igalia.com, svillar@igalia.com

[svillar, 2015-09-28 08:09:28]

lgtm

https://codereview.chromium.org/1362683003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):

https://codereview.chromium.org/1362683003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:1471: }
So now you're doing this before all the child->setXXX calls in
layoutPositionedObjects, are you 100% sure that the order does not matter?

They seem like simple setters but better double check that there are no hidden
side effects.

[Manuel Rego, 2015-09-28 09:49:09]

Thanks for the review.

https://codereview.chromium.org/1362683003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):

https://codereview.chromium.org/1362683003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:1471: }
On 2015/09/28 08:09:28, svillar wrote:
> So now you're doing this before all the child->setXXX calls in
> layoutPositionedObjects, are you 100% sure that the order does not matter?
> 
> They seem like simple setters but better double check that there are no hidden
> side effects.

Yeah, I wouldn't expect other effects as they're just setting different values.

[Manuel Rego, 2015-09-28 09:49:12]

The CQ bit was checked by rego@igalia.com

[commit-bot: I haz the power, 2015-09-28 09:49:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1362683003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1362683003/1

[commit-bot: I haz the power, 2015-09-28 11:18:25]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-09-28 11:19:42]

Patchset 1 (id:??) landed as
https://crrev.com/7c8c5489cd8c2367316d96a5a3d3a0d6d43831c4
Cr-Commit-Position: refs/heads/master@{#351056}

[cbiesinger, 2015-09-30 14:27:35]

lgtm

https://codereview.chromium.org/1362683003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):

https://codereview.chromium.org/1362683003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:1407: // FIXME: Detect
properly if start/end is auto for inexistent named grid lines.
This comment should probably move to offsetAndBreadthForPositionedChild? Or
rather be removed after https://codereview.chromium.org/1375173002/

[Manuel Rego, 2015-09-30 20:53:51]

https://chromiumcodereview.appspot.com/1362683003/diff/1/third_party/WebKit/S...
File third_party/WebKit/Source/core/layout/LayoutGrid.cpp (right):

https://chromiumcodereview.appspot.com/1362683003/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/core/layout/LayoutGrid.cpp:1407: // FIXME: Detect
properly if start/end is auto for inexistent named grid lines.
On 2015/09/30 14:27:35, cbiesinger wrote:
> This comment should probably move to offsetAndBreadthForPositionedChild? Or
> rather be removed after https://codereview.chromium.org/1375173002/

Yep, you're right, I forgot to move it in this patch.
I'm already removing it in the mentioned patch.