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

Issue 64293008: Wrap CSS length conversion arguments in an object (Closed)

Created:
7 years, 1 month ago by Timothy Loh
Modified:
7 years ago
CC:
blink-reviews, kenneth.christiansen, shans, rjwright, alancutter (OOO until 2018), Mike Lawther (Google), dglazkov+blink, dstockwell, apavlov+blink_chromium.org, darktears, Steve Block, dino_apple.com, Eric Willigers
Base URL:
https://chromium.googlesource.com/chromium/blink@master
Visibility:
Public.

Description

Wrap CSS length conversion arguments in an object This patch introduces a class CSSToLengthConversionData to wrap the data required to convert CSS lengths to Lengths. This simplifies the plumbing that goes on whenever we need to resolve CSS lengths and makes it easier to update the arguments needed for resolving these (in particular adding a RenderView for resolving viewport units at style recalc time; removing the computingFontSize bool also appears possible). Note that the zoom argument, which was previously a float in some places and a double in others is now a float. BUG=322365 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162881

Patch Set 1 #

Patch Set 2 : rebased #

Patch Set 3 : rebased #

Patch Set 4 : --no-find-copies #

Patch Set 5 : make renderstyles getters return refs #

Patch Set 6 : erm.. use DEFINE_STATIC_REF #

Total comments: 14

Patch Set 7 : rebased #

Patch Set 8 : address some comments #

Patch Set 9 : bool for useEffectiveZoom instead of -1 magic number #

Total comments: 6

Patch Set 10 : comments addressed. #

Patch Set 11 : rebased #

Patch Set 12 : don't make it noncopyable ; clang doesn't do the RVO stuffs? #

