New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug

Blocking:
issue 591099



Sign in to add a comment
link

Issue 845235: [LayoutNG] Flexbox implementation

Reported by dgro...@chromium.org, May 21 2018 Project Member

Issue description

Implement Flexbox in NG
 

Comment 1 by dgro...@chromium.org, May 21 2018

Patches that have already landed
https://chromium-review.googlesource.com/961793 [LayoutNG] Basic flexbox logic behind runtime flag
https://chromium-review.googlesource.com/1003526 [LayoutNG] Fix a few ng flexbox nits
https://chromium-review.googlesource.com/1015985 [LayoutNG] Add flexbox tests to virtual/layout_ng_experimental

Comment 2 by bugdroid1@chromium.org, May 24 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d8da92027fb6e434d47975194d69004122ce5b7

commit 3d8da92027fb6e434d47975194d69004122ce5b7
Author: David Grogan <dgrogan@chromium.org>
Date: Thu May 24 22:35:59 2018

[LayoutNG] Make LayoutNGFlexibleBox inherit from LayoutBlock

It previously inherited from LayoutNGBlockFlow, which, aside from
violating is-a, prevented inline flex children from being blockified --
LayoutNGBlockFlow::AddChild does not call LayoutBlock::AddChild, which
is where the blockification happens.

Considered inheriting from LayoutFlexibleBox to get better baseline and
intrinsic size support, but those will have to be reimplemented in
LayoutNG anyway.

With this change, IsLayoutNGMixin() is even further insufficient to
check if a LayoutObject is implemented in NG. I modified some of the
checks to include IsLayoutNGFlexibleBox, but am not sure about the
accuracy.

mac-specific css3/flexbox/button-expected.png is because the underline
in the link is 1 pixel shorter.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I42811edeb53e8e2a80a64d5b30bf6627e0957b7d
Bug: 845235
Reviewed-on: https://chromium-review.googlesource.com/1062499
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561661}
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/WebKit/LayoutTests/platform/mac/virtual/layout_ng_experimental/css3/flexbox/button-expected.png
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/BUILD.gn
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/layout_flexible_box.cc
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/layout_object.h
[add] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/ng/layout_ng_flexible_box.cc
[add] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/ng/layout_ng_flexible_box.h
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/ng/legacy_layout_tree_walking.cc
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/ng/legacy_layout_tree_walking.h
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/ng/ng_block_node.cc
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/3d8da92027fb6e434d47975194d69004122ce5b7/third_party/blink/renderer/core/layout/ng/ng_physical_fragment.cc

Comment 3 by bugdroid1@chromium.org, Jun 6 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/906e303a44b795b46ae50e9bed29266c93789cd5

commit 906e303a44b795b46ae50e9bed29266c93789cd5
Author: David Grogan <dgrogan@chromium.org>
Date: Wed Jun 06 23:08:24 2018

[LayoutNG] Support all types of flex base sizes

Old code only supported auto and fixed flex bases. This makes 3 more
tests pass.

Also added a ton of comments.

From spot checking, it seems many non-crash failures are due to not
setting the flex container's height correctly, e.g.
external/wpt/css/css-flexbox/align-content-001.htm
css3/flexbox/relpos-with-percentage-top.html

Bug: 845235
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I06a3d8f4b8923c8acb1d6fb07826da66d55826a5
Reviewed-on: https://chromium-review.googlesource.com/1088229
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565084}
[modify] https://crrev.com/906e303a44b795b46ae50e9bed29266c93789cd5/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/906e303a44b795b46ae50e9bed29266c93789cd5/third_party/blink/renderer/core/layout/ng/layout_ng_flexible_box.cc
[modify] https://crrev.com/906e303a44b795b46ae50e9bed29266c93789cd5/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/906e303a44b795b46ae50e9bed29266c93789cd5/third_party/blink/renderer/core/layout/ng/ng_length_utils.h

Comment 4 by bugdroid1@chromium.org, Jun 20 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/760dd0e8391bf8ece7016e73074512fda0f16b03

