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

Issue 2945213004: [LayoutNG] Float behavior simplification before larger patch. (Closed)

Created:
3 years, 6 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] Float behavior simplification before larger patch. This is just an intermediate patch which simplifies some of the float logic. As we won't be "re-parenting" floats away from their actual parents anymore a lot of the complex logic around the offsets becomes simpler. Specifically we won't need the "paint-offset" calculation anymore. This is just an initial patch to reduce the behaviour, it has some regressions, but these should be fixed with the new code. BUG=635619 Review-Url: https://codereview.chromium.org/2945213004 Cr-Commit-Position: refs/heads/master@{#483826} Committed: https://chromium.googlesource.com/chromium/src/+/e65e2d1acb3c3bb037f0183bdd3db4cf35da45c2

Patch Set 1 #

Patch Set 2 : ... #

Patch Set 3 : reduce++ #

Patch Set 4 : more simplifying #

Patch Set 5 : rawr. #

Patch Set 6 : falalala. #

Patch Set 7 : testrun #

Patch Set 8 : try? #

Patch Set 9 : testrun #

Patch Set 10 : testrun #

Patch Set 11 : [LayoutNG] Float behavior simplification before larger patch. #

Total comments: 2

Patch Set 12 : update test expectations. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -100 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +6 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 2 chunks +7 lines, -8 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 4 chunks +16 lines, -27 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_floats_utils.h View 1 2 3 1 chunk +4 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_floats_utils.cc View 1 2 3 4 5 6 7 8 9 9 chunks +24 lines, -19 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_unpositioned_float.h View 1 2 3 4 5 6 7 8 9 3 chunks +15 lines, -44 lines 0 comments Download

Messages

Total messages: 48 (42 generated)
ikilpatrick
3 years, 5 months ago (2017-06-29 22:13:53 UTC) #30
eae
I like it, thanks for working on this! https://codereview.chromium.org/2945213004/diff/200001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc (right): https://codereview.chromium.org/2945213004/diff/200001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc#newcode108 third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:108: container_builder->MutableUnpositionedFloats().clear(); ...
3 years, 5 months ago (2017-06-29 22:21:18 UTC) #33
ikilpatrick
https://codereview.chromium.org/2945213004/diff/200001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc (right): https://codereview.chromium.org/2945213004/diff/200001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc#newcode108 third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:108: container_builder->MutableUnpositionedFloats().clear(); On 2017/06/29 22:21:18, eae wrote: > Exposing the ...
3 years, 5 months ago (2017-06-29 22:34:42 UTC) #34
eae
On 2017/06/29 22:34:42, ikilpatrick wrote: > https://codereview.chromium.org/2945213004/diff/200001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc > File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc > (right): > > https://codereview.chromium.org/2945213004/diff/200001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc#newcode108 ...
3 years, 5 months ago (2017-06-30 19:19:02 UTC) #40
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/2945213004/220001
3 years, 5 months ago (2017-06-30 20:53:01 UTC) #45
commit-bot: I haz the power
3 years, 5 months ago (2017-06-30 21:23:51 UTC) #48
Message was sent while issue was closed.
Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/e65e2d1acb3c3bb037f0183bdd3d...

Powered by Google App Engine
This is Rietveld 408576698