[LayoutNG] Initial support for the 'vertical-align' property

[LayoutNG] Initial support for the 'vertical-align' property

This patch supports the 'vertical-align' property.

Positioning based on 'vertial-align' requires, depends on values:
a. Per-element positioning to support nested cases.
b. The layout size of the element.
c. The layout size of the parent element.
d. The layout size of the line box.

This patch adds NGInlineBoxState to keep track of the current inline
box. This is also used to cache information common to a box, similar to
what VerticalPositionCache does in LayoutObject.

Another struct, NGPendingPositions, keeps track of descendants that need
to be positioned when ancestor layout is done. NGInlineBoxState is
expected to be used for other prorperties, while NGPendingPositions may
be only for 'vertical-align'.

This patch passes several tests in CSS2/linebox, but their expected
files use yet to be supported properties such as inline margins or
relative positions that no tests can be marked as pass in
TestExpectations yet.

BUG=636993
Review-Url: https://codereview.chromium.org/2803433002
Cr-Commit-Position: refs/heads/master@{#465409}
Committed: https://chromium.googlesource.com/chromium/src/+/2333a26198a16f94991afa411f1066c9d4687423

[kojii, 2017-04-05 06:04:06]

Description was changed from

==========
WIP: ngline

BUG=
==========

to

==========
WIP: ngvalign

BUG=
==========

[kojii, 2017-04-06 07:59:59]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-06 08:00:11]

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

[commit-bot: I haz the power, 2017-04-06 09:19:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-06 09:19:50]

Dry run: This issue passed the CQ dry run.

[kojii, 2017-04-16 19:31:12]

Patchset #10 (id:180001) has been deleted

[kojii, 2017-04-17 01:39:50]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 01:39:53]

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

[kojii, 2017-04-17 03:07:04]

Description was changed from

==========
WIP: ngvalign

BUG=
==========

to

==========
[LayoutNG] Initial support of 'vertical-align' property

This patch supports the 'vertical-align' property.

Positioning based on 'vertial-align' requires, depends on values:
a. Per-element positioning to support nested cases.
b. The layout size of the element.
c. The layout size of the parent element.
d. The layout size of the line box.

This patch adds NGInlineBoxState to keep track of the current inline
box. This is also used to cache information common to a box, similar to
what VerticalPositionCache does in LayoutObject.

Another struct, NGPendingPositions, keeps track of descendants that need
to be positioned when ancestor layout is done. NGInlineBoxState is
expected to be used for other prorperties, while NGPendingPositions may
be only for 'vertical-align'.

This patch passes several tests in CSS2/linebox, but their expected
files use yet to be supported properties such as inline margins or
relative positions that no tests can be marked as pass in
TestExpectations yet.

BUG=636993
==========

[kojii, 2017-04-17 03:07:15]

Description was changed from

==========
[LayoutNG] Initial support of 'vertical-align' property

This patch supports the 'vertical-align' property.

Positioning based on 'vertial-align' requires, depends on values:
a. Per-element positioning to support nested cases.
b. The layout size of the element.
c. The layout size of the parent element.
d. The layout size of the line box.

This patch adds NGInlineBoxState to keep track of the current inline
box. This is also used to cache information common to a box, similar to
what VerticalPositionCache does in LayoutObject.

Another struct, NGPendingPositions, keeps track of descendants that need
to be positioned when ancestor layout is done. NGInlineBoxState is
expected to be used for other prorperties, while NGPendingPositions may
be only for 'vertical-align'.

This patch passes several tests in CSS2/linebox, but their expected
files use yet to be supported properties such as inline margins or
relative positions that no tests can be marked as pass in
TestExpectations yet.

BUG=636993
==========

to

==========
[LayoutNG] Initial support for the 'vertical-align' property

This patch supports the 'vertical-align' property.

Positioning based on 'vertial-align' requires, depends on values:
a. Per-element positioning to support nested cases.
b. The layout size of the element.
c. The layout size of the parent element.
d. The layout size of the line box.

This patch adds NGInlineBoxState to keep track of the current inline
box. This is also used to cache information common to a box, similar to
what VerticalPositionCache does in LayoutObject.

Another struct, NGPendingPositions, keeps track of descendants that need
to be positioned when ancestor layout is done. NGInlineBoxState is
expected to be used for other prorperties, while NGPendingPositions may
be only for 'vertical-align'.

This patch passes several tests in CSS2/linebox, but their expected
files use yet to be supported properties such as inline margins or
relative positions that no tests can be marked as pass in
TestExpectations yet.

BUG=636993
==========

[commit-bot: I haz the power, 2017-04-17 03:14:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-17 03:14:14]

Dry run: This issue passed the CQ dry run.

[kojii, 2017-04-17 03:15:23]

kojii@chromium.org changed reviewers:
+ eae@chromium.org, ikilpatrick@chromium.org

[kojii, 2017-04-17 03:15:24]

PTAL. CSSWG tests are back online and this is ready for review.

I ended up that stack is the best to handle inline tree. I tried bidi-level-like
algorithm, but handling close+open case was troublesome. Thought about
min-level, but traversing style tree is still needed.

Also, the current code caching per-box-information in VerticalPositionCache
looks very promising in most common cases, this is supported now.

[kojii, 2017-04-17 08:57:19]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 08:57:38]

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

[commit-bot: I haz the power, 2017-04-17 12:16:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-17 12:16:49]

