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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1376573004: Split out Color from CSSPrimitiveValue (Closed)

Created:
5 years, 2 months ago by sashab
Modified:
5 years, 2 months ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-animation_chromium.org, blink-reviews-css, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, Eric Willigers, rjwright, rwlbuis, shans, sof
Base URL:
https://chromium.googlesource.com/chromium/src.git@split_property
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Split out Color from CSSPrimitiveValue Split out Color from CSSPrimitiveValue, into a separate class CSSColorValue, and update callsites. BUG=523893 Committed: https://crrev.com/e3cb70c1e5d814cd28ec8ba3a124925207f2a0dc Cr-Commit-Position: refs/heads/master@{#354725}

Patch Set 1 #

Total comments: 6

Patch Set 2 : Moved all to header file and review feedback #

Patch Set 3 : Fix small bug in StyleBuilderFunctions #

Patch Set 4 : Rebase #

Total comments: 2

Patch Set 5 : Rebase #

Patch Set 6 : Rebase and review feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+173 lines, -135 lines) Patch
M third_party/WebKit/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl View 1 2 3 4 5 1 chunk +7 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/core/animation/ColorInterpolationType.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/animation/ColorStyleInterpolation.h View 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/ColorStyleInterpolation.cpp View 5 chunks +8 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/core/animation/ColorStyleInterpolationTest.cpp View 3 chunks +16 lines, -15 lines 0 comments Download
M third_party/WebKit/Source/core/animation/DeferredLegacyStyleInterpolation.cpp View 2 chunks +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/ListStyleInterpolationTest.cpp View 3 chunks +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/animation/ShadowStyleInterpolation.cpp View 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/animation/ShadowStyleInterpolationTest.cpp View 5 chunks +5 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/core.gypi View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSCalculationValue.cpp View 1 chunk +0 lines, -1 line 0 comments Download
A third_party/WebKit/Source/core/css/CSSColorValue.h View 1 2 3 4 5 1 chunk +52 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSGradientValue.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/CSSGradientValue.cpp View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSPrimitiveValue.h View 8 chunks +0 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSPrimitiveValue.cpp View 1 2 3 4 5 5 chunks +0 lines, -15 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSShadowValue.h View 3 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSShadowValue.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/CSSValue.h View 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSValue.cpp View 6 chunks +14 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSValuePool.h View 3 chunks +6 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSValuePool.cpp View 3 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.h View 2 chunks +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp View 1 2 3 4 3 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSParser.cpp View 2 chunks +3 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSPropertyParser.h View 1 2 3 4 5 3 chunks +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/LegacyCSSPropertyParser.cpp View 1 2 3 4 5 4 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/FilterOperationResolver.cpp View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp View 1 2 3 4 2 chunks +3 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp View 1 2 3 4 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/core/dom/TextLinkColors.h View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/dom/TextLinkColors.cpp View 1 2 3 4 2 chunks +5 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/editing/EditingStyle.cpp View 3 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLHRElement.cpp View 1 chunk +1 line, -1 line 0 comments Download

Dependent Patchsets:

Issue 1399853005 Patch 1

Messages

Total messages: 17 (4 generated)
sashab
5 years, 2 months ago (2015-10-06 02:08:32 UTC) #2
alancutter (OOO until 2018)
Looks like tests are crashing. https://codereview.chromium.org/1376573004/diff/1/third_party/WebKit/Source/core/css/CSSColorValue.cpp File third_party/WebKit/Source/core/css/CSSColorValue.cpp (right): https://codereview.chromium.org/1376573004/diff/1/third_party/WebKit/Source/core/css/CSSColorValue.cpp#newcode25 third_party/WebKit/Source/core/css/CSSColorValue.cpp:25: } These methods are ...
5 years, 2 months ago (2015-10-07 00:56:22 UTC) #3
sashab
https://codereview.chromium.org/1376573004/diff/1/third_party/WebKit/Source/core/css/CSSColorValue.cpp File third_party/WebKit/Source/core/css/CSSColorValue.cpp (right): https://codereview.chromium.org/1376573004/diff/1/third_party/WebKit/Source/core/css/CSSColorValue.cpp#newcode25 third_party/WebKit/Source/core/css/CSSColorValue.cpp:25: } On 2015/10/07 00:56:22, alancutter wrote: > These methods ...
5 years, 2 months ago (2015-10-07 06:09:20 UTC) #4
alancutter (OOO until 2018)
lgtm with nit. https://codereview.chromium.org/1376573004/diff/60001/third_party/WebKit/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl File third_party/WebKit/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl (right): https://codereview.chromium.org/1376573004/diff/60001/third_party/WebKit/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl#newcode520 third_party/WebKit/Source/build/scripts/templates/StyleBuilderFunctions.cpp.tmpl:520: } else if (value->isPrimitiveValue() && toCSSPrimitiveValue(value)->getValueID() ...
5 years, 2 months ago (2015-10-07 06:34:25 UTC) #5
sashab
timloh please review. Sorry this patch is so large -- most of it is small ...
5 years, 2 months ago (2015-10-12 23:04:50 UTC) #7
Timothy Loh
This is OK but I'm not sure about the naming. CSSColorValue is misleading because it ...
5 years, 2 months ago (2015-10-13 05:42:51 UTC) #8
alancutter (OOO until 2018)
On 2015/10/13 at 05:42:51, timloh wrote: > This is OK but I'm not sure about ...
5 years, 2 months ago (2015-10-13 06:29:00 UTC) #9
sashab
Done. Added a TODO to make value() return a Color, then implemented it in a ...
5 years, 2 months ago (2015-10-15 01:50:49 UTC) #10
sashab
ping tim
5 years, 2 months ago (2015-10-18 22:36:32 UTC) #11
Timothy Loh
On 2015/10/18 22:36:32, sashab wrote: > ping tim ok lgtm
5 years, 2 months ago (2015-10-19 02:53:04 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1376573004/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1376573004/100001
5 years, 2 months ago (2015-10-19 02:56:14 UTC) #15
commit-bot: I haz the power
Committed patchset #6 (id:100001)
5 years, 2 months ago (2015-10-19 05:01:49 UTC) #16
commit-bot: I haz the power
5 years, 2 months ago (2015-10-19 05:02:29 UTC) #17
Message was sent while issue was closed. 
Patchset 6 (id:??) landed as
https://crrev.com/e3cb70c1e5d814cd28ec8ba3a124925207f2a0dc
Cr-Commit-Position: refs/heads/master@{#354725}

Powered by Google App Engine
This is Rietveld 408576698