commit 760dd0e8391bf8ece7016e73074512fda0f16b03
Author: David Grogan <dgrogan@chromium.org>
Date: Wed Jun 20 00:01:50 2018

[LayoutNG] Set flexbox's inline and block sizes

Also pass content size to the flex algorithm in a few places where
border-box size had been getting passed.

33 new passes

Bug: 845235
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I3d6556bed482a6aed7b5089390f874654a87ed60
Reviewed-on: https://chromium-review.googlesource.com/1096451
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568652}
[modify] https://crrev.com/760dd0e8391bf8ece7016e73074512fda0f16b03/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/760dd0e8391bf8ece7016e73074512fda0f16b03/third_party/blink/renderer/core/layout/flexible_box_algorithm.h
[modify] https://crrev.com/760dd0e8391bf8ece7016e73074512fda0f16b03/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc

Comment 5 by bugdroid1@chromium.org, Jul 13 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef2f8876957a5725fafb37c43e4d7cdb59d5d119

commit ef2f8876957a5725fafb37c43e4d7cdb59d5d119
Author: David Grogan <dgrogan@chromium.org>
Date: Fri Jul 13 05:44:43 2018

[LayoutNG] Don't DCHECK on items with orthogonal writing mode

Instead use ComputeMinMaxSize's new functionality that runs full layout
in such situations.

Only 4 tests were most proximately hitting this DCHECK. 3 of them now
hit another and 1 has a mismatch.

Bug: 845235
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I7b1373e2e61a655043c568aa133bc4b1676e2ad0
Reviewed-on: https://chromium-review.googlesource.com/1136052
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574846}
[modify] https://crrev.com/ef2f8876957a5725fafb37c43e4d7cdb59d5d119/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/ef2f8876957a5725fafb37c43e4d7cdb59d5d119/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc

Comment 6 by bugdroid1@chromium.org, Jul 18 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f4499cd3f70fb1fde45d470ce929e4f0a572961

commit 7f4499cd3f70fb1fde45d470ce929e4f0a572961
Author: David Grogan <dgrogan@chromium.org>
Date: Wed Jul 18 15:27:59 2018

[LayoutNG] Use max content width for resolving auto flex basis

Used to include padding scrollbars border

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I69a56029aded1f33bf232444de79026335a3bfa5
Bug: 845235
Reviewed-on: https://chromium-review.googlesource.com/1136058
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576068}
[modify] https://crrev.com/7f4499cd3f70fb1fde45d470ce929e4f0a572961/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/7f4499cd3f70fb1fde45d470ce929e4f0a572961/third_party/blink/renderer/core/layout/flexible_box_algorithm.h
[modify] https://crrev.com/7f4499cd3f70fb1fde45d470ce929e4f0a572961/third_party/blink/renderer/core/layout/layout_flexible_box.cc
[modify] https://crrev.com/7f4499cd3f70fb1fde45d470ce929e4f0a572961/third_party/blink/renderer/core/layout/ng/ng_block_node.h
[modify] https://crrev.com/7f4499cd3f70fb1fde45d470ce929e4f0a572961/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/7f4499cd3f70fb1fde45d470ce929e4f0a572961/third_party/blink/renderer/core/layout/ng/ng_layout_input_node.h

Comment 7 by bugdroid1@chromium.org, Jul 27 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81160a6f973debbbf0290256a9c0209fba84a071

commit 81160a6f973debbbf0290256a9c0209fba84a071
Author: David Grogan <dgrogan@chromium.org>
Date: Fri Jul 27 08:51:46 2018

[LayoutNG] Flex algorithm sizing improvements

We now set AvailableSize and PercentageResolutionSize on the
ConstraintSpace properly. And also now give the flex layout algorithm
more accurate offsets and main axis size.