Dry run: This issue passed the CQ dry run.

[kojii, 2017-04-17 14:03:56]

Split NGInlineBoxState to a separate file.

Now I'm not sure which class should ApplyBaselineShift be, it could be in either
way. Any preference?

[ikilpatrick, 2017-04-18 01:04:37]

https://codereview.chromium.org/2803433002/diff/240001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h
(right):

https://codereview.chromium.org/2803433002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h:141:
NGInlineBoxState* OnBeginPlaceItems();
Oh, sorry, does it make sense to move, OnBeginPlaceItems, OnOpenTag, OnCloseTag,
etc, inside that object?

[eae, 2017-04-18 02:08:59]

This is awesome, LGTM pending Ians comments.

[kojii, 2017-04-18 02:38:56]

Thank you for having look, both.

On 2017/04/18 at 01:04:37, ikilpatrick wrote:
> Oh, sorry, does it make sense to move, OnBeginPlaceItems, OnOpenTag,
OnCloseTag, etc, inside that object?

Oh, sorry, I misunderstood. That makes sense, will do.

[ikilpatrick, 2017-04-18 02:54:21]

On 2017/04/18 02:38:56, kojii wrote:
> Thank you for having look, both.
> 
> On 2017/04/18 at 01:04:37, ikilpatrick wrote:
> > Oh, sorry, does it make sense to move, OnBeginPlaceItems, OnOpenTag,
> OnCloseTag, etc, inside that object?
> 
> Oh, sorry, I misunderstood. That makes sense, will do.

Sorry :)

[kojii, 2017-04-18 02:58:52]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 02:59:16]

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

[kojii, 2017-04-18 03:04:33]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[kojii, 2017-04-18 03:04:39]

Patchset #13 (id:260001) has been deleted

[commit-bot: I haz the power, 2017-04-18 03:04:55]

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

[commit-bot: I haz the power, 2017-04-18 04:19:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 04:19:08]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[kojii, 2017-04-18 05:01:15]

The CQ bit was checked by kojii@chromium.org to run a CQ dry run

[kojii, 2017-04-18 05:01:19]

Patchset #13 (id:280001) has been deleted

[commit-bot: I haz the power, 2017-04-18 05:01:32]

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

[commit-bot: I haz the power, 2017-04-18 06:52:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 06:52:40]

Dry run: This issue passed the CQ dry run.

[kojii, 2017-04-18 14:29:58]

Done, PTAL.

# no sorry please ;)

[ikilpatrick, 2017-04-18 20:55:02]

lgtm

[kojii, 2017-04-18 22:48:07]

The CQ bit was checked by kojii@chromium.org

[kojii, 2017-04-18 22:48:08]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2803433002/#ps300001
(title: "Move all On*() to NGInlineLayoutStateStack")

[commit-bot: I haz the power, 2017-04-18 22:48:44]

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

[commit-bot: I haz the power, 2017-04-18 22:55:25]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1492555687641230,
"parent_rev": "125049e04ba04377a30dcdd62df014f2ee396de9", "commit_rev":
"2333a26198a16f94991afa411f1066c9d4687423"}

[commit-bot: I haz the power, 2017-04-18 22:55:38]

Description was changed from

==========
[LayoutNG] Initial support for the 'vertical-align' property

This patch supports the 'vertical-align' property.

Positioning based on 'vertial-align' requires, depends on values:
a. Per-element positioning to support nested cases.
b. The layout size of the element.
c. The layout size of the parent element.
d. The layout size of the line box.

This patch adds NGInlineBoxState to keep track of the current inline
box. This is also used to cache information common to a box, similar to
what VerticalPositionCache does in LayoutObject.

Another struct, NGPendingPositions, keeps track of descendants that need
to be positioned when ancestor layout is done. NGInlineBoxState is
expected to be used for other prorperties, while NGPendingPositions may
be only for 'vertical-align'.

This patch passes several tests in CSS2/linebox, but their expected
files use yet to be supported properties such as inline margins or
relative positions that no tests can be marked as pass in
TestExpectations yet.

BUG=636993
==========

to

==========
[LayoutNG] Initial support for the 'vertical-align' property

This patch supports the 'vertical-align' property.

Positioning based on 'vertial-align' requires, depends on values:
a. Per-element positioning to support nested cases.
b. The layout size of the element.
c. The layout size of the parent element.
d. The layout size of the line box.

This patch adds NGInlineBoxState to keep track of the current inline
box. This is also used to cache information common to a box, similar to
what VerticalPositionCache does in LayoutObject.

Another struct, NGPendingPositions, keeps track of descendants that need
to be positioned when ancestor layout is done. NGInlineBoxState is
expected to be used for other prorperties, while NGPendingPositions may
be only for 'vertical-align'.

This patch passes several tests in CSS2/linebox, but their expected
files use yet to be supported properties such as inline margins or
relative positions that no tests can be marked as pass in
TestExpectations yet.

BUG=636993

Review-Url: https://codereview.chromium.org/2803433002
Cr-Commit-Position: refs/heads/master@{#465409}
Committed:
https://chromium.googlesource.com/chromium/src/+/2333a26198a16f94991afa411f10...
==========

[commit-bot: I haz the power, 2017-04-18 22:55:39]

Committed patchset #13 (id:300001) as
https://chromium.googlesource.com/chromium/src/+/2333a26198a16f94991afa411f10...