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

Issue 1439793003: SVG: Promote d to a property (Closed)

Created:
5 years, 1 month ago by Eric Willigers
Modified:
5 years ago
Reviewers:
haraken, pdr., Ilya Sherman, fs
CC:
darktears, apavlov+blink_chromium.org, asvitkine+watch_chromium.org, blink-reviews, blink-reviews-animation_chromium.org, blink-reviews-css, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, krit, Eric Willigers, f(malita), gyuyoung2, kouhei+svg_chromium.org, pdr+svgwatchlist_chromium.org, rjwright, rwlbuis, Stephen Chennney, shans
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

SVG: Promote d to a property See SVG Working Group Action http://www.w3.org/Graphics/SVG/WG/track/actions/3821 BUG=535429 Committed: https://crrev.com/c0f950e6701367e996f94d379f36c2109adab8ee Cr-Commit-Position: refs/heads/master@{#365163}

Patch Set 1 : stream #

Patch Set 2 : with-css-animation #

Patch Set 3 : without animations #

Patch Set 4 : CSSPropertyParser #

Total comments: 28

Patch Set 5 : CSSPathValue #

Patch Set 6 : RefPtrWillBePersistent #

Total comments: 15

Patch Set 7 : cache Path #

Patch Set 8 : parser #

Total comments: 7

Patch Set 9 : DECLARE_VIRTUAL_TRACE #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+313 lines, -118 lines) Patch
A third_party/WebKit/LayoutTests/css-parser/d-parsing.html View 1 2 3 4 5 6 7 1 chunk +11 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-listing-expected.txt View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-listing-expected.txt View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/css/css-d.svg View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/css/css-d-expected.svg View 1 1 chunk +8 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/svg/css/getComputedStyle-listing-expected.txt View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/css/inherit-d.svg View 1 2 3 4 1 chunk +29 lines, -0 lines 0 comments Download
A + third_party/WebKit/LayoutTests/svg/css/inherit-d-expected.svg View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download
A third_party/WebKit/LayoutTests/svg/css/path-element.html View 1 2 3 4 1 chunk +25 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/css-properties-as-js-properties-expected.txt View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/animation/PathSVGInterpolation.cpp View 1 2 3 4 2 chunks +3 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSComputedStyleDeclaration.cpp View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSPathValue.h View 1 2 3 4 5 6 2 chunks +14 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSPathValue.cpp View 1 2 3 4 5 6 2 chunks +26 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSProperties.in View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp View 1 2 3 4 5 6 2 chunks +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp View 1 2 3 4 5 6 7 3 chunks +17 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/LegacyCSSPropertyParser.cpp View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleBuilderConverter.cpp View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/UseCounter.cpp View 1 2 3 4 5 6 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyle.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/style/SVGComputedStyle.h View 1 2 3 4 5 6 3 chunks +7 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/style/SVGComputedStyle.cpp View 1 2 3 4 2 chunks +3 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/style/SVGComputedStyleDefs.h View 1 2 3 4 5 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/style/SVGComputedStyleDefs.cpp View 1 2 3 4 5 3 chunks +5 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGPath.h View 1 2 3 4 5 6 7 8 3 chunks +9 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGPath.cpp View 1 2 3 4 5 6 4 chunks +37 lines, -58 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGPathByteStream.h View 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGPathElement.h View 1 2 3 4 5 6 2 chunks +7 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGPathElement.cpp View 1 2 3 4 5 6 7 8 2 chunks +60 lines, -17 lines 0 comments Download
M third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 2 chunks +16 lines, -1 line 1 comment Download

Messages

Total messages: 37 (18 generated)
Eric Willigers
5 years ago (2015-11-26 05:17:12 UTC) #4
fs
I'm not pdr, but here are my thoughts... First a rant about path representation... So ...
5 years ago (2015-11-26 12:51:40 UTC) #6
pdr.
@fs, I originally thought a string would be optimal (bytestream doesn't actually save much space) ...
5 years ago (2015-12-02 05:36:50 UTC) #7
fs
On 2015/12/02 at 05:36:50, pdr wrote: > @fs, I originally thought a string would be ...
5 years ago (2015-12-02 09:57:56 UTC) #8
Eric Willigers
https://codereview.chromium.org/1439793003/diff/120001/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right): https://codereview.chromium.org/1439793003/diff/120001/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp#newcode1950 third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1950: static PassRefPtrWillBeRawPtr<CSSValue> consumeSVGDPath(CSSParserTokenRange& range) On 2015/11/26 12:51:39, fs wrote: ...
5 years ago (2015-12-10 23:52:53 UTC) #11
fs
This looks pretty good. With some things addressed, we could probably iterate from here (given ...
5 years ago (2015-12-11 13:01:58 UTC) #13
Eric Willigers
https://codereview.chromium.org/1439793003/diff/120001/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp File third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp (right): https://codereview.chromium.org/1439793003/diff/120001/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp#newcode1957 third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp:1957: if (!buildPathFromString(pathString, path) || !range.atEnd()) On 2015/12/11 13:01:58, fs ...
5 years ago (2015-12-14 05:36:46 UTC) #17
fs
This LGTM (w/ some additional nits) for a first iteration. I'm a tad worried that ...
5 years ago (2015-12-14 11:51:40 UTC) #18
pdr.
On 2015/12/14 at 11:51:40, fs wrote: > This LGTM (w/ some additional nits) for a ...
5 years ago (2015-12-14 19:12:06 UTC) #19
Eric Willigers
Lots of thanks. https://codereview.chromium.org/1439793003/diff/280001/third_party/WebKit/Source/core/svg/SVGPath.h File third_party/WebKit/Source/core/svg/SVGPath.h (right): https://codereview.chromium.org/1439793003/diff/280001/third_party/WebKit/Source/core/svg/SVGPath.h#newcode73 third_party/WebKit/Source/core/svg/SVGPath.h:73: DECLARE_TRACE(); On 2015/12/14 11:51:40, fs wrote: ...
5 years ago (2015-12-14 22:18:23 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1439793003/300001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1439793003/300001
5 years ago (2015-12-14 22:21:05 UTC) #23
Eric Willigers
+haraken for platform owners
5 years ago (2015-12-15 02:13:25 UTC) #25
haraken
platform LGTM
5 years ago (2015-12-15 02:21:30 UTC) #26
Ilya Sherman
histograms.xml lgtm, though... https://codereview.chromium.org/1439793003/diff/300001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/1439793003/diff/300001/tools/metrics/histograms/histograms.xml#newcode69328 tools/metrics/histograms/histograms.xml:69328: + <int value="518" label="d"/> I am ...
5 years ago (2015-12-15 02:29:15 UTC) #28
pdr.
On 2015/12/15 at 02:29:15, isherman wrote: > histograms.xml lgtm, though... > > https://codereview.chromium.org/1439793003/diff/300001/tools/metrics/histograms/histograms.xml > File ...
5 years ago (2015-12-15 04:17:03 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/128756) linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, ...
5 years ago (2015-12-15 04:26:36 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1439793003/300001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1439793003/300001
5 years ago (2015-12-15 05:22:55 UTC) #33
commit-bot: I haz the power
Committed patchset #9 (id:300001)
5 years ago (2015-12-15 05:29:49 UTC) #35
commit-bot: I haz the power
5 years ago (2015-12-15 05:30:41 UTC) #37
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/c0f950e6701367e996f94d379f36c2109adab8ee
Cr-Commit-Position: refs/heads/master@{#365163}

Powered by Google App Engine
This is Rietveld 408576698