Bug: 845235
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ia24778ffd24a6f1122e778919686a082a63d4b06
Reviewed-on: https://chromium-review.googlesource.com/1142729
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578567}
[modify] https://crrev.com/81160a6f973debbbf0290256a9c0209fba84a071/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/81160a6f973debbbf0290256a9c0209fba84a071/third_party/WebKit/LayoutTests/css3/flexbox/block-available-size.html
[modify] https://crrev.com/81160a6f973debbbf0290256a9c0209fba84a071/third_party/blink/renderer/core/layout/flexible_box_algorithm.h
[modify] https://crrev.com/81160a6f973debbbf0290256a9c0209fba84a071/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc

Comment 8 by bugdroid1@chromium.org, Nov 1

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7b541267263fa48be66f29fbe346cc403ccafba

commit a7b541267263fa48be66f29fbe346cc403ccafba
Author: David Grogan <dgrogan@chromium.org>
Date: Thu Nov 01 00:10:37 2018

[LayoutNG] Make flexboxes stretch their items

Includes moving the stretched size computation from legacy to FlexItem
so that NG can use it.

A big hack is that stretched items don't use the height of their parent
lines, they use the whole container height because we haven't
implemented line size calculation yet. This works for the majority of
cases.

About 6 tests newly fail. Some multi-line flex containers now fail
because of the above hack. But some tests with images that are flex
items fail because NG doesn't copy their data to legacy because there
are unplaced floats internal to the image (something related to alt
text). I haven't looked into why this used to work but fails now.

This patch nets roughly 110 layout test lines removed.

Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I0083e80797c1f01c8cc4c63ac696d68063f67ff2
Bug: 845235
Reviewed-on: https://chromium-review.googlesource.com/c/1285851
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604457}
[modify] https://crrev.com/a7b541267263fa48be66f29fbe346cc403ccafba/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/a7b541267263fa48be66f29fbe346cc403ccafba/third_party/blink/renderer/core/layout/flexible_box_algorithm.cc
[modify] https://crrev.com/a7b541267263fa48be66f29fbe346cc403ccafba/third_party/blink/renderer/core/layout/flexible_box_algorithm.h
[modify] https://crrev.com/a7b541267263fa48be66f29fbe346cc403ccafba/third_party/blink/renderer/core/layout/layout_flexible_box.cc
[modify] https://crrev.com/a7b541267263fa48be66f29fbe346cc403ccafba/third_party/blink/renderer/core/layout/layout_flexible_box.h
[modify] https://crrev.com/a7b541267263fa48be66f29fbe346cc403ccafba/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc

Comment 10 by bugdroid1@chromium.org, Nov 27

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1703e743503d1194ec730bbc029f45216b2e95d2

commit 1703e743503d1194ec730bbc029f45216b2e95d2
Author: Ken Rockot <rockot@google.com>
Date: Tue Nov 27 23:22:26 2018

Revert "[LayoutNG] Support non-horizontal flows in Flexbox"

This reverts commit 7fbb7857e15ccc0136737b3081259212596b6f8c.

Reason for revert: Persistent failures on https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20ASAN/ - see  https://crbug.com/909021 

Original change's description:
> [LayoutNG] Support non-horizontal flows in Flexbox
> 
> Meaning: column containers, ortho children, and non-horizontal
> writing modes
> 
> Bug: 845235
> Change-Id: Iba6525752d0c14a77bc9a13cc75a484cdf3a711d
> Reviewed-on: https://chromium-review.googlesource.com/c/1332247
> Commit-Queue: David Grogan <dgrogan@chromium.org>
> Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#611268}

TBR=cbiesinger@chromium.org,dgrogan@chromium.org

Change-Id: Ib66afedb725af13acd85a298c4fd565f02b32019
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 845235, 909021 
Reviewed-on: https://chromium-review.googlesource.com/c/1352670
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#611402}
[modify] https://crrev.com/1703e743503d1194ec730bbc029f45216b2e95d2/third_party/blink/renderer/core/layout/flexible_box_algorithm.h
[modify] https://crrev.com/1703e743503d1194ec730bbc029f45216b2e95d2/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/1703e743503d1194ec730bbc029f45216b2e95d2/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.h
[modify] https://crrev.com/1703e743503d1194ec730bbc029f45216b2e95d2/third_party/blink/renderer/core/layout/ng/ng_length_utils.h
[modify] https://crrev.com/1703e743503d1194ec730bbc029f45216b2e95d2/third_party/blink/web_tests/TestExpectations

