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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1098593002: [css-flexbox] Performance optimization: Only apply min-width: auto when flex-shrinking an item (Closed)

Created:
5 years, 8 months ago by cbiesinger
Modified:
5 years, 8 months ago
CC:
blink-reviews, blink-reviews-rendering, Dominik Röttsches, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

[css-flexbox] Performance optimization: Only apply min-width: auto when flex-shrinking an item min-width: auto will only matter if the size shrunk due to flex-shrink. So we can bypass the expensive computations here in the other cases. BUG=477183 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193971

Patch Set 1 #

Total comments: 2

Patch Set 2 : review comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -6 lines) Patch
M Source/core/layout/LayoutFlexibleBox.h View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/layout/LayoutFlexibleBox.cpp View 1 3 chunks +8 lines, -5 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
cbiesinger
This fixes a 28% regression on our layout tests (see bug)
5 years, 8 months ago (2015-04-17 19:13:49 UTC) #2
leviw_travelin_and_unemployed
LGTM. Not super excited about the boolean params. https://codereview.chromium.org/1098593002/diff/1/Source/core/layout/LayoutFlexibleBox.cpp File Source/core/layout/LayoutFlexibleBox.cpp (right): https://codereview.chromium.org/1098593002/diff/1/Source/core/layout/LayoutFlexibleBox.cpp#newcode908 Source/core/layout/LayoutFlexibleBox.cpp:908: LayoutUnit ...
5 years, 8 months ago (2015-04-17 19:17:40 UTC) #3
cbiesinger
https://codereview.chromium.org/1098593002/diff/1/Source/core/layout/LayoutFlexibleBox.cpp File Source/core/layout/LayoutFlexibleBox.cpp (right): https://codereview.chromium.org/1098593002/diff/1/Source/core/layout/LayoutFlexibleBox.cpp#newcode908 Source/core/layout/LayoutFlexibleBox.cpp:908: LayoutUnit childMinMaxAppliedMainAxisExtent = adjustChildSizeForMinAndMax(*child, childMainAxisExtent, false); On 2015/04/17 19:17:40, ...
5 years, 8 months ago (2015-04-17 19:39:26 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1098593002/20001
5 years, 8 months ago (2015-04-17 19:39:28 UTC) #7
commit-bot: I haz the power
5 years, 8 months ago (2015-04-18 01:59:30 UTC) #8
Message was sent while issue was closed. 
Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193971

Powered by Google App Engine
This is Rietveld 408576698