Unified diffs Side-by-side diffs Delta from patch set Stats (+353 lines, -232 lines) Patch
M Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/AnimatableLength.h View 1 1 chunk +1 line, -1 line 0 comments Download
M Source/core/animation/AnimatableLength.cpp View 1 chunk +3 lines, -3 lines 0 comments Download
M Source/core/animation/AnimatableLengthTest.cpp View 1 2 3 4 5 6 7 8 8 chunks +19 lines, -14 lines 0 comments Download
M Source/core/core.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/css/BasicShapeFunctions.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/CSSCalculationValue.h View 3 chunks +5 lines, -6 lines 0 comments Download
M Source/core/css/CSSCalculationValue.cpp View 1 2 3 4 5 6 7 8 9 10 5 chunks +13 lines, -13 lines 0 comments Download
M Source/core/css/CSSCalculationValueTest.cpp View 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/css/CSSGradientValue.h View 4 chunks +5 lines, -5 lines 0 comments Download
M Source/core/css/CSSGradientValue.cpp View 15 chunks +32 lines, -39 lines 0 comments Download
M Source/core/css/CSSMatrix.cpp View 1 2 3 4 5 2 chunks +5 lines, -1 line 0 comments Download
M Source/core/css/CSSPrimitiveValue.h View 4 chunks +5 lines, -5 lines 0 comments Download
M Source/core/css/CSSPrimitiveValue.cpp View 1 2 3 4 5 6 7 8 9 10 3 chunks +28 lines, -27 lines 0 comments Download
M Source/core/css/CSSPrimitiveValueMappings.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +4 lines, -5 lines 0 comments Download
A Source/core/css/CSSToLengthConversionData.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +82 lines, -0 lines 0 comments Download
A Source/core/css/CSSToLengthConversionData.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +45 lines, -0 lines 0 comments Download
M Source/core/css/CSSToStyleMap.h View 1 2 3 4 5 6 7 8 9 2 chunks +3 lines, -3 lines 0 comments Download
M Source/core/css/CSSToStyleMap.cpp View 1 2 3 4 5 6 7 8 9 8 chunks +17 lines, -27 lines 0 comments Download
M Source/core/css/MediaQueryEvaluator.cpp View 1 2 3 4 5 6 7 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/css/resolver/AnimatedStyleBuilder.cpp View 1 2 3 4 5 6 1 chunk +4 lines, -6 lines 0 comments Download
M Source/core/css/resolver/FilterOperationResolver.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/css/resolver/FilterOperationResolver.cpp View 6 chunks +7 lines, -11 lines 0 comments Download
M Source/core/css/resolver/FontBuilder.cpp View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -2 lines 0 comments Download
M Source/core/css/resolver/StyleBuilderCustom.h View 2 chunks +4 lines, -6 lines 0 comments Download
M Source/core/css/resolver/StyleBuilderCustom.cpp View 1 2 3 4 5 6 7 8 9 10 23 chunks +34 lines, -35 lines 0 comments Download
M Source/core/css/resolver/StyleResolverState.h View 3 chunks +6 lines, -1 line 0 comments Download
M Source/core/css/resolver/StyleResolverState.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/resolver/TransformBuilder.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/css/resolver/TransformBuilder.cpp View 5 chunks +13 lines, -13 lines 0 comments Download
M Source/core/css/resolver/ViewportStyleResolver.cpp View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 13 (0 generated)
Timothy Loh
7 years ago (2013-11-24 23:37:03 UTC) #1
esprehn
On 2013/11/24 23:37:03, Timothy Loh wrote: You should upload with the no-find-copies flag. It also ...
7 years ago (2013-11-24 23:49:52 UTC) #2
Timothy Loh
On 2013/11/24 23:49:52, esprehn wrote: > On 2013/11/24 23:37:03, Timothy Loh wrote: > > You ...
7 years ago (2013-11-25 00:09:20 UTC) #3
Timothy Loh
On 2013/11/25 00:09:20, Timothy Loh wrote: > I wanted to do this (or at least ...
7 years ago (2013-11-25 02:23:37 UTC) #4
Julien - ping for review
https://codereview.chromium.org/64293008/diff/340001/Source/core/css/CSSToLengthConversionData.cpp File Source/core/css/CSSToLengthConversionData.cpp (right): https://codereview.chromium.org/64293008/diff/340001/Source/core/css/CSSToLengthConversionData.cpp#newcode45 Source/core/css/CSSToLengthConversionData.cpp:45: ASSERT(zoom == -1 || zoom > 0); I don't ...
7 years ago (2013-11-26 05:31:06 UTC) #5
Timothy Loh
https://codereview.chromium.org/64293008/diff/340001/Source/core/css/CSSToLengthConversionData.cpp File Source/core/css/CSSToLengthConversionData.cpp (right): https://codereview.chromium.org/64293008/diff/340001/Source/core/css/CSSToLengthConversionData.cpp#newcode45 Source/core/css/CSSToLengthConversionData.cpp:45: ASSERT(zoom == -1 || zoom > 0); On 2013/11/26 ...
7 years ago (2013-11-27 05:44:50 UTC) #6
alancutter (OOO until 2018)
lgtm, this is a very welcome step towards taming the ugliness around resolving CSS lengths. ...
7 years ago (2013-11-27 09:30:58 UTC) #7
Timothy Loh
https://codereview.chromium.org/64293008/diff/420001/Source/core/css/CSSToLengthConversionData.cpp File Source/core/css/CSSToLengthConversionData.cpp (right): https://codereview.chromium.org/64293008/diff/420001/Source/core/css/CSSToLengthConversionData.cpp#newcode55 Source/core/css/CSSToLengthConversionData.cpp:55: } On 2013/11/27 09:30:58, alancutter wrote: > It probably ...
7 years ago (2013-11-28 03:41:53 UTC) #8
dstockwell
rslgtm
7 years ago (2013-11-29 03:54:36 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/timloh@chromium.org/64293008/460001
7 years ago (2013-11-29 03:58:33 UTC) #10
commit-bot: I haz the power
Retried try job too often on mac_layout for step(s) webkit_lint http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout&number=14597
7 years ago (2013-11-29 04:42:15 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/timloh@chromium.org/64293008/480001
7 years ago (2013-11-29 07:03:44 UTC) #12
commit-bot: I haz the power
7 years ago (2013-11-29 09:57:21 UTC) #13
Message was sent while issue was closed.
Change committed as 162881

Powered by Google App Engine
This is Rietveld 408576698