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

Issue 2446243003: [LayoutNG] Add NGConstraintSpaceBuilder class. (Closed)

Created:
4 years, 1 month ago by ikilpatrick
Modified:
4 years, 1 month ago
Reviewers:
cbiesinger, Gleb Lanbin, eae
CC:
chromium-reviews, cbiesinger, ojan+watch_chromium.org, szager+layoutwatch_chromium.org, glebl+reviews_chromium.org, zoltan1, blink-reviews-layout_chromium.org, pdr+renderingwatchlist_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[LayoutNG] Add NGConstraintSpaceBuilder class. This patch just introduces the builder without changing the API surface of the constraint space. Once everything is moved to using the builder; I'll start changing the API surface of the constraint space to have PercentageSize() etc. BUG=635619 Committed: https://crrev.com/b1409531577199ba07628761ac4c00385177c850 Cr-Commit-Position: refs/heads/master@{#427769}

Patch Set 1 #

Patch Set 2 : . #

Total comments: 14

Patch Set 3 : address comments. #

Total comments: 2

Patch Set 4 : it's -> its \o/ #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+149 lines, -0 lines) Patch
M third_party/WebKit/Source/core/layout/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h View 1 2 3 1 chunk +53 lines, -0 lines 2 comments Download
A third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.cc View 1 chunk +66 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_physical_constraint_space.h View 1 chunk +10 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_physical_constraint_space.cc View 1 chunk +18 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (8 generated)
ikilpatrick
4 years, 1 month ago (2016-10-25 16:49:16 UTC) #2
eae
https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h File third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h (right): https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h#newcode13 third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h:13: class CORE_EXPORT NGConstraintSpaceBuilder final Should this be PhysicalConstraintSpaceBuilder? Also ...
4 years, 1 month ago (2016-10-25 17:26:27 UTC) #3
Gleb Lanbin
just a couple of minor comments about the style. https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.cc File third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.cc (right): https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.cc#newcode56 third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.cc:56: ...
4 years, 1 month ago (2016-10-25 17:37:27 UTC) #4
cbiesinger
I'll leave the detailed review to Emil and Gleb but this lgtm in principle. https://codereview.chromium.org/2446243003/diff/40001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h ...
4 years, 1 month ago (2016-10-25 19:53:07 UTC) #5
ikilpatrick
https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h File third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h (right): https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h#newcode13 third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h:13: class CORE_EXPORT NGConstraintSpaceBuilder final On 2016/10/25 17:26:27, eae wrote: ...
4 years, 1 month ago (2016-10-25 23:20:43 UTC) #6
eae
On 2016/10/25 23:20:43, ikilpatrick wrote: > https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h > File third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h > (right): > > https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h#newcode13 ...
4 years, 1 month ago (2016-10-25 23:54:47 UTC) #7
ikilpatrick
On 2016/10/25 23:54:47, eae wrote: > On 2016/10/25 23:20:43, ikilpatrick wrote: > > > https://codereview.chromium.org/2446243003/diff/20001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h ...
4 years, 1 month ago (2016-10-26 16:15:42 UTC) #8
Gleb Lanbin
https://codereview.chromium.org/2446243003/diff/60001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h File third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h (right): https://codereview.chromium.org/2446243003/diff/60001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h#newcode19 third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h:19: NGConstraintSpaceBuilder& SetFixedSize(bool fixed_inline, bool fixed_block); you don't need to ...
4 years, 1 month ago (2016-10-26 18:22:15 UTC) #11
ikilpatrick
https://codereview.chromium.org/2446243003/diff/60001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h File third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h (right): https://codereview.chromium.org/2446243003/diff/60001/third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h#newcode19 third_party/WebKit/Source/core/layout/ng/ng_constraint_space_builder.h:19: NGConstraintSpaceBuilder& SetFixedSize(bool fixed_inline, bool fixed_block); On 2016/10/26 18:22:14, glebl ...
4 years, 1 month ago (2016-10-26 18:56:35 UTC) #14
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/2446243003/60001
4 years, 1 month ago (2016-10-26 18:57:53 UTC) #17
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 1 month ago (2016-10-26 19:04:42 UTC) #18
commit-bot: I haz the power
4 years, 1 month ago (2016-10-26 19:22:21 UTC) #20
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/b1409531577199ba07628761ac4c00385177c850
Cr-Commit-Position: refs/heads/master@{#427769}

Powered by Google App Engine
This is Rietveld 408576698