Comment 11 by bugdroid1@chromium.org, Dec 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca

commit 17e1310d067d7eb5fb00e04da7c88ab97e1f6eca
Author: David Grogan <dgrogan@chromium.org>
Date: Tue Dec 11 00:15:48 2018

[LayoutNG] Stop casting LayoutNGFlexibleBox to LayoutFlexibleBox

LayoutNGFlexibleBox allowed itself to be casted to a LayoutFlexibleBox
but it wasn't one. This caused crashes, naturally.

I audited all the calls to IsFlexibleBox* and IsFlexItem*, updating to
NG where appropriate.

Bug: 845235,  909021 
Change-Id: Ibb3ee32a5f7a78b5d46702ad7426597f23e39d2a
Reviewed-on: https://chromium-review.googlesource.com/c/1368690
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615341}
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/css/properties/computed_style_utils.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/dom/first_letter_pseudo_element.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/dom/text.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/editing/visible_units.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/flexible_box_algorithm.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_block.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_block_flow.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_box.h
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_deprecated_flexible_box.h
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_flexible_box.h
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_media.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_object.h
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/layout_table.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/ng/layout_ng_flexible_box.h
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/ng/ng_layout_input_node.h
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/ng/ng_length_utils.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/table_layout_algorithm_auto.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/renderer/core/layout/text_autosizer.cc
[modify] https://crrev.com/17e1310d067d7eb5fb00e04da7c88ab97e1f6eca/third_party/blink/web_tests/TestExpectations

Comment 12 by bugdroid1@chromium.org, Dec 11

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 15

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69ec52bd0b32622770a25952386596ccb4ad6434

commit 69ec52bd0b32622770a25952386596ccb4ad6434
Author: David Grogan <dgrogan@chromium.org>
Date: Sat Dec 15 01:42:40 2018

[LayoutNG] A few small cleanups in flex code

 - Don't use legacy when we don't have to
 - Store NGBlockNode instead of NGLayoutInputNode

Bug: 845235
Change-Id: I99e48f7f69e4f53f0391e2cd542e117b501cb7d6
Reviewed-on: https://chromium-review.googlesource.com/c/1379061
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616907}
[modify] https://crrev.com/69ec52bd0b32622770a25952386596ccb4ad6434/third_party/blink/renderer/core/layout/flexible_box_algorithm.h
[modify] https://crrev.com/69ec52bd0b32622770a25952386596ccb4ad6434/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc

Comment 14 by bugdroid1@chromium.org, Dec 18

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/446c78426159cffe11a71580c8586e979c8f3933

commit 446c78426159cffe11a71580c8586e979c8f3933
Author: David Grogan <dgrogan@chromium.org>
Date: Tue Dec 18 19:40:41 2018

[LayoutNG] Don't crash when flex has out of flow children

Static position isn't calculated in this patch but at least we now
layout the out of flow children instead of crashing.

Bug: 845235
Change-Id: I96ca19179de35624978fb28aade2c02a4ce5cb7b
Reviewed-on: https://chromium-review.googlesource.com/c/1379109
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617592}
[modify] https://crrev.com/446c78426159cffe11a71580c8586e979c8f3933/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/446c78426159cffe11a71580c8586e979c8f3933/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.h
[modify] https://crrev.com/446c78426159cffe11a71580c8586e979c8f3933/third_party/blink/web_tests/TestExpectations

Comment 15 by bugdroid1@chromium.org, Dec 21

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8bdb07e6e3ac1f210337acc9c84c79a379c1e940

commit 8bdb07e6e3ac1f210337acc9c84c79a379c1e940
Author: David Grogan <dgrogan@chromium.org>
Date: Fri Dec 21 21:39:02 2018

