Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(135)

Issue 2954953002: [LayoutNG] Abort a layout once the BFC offset is resolved. (Closed)

Created:
3 years, 5 months ago by ikilpatrick
Modified:
3 years, 5 months ago
Reviewers:
cbiesinger, kojii, eae
CC:
chromium-reviews, cbiesinger, ojan+watch_chromium.org, szager+layoutwatch_chromium.org, glebl+reviews_chromium.org, dgrogan+ng_chromium.org, atotic+reviews_chromium.org, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, zoltan1, jchaffraix+rendering, blink-reviews, lchoi+reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[LayoutNG] Abort a layout once the BFC offset is resolved. This patch allows layouts to "abort" if they have resolved their BFC offset. This occurs when there is a previous sibling which has an unpositioned float inside of it. We resolve the BFC offset, then abort the layout up to the point where we can begin the relayout (a parent which knows its BFC offset). The algorithm in this state works exactly as before, except that floats are positioned at their calculated BFC offset. BUG=635619 Review-Url: https://codereview.chromium.org/2954953002 Cr-Commit-Position: refs/heads/master@{#486065} Committed: https://chromium.googlesource.com/chromium/src/+/f905d51bad504b4b43149637760b649aea7ddc24

Patch Set 1 #

Patch Set 2 : tinker #

Patch Set 3 : tailor #

Patch Set 4 : soldier #

Patch Set 5 : spy #

Patch Set 6 : tinker #

Patch Set 7 : tailor #

Patch Set 8 : soldier #

Patch Set 9 : spy #

Patch Set 10 : [LayoutNG] A different approach to floats... #

Patch Set 11 : remove hacks. #

Patch Set 12 : \o #

Patch Set 13 : \o/ #

Patch Set 14 : [LayoutNG] A different approach to floats... #

Patch Set 15 : /o\ #

Patch Set 16 : ... #

Patch Set 17 : \o/ #

Patch Set 18 : [LayoutNG] A different approach to floats... #

Patch Set 19 : [LayoutNG] A different approach to floats... #

Patch Set 20 : ./ #

Total comments: 18

Patch Set 21 : address eae comments. #

Patch Set 22 : rebase. #

Patch Set 23 : rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+456 lines, -246 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 10 chunks +10 lines, -30 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBlockFlow.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 4 chunks +34 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/inline/ng_inline_node.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +9 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +18 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker_test.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +4 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 4 chunks +45 lines, -18 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 25 chunks +243 lines, -117 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 7 chunks +14 lines, -19 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_node.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +19 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_constraint_space.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +9 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +5 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_fragment.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 4 chunks +5 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_fragment_builder.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +10 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_layout_result.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 4 chunks +16 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_layout_result.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +4 lines, -2 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 89 (77 generated)
eae
This is really interesting. Would you mind going into a little bit of detail about ...
3 years, 5 months ago (2017-06-26 18:25:25 UTC) #7
ikilpatrick
On 2017/06/26 18:25:25, eae wrote: > This is really interesting. Would you mind going into ...
3 years, 5 months ago (2017-06-26 19:15:03 UTC) #8
eae
This is starting to look really good, so much simpler than before!
3 years, 5 months ago (2017-07-10 17:55:33 UTC) #55
ikilpatrick
Thanks! This is ready for review now. There are lots of TODOs everywhere, I want ...
3 years, 5 months ago (2017-07-10 22:15:27 UTC) #62
ikilpatrick
Thanks! This is ready for review now. There are lots of TODOs everywhere, I want ...
3 years, 5 months ago (2017-07-10 22:15:29 UTC) #63
eae
This is awesome, got a few comments and suggestions for you but overall I love ...
3 years, 5 months ago (2017-07-10 23:27:50 UTC) #64
ikilpatrick
https://codereview.chromium.org/2954953002/diff/380001/third_party/WebKit/LayoutTests/TestExpectations File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2954953002/diff/380001/third_party/WebKit/LayoutTests/TestExpectations#newcode375 third_party/WebKit/LayoutTests/TestExpectations:375: crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/block-in-inline-float-between-001.xht [ Crash ] On 2017/07/10 23:27:49, eae ...
3 years, 5 months ago (2017-07-11 17:20:41 UTC) #69
eae
Thank you so much! LGTM
3 years, 5 months ago (2017-07-11 22:20:47 UTC) #70
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2954953002/420001
3 years, 5 months ago (2017-07-12 03:52:16 UTC) #77
commit-bot: I haz the power
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/218297)
3 years, 5 months ago (2017-07-12 05:17:53 UTC) #79
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2954953002/440001
3 years, 5 months ago (2017-07-12 16:21:32 UTC) #86
commit-bot: I haz the power
3 years, 5 months ago (2017-07-12 19:50:18 UTC) #89
Message was sent while issue was closed.
Committed patchset #23 (id:440001) as
https://chromium.googlesource.com/chromium/src/+/f905d51bad504b4b43149637760b...

Powered by Google App Engine
This is Rietveld 408576698