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

Issue 2645613002: [LayoutNG] Remove state machine from min/max content method. (Closed)

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

Description

[LayoutNG] Remove state machine from min/max content method. I'll remove the non-"Sync" version of the methods on NGBlockNode in a separate patch. BUG=635619 Review-Url: https://codereview.chromium.org/2645613002 Cr-Commit-Position: refs/heads/master@{#444651} Committed: https://chromium.googlesource.com/chromium/src/+/8e9dde1be1c2411de2a8d7a7b7c331d46c324a3f

Patch Set 1 #

Patch Set 2 : update comments. #

Patch Set 3 : Add DCHECK. #

Total comments: 4

Patch Set 4 : address comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -87 lines) Patch
M third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.h View 1 2 3 3 chunks +1 line, -4 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc View 1 2 3 4 chunks +24 lines, -46 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc View 1 2 3 1 chunk +1 line, -8 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_node.h View 1 2 3 2 chunks +0 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_block_node.cc View 1 2 3 4 chunks +3 lines, -19 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ng/ng_layout_algorithm.h View 1 1 chunk +6 lines, -8 lines 0 comments Download

Messages

Total messages: 12 (7 generated)
ikilpatrick
3 years, 11 months ago (2017-01-18 18:07:21 UTC) #4
cbiesinger
lgtm https://codereview.chromium.org/2645613002/diff/40001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc (right): https://codereview.chromium.org/2645613002/diff/40001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc#newcode62 third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:62: DCHECK(algorithm.ComputeMinAndMaxContentSizes(&sizes)); Not super-happy with DCHECK in a test ...
3 years, 11 months ago (2017-01-18 18:42:56 UTC) #5
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/2645613002/60001
3 years, 11 months ago (2017-01-19 03:33:28 UTC) #8
ikilpatrick
https://codereview.chromium.org/2645613002/diff/40001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc (right): https://codereview.chromium.org/2645613002/diff/40001/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc#newcode62 third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm_test.cc:62: DCHECK(algorithm.ComputeMinAndMaxContentSizes(&sizes)); On 2017/01/18 18:42:56, cbiesinger wrote: > Not super-happy ...
3 years, 11 months ago (2017-01-19 03:34:09 UTC) #9
commit-bot: I haz the power
3 years, 11 months ago (2017-01-19 05:18:06 UTC) #12
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/8e9dde1be1c2411de2a8d7a7b7c3...

Powered by Google App Engine
This is Rietveld 408576698