[LayoutNG] Give flex algorithm correct main-axis min/max sizes for items

We don't yet compute min-width:auto though.

Bug: 845235
Change-Id: I74fff512b283647cad12406233fc8b68dccfb55e
Reviewed-on: https://chromium-review.googlesource.com/c/1381836
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618609}
[modify] https://crrev.com/8bdb07e6e3ac1f210337acc9c84c79a379c1e940/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/8bdb07e6e3ac1f210337acc9c84c79a379c1e940/third_party/blink/web_tests/TestExpectations

Comment 16 by bugdroid1@chromium.org, Jan 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c7c45c1cf913b95e2aca6244ef3d03a11e6735d

commit 4c7c45c1cf913b95e2aca6244ef3d03a11e6735d
Author: David Grogan <dgrogan@chromium.org>
Date: Sat Jan 05 21:22:28 2019

[LayoutNG] Fix flexbox height calculation

We were double counting the block-start border.

Bug: 845235
Change-Id: I7a1217c415c800fb5d5517155b63da84e720f8b7
Reviewed-on: https://chromium-review.googlesource.com/c/1396316
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620195}
[modify] https://crrev.com/4c7c45c1cf913b95e2aca6244ef3d03a11e6735d/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/4c7c45c1cf913b95e2aca6244ef3d03a11e6735d/third_party/blink/web_tests/TestExpectations

Comment 17 by bugdroid1@chromium.org, Jan 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca41baba579f7e0ec716cc09470ee222b1bd2f21

commit ca41baba579f7e0ec716cc09470ee222b1bd2f21
Author: David Grogan <dgrogan@chromium.org>
Date: Tue Jan 08 20:00:52 2019

[LayoutNG] Calculate flex container intrinsic sizes

This is based off the old algorithm that legacy implements. The new
algorithm makes more sense but will have compat problems.

Bug: 845235
Change-Id: Ib4c115e9ad955ce6ab8a5fbcdd2d993feeebfd4f
Reviewed-on: https://chromium-review.googlesource.com/c/1389315
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620849}
[modify] https://crrev.com/ca41baba579f7e0ec716cc09470ee222b1bd2f21/third_party/blink/renderer/core/layout/ng/geometry/ng_box_strut.h
[modify] https://crrev.com/ca41baba579f7e0ec716cc09470ee222b1bd2f21/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/ca41baba579f7e0ec716cc09470ee222b1bd2f21/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.h
[modify] https://crrev.com/ca41baba579f7e0ec716cc09470ee222b1bd2f21/third_party/blink/web_tests/TestExpectations

Comment 18 by bugdroid1@chromium.org, Jan 9

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c2d7130928c578cf9b68f7eaf192ae2ff4a228fb

commit c2d7130928c578cf9b68f7eaf192ae2ff4a228fb
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Wed Jan 09 00:52:20 2019

Revert "[LayoutNG] Calculate flex container intrinsic sizes"

This reverts commit ca41baba579f7e0ec716cc09470ee222b1bd2f21.

Reason for revert: This causes multiple layout test failures on some bots:  crbug.com/920039 

Original change's description:
> [LayoutNG] Calculate flex container intrinsic sizes
> 
> This is based off the old algorithm that legacy implements. The new
> algorithm makes more sense but will have compat problems.
> 
> Bug: 845235
> Change-Id: Ib4c115e9ad955ce6ab8a5fbcdd2d993feeebfd4f
> Reviewed-on: https://chromium-review.googlesource.com/c/1389315
> Commit-Queue: David Grogan <dgrogan@chromium.org>
> Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620849}

TBR=cbiesinger@chromium.org,dgrogan@chromium.org,mstensho@chromium.org

Change-Id: I14cdf536035404693d11d656c09f19124d299fe2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 845235
Reviewed-on: https://chromium-review.googlesource.com/c/1401861
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620970}
[modify] https://crrev.com/c2d7130928c578cf9b68f7eaf192ae2ff4a228fb/third_party/blink/renderer/core/layout/ng/geometry/ng_box_strut.h
[modify] https://crrev.com/c2d7130928c578cf9b68f7eaf192ae2ff4a228fb/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/c2d7130928c578cf9b68f7eaf192ae2ff4a228fb/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.h
[modify] https://crrev.com/c2d7130928c578cf9b68f7eaf192ae2ff4a228fb/third_party/blink/web_tests/TestExpectations

Comment 19 by bugdroid1@chromium.org, Jan 9

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99d8923f2015fbf90c100abda3a9989d4096119d

commit 99d8923f2015fbf90c100abda3a9989d4096119d
Author: David Grogan <dgrogan@chromium.org>
Date: Wed Jan 09 02:46:57 2019

Reland [LayoutNG] Calculate flex container intrinsic sizes

The 7 tests marked Crash crash locally for me and crashed on the trybots
but don't on one of the continuous builders[1], possibly that runs
without DCHECK enabled. 5 of the tests failed and 2 passed. The failures
surfaced as regressions, which got this patch reverted. So just go back
to Skip.

[1] https://test-results.appspot.com/data/layout_results/Linux_Tests/74761/webkit_layout_tests/layout-test-results/results.html

TBR=mstensho@chromium.org

Bug: 845235, 920039 
Change-Id: I3970b3fd77633240a21198358b3f1913022977f3
Reviewed-on: https://chromium-review.googlesource.com/c/1401673
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621015}
[modify] https://crrev.com/99d8923f2015fbf90c100abda3a9989d4096119d/third_party/blink/renderer/core/layout/ng/geometry/ng_box_strut.h
[modify] https://crrev.com/99d8923f2015fbf90c100abda3a9989d4096119d/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[modify] https://crrev.com/99d8923f2015fbf90c100abda3a9989d4096119d/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.h
[modify] https://crrev.com/99d8923f2015fbf90c100abda3a9989d4096119d/third_party/blink/web_tests/TestExpectations

Comment 20 by benhenry@google.com, Jan 11

Status: Available (was: Started)
This issue has been marked as started, but has no owner. Making available.

Comment 21 by dgro...@chromium.org, Jan 15

Owner: dgro...@chromium.org
Status: Started (was: Available)

Comment 22 by bugdroid1@chromium.org, Jan 15

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2828b9aff274de87efa0619f02e529a17799c49b

commit 2828b9aff274de87efa0619f02e529a17799c49b
Author: David Grogan <dgrogan@chromium.org>
Date: Tue Jan 15 02:33:55 2019

[LayoutNG] Fix bug in resolving flex-basis:auto

When a column flexbox and an item had orthogonal writing modes (meaning
the child's inline direction matches the container's main axis), we were
using the item's max content size in the parent's writing mode to
resolve its flex-basis, but we should have been using it in the child's
writing mode.

Bug: 845235
Change-Id: I53bd5ba39f9c8cb4ac84dc4638c8abc57e38f974
Reviewed-on: https://chromium-review.googlesource.com/c/1409810
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622688}
[modify] https://crrev.com/2828b9aff274de87efa0619f02e529a17799c49b/third_party/blink/renderer/core/layout/ng/ng_flex_layout_algorithm.cc
[add] https://crrev.com/2828b9aff274de87efa0619f02e529a17799c49b/third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-basis-009.html

Comment 24 by bugdroid, Feb 13 (5 days ago)

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/415523d3c50b0d4248e8c284afc9413a26673226

commit 415523d3c50b0d4248e8c284afc9413a26673226
Author: David Grogan <dgrogan@chromium.org>
Date: Wed Feb 13 18:45:19 2019

[LayoutNG] Some flex tests now pass because of non-flex NG work

Bug: 845235
Change-Id: Ib29ad01e7337bf818c6c7b7d69282c30aad3b01b
Reviewed-on: https://chromium-review.googlesource.com/c/1468767
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#631811}
[modify] https://crrev.com/415523d3c50b0d4248e8c284afc9413a26673226/third_party/blink/web_tests/TestExpectations

Sign in to add a comment