[css-grid] Cleanup LayoutGrid::Grid

[css-grid] Cleanup LayoutGrid::Grid

This is a cleanup of some recent additions to the Grid class, basically some
renames and an API change. The hasInFlowGridItems was renamed to
hasGridItems because by definition all grid items are in flow children of
the grid container. Apart from that, in order to remove an extra attribute
for the Grid::insert method, a new method called
setHasAnyOrthogonalGridItem was added to track the existence of orthogonal
items.

BUG=627812
Committed: https://crrev.com/9b31974d98b5e1c03990d68129e6debca1e37cd6
Cr-Commit-Position: refs/heads/master@{#434641}

[svillar, 2016-11-25 18:16:09]

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

[Manuel Rego, 2016-11-25 18:23:25]

lgtm

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

https://codereview.chromium.org/2526383004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.h:351: bool hasGridItems()
const { return !m_gridItemArea.isEmpty(); }
Nit: Maybe we could have a isEmpty() method. Not 100% sure
as it might be confusing due to the out-of-flow
and in-flow children.

[svillar, 2016-11-28 11:21:25]

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

https://codereview.chromium.org/2526383004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.h:351: bool hasGridItems()
const { return !m_gridItemArea.isEmpty(); }
On 2016/11/25 18:23:25, Manuel Rego wrote:
> Nit: Maybe we could have a isEmpty() method. Not 100% sure
> as it might be confusing due to the out-of-flow
> and in-flow children.

I don't think we need that. We could always check the existence of children
using the LayoutBox methods.

[svillar, 2016-11-28 11:21:29]

The CQ bit was checked by svillar@igalia.com

[commit-bot: I haz the power, 2016-11-28 11:21:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Manuel Rego, 2016-11-28 11:46:04]

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

https://codereview.chromium.org/2526383004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutGrid.h:351: bool hasGridItems()
const { return !m_gridItemArea.isEmpty(); }
On 2016/11/28 11:21:25, svillar wrote:
> On 2016/11/25 18:23:25, Manuel Rego wrote:
> > Nit: Maybe we could have a isEmpty() method. Not 100% sure
> > as it might be confusing due to the out-of-flow
> > and in-flow children.
> 
> I don't think we need that. We could always check the existence of children
> using the LayoutBox methods.

It's clear I choose a bad name, I was thinking in:
  bool isEmpty() const { return m_gridItemArea.isEmpty(); }

[commit-bot: I haz the power, 2016-11-28 12:35:32]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1480332089923080, "parent_rev":
"5c2b14d1bfbca87831cf9e84b052a9ef0068839f", "commit_rev":
"63ee9360cc0adc9ae9b0849aceeb2a5dde11474d"}

[commit-bot: I haz the power, 2016-11-28 12:35:58]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-28 12:37:23]

Description was changed from

==========
[css-grid] Cleanup LayoutGrid::Grid

This is a cleanup of some recent additions to the Grid class, basically some
renames and an API change. The hasInFlowGridItems was renamed to
hasGridItems because by definition all grid items are in flow children of
the grid container. Apart from that, in order to remove an extra attribute
for the Grid::insert method, a new method called
setHasAnyOrthogonalGridItem was added to track the existence of orthogonal
items.

BUG=627812
==========

to

==========
[css-grid] Cleanup LayoutGrid::Grid

This is a cleanup of some recent additions to the Grid class, basically some
renames and an API change. The hasInFlowGridItems was renamed to
hasGridItems because by definition all grid items are in flow children of
the grid container. Apart from that, in order to remove an extra attribute
for the Grid::insert method, a new method called
setHasAnyOrthogonalGridItem was added to track the existence of orthogonal
items.

BUG=627812

Committed: https://crrev.com/9b31974d98b5e1c03990d68129e6debca1e37cd6
Cr-Commit-Position: refs/heads/master@{#434641}
==========

[commit-bot: I haz the power, 2016-11-28 12:37:24]

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

[svillar, 2016-11-28 14:36:53]

On 2016/11/28 11:46:04, Manuel Rego wrote:
>
https://codereview.chromium.org/2526383004/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/layout/LayoutGrid.h (right):
> 
>
https://codereview.chromium.org/2526383004/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/layout/LayoutGrid.h:351: bool hasGridItems()
> const { return !m_gridItemArea.isEmpty(); }
> On 2016/11/28 11:21:25, svillar wrote:
> > On 2016/11/25 18:23:25, Manuel Rego wrote:
> > > Nit: Maybe we could have a isEmpty() method. Not 100% sure
> > > as it might be confusing due to the out-of-flow
> > > and in-flow children.
> > 
> > I don't think we need that. We could always check the existence of children
> > using the LayoutBox methods.
> 
> It's clear I choose a bad name, I was thinking in:
>   bool isEmpty() const { return m_gridItemArea.isEmpty(); }

So the thing is that m_gridItemArea is a very specific implementation detail of
Grid. I don't think we should expose it too much.