Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 7002001: Revert 79985 - 2011-03-01 Nikolas Zimmermann <nzimmermann@rim.com> (Closed)

Created:
8 years, 1 month ago by jamesr
Modified:
8 years ago
Reviewers:
jamesr
CC:
chromium-reviews
Base URL:
http://svn.webkit.org/repository/webkit/branches/chromium/742/
Visibility:
Public.

Description

Revert 79985 - 2011-03-01 Nikolas Zimmermann <nzimmermann@rim.com>; Reviewed by Antti Koivisto. SVG 1.1 2nd Edition color-prop-05-t.svg exposes bug in 'currentColor' handling https://bugs.webkit.org/show_bug.cgi?id=54800 Add new layout test from the SVG 1.1 2nd Edition test suite, covering currentColor handling. Add tests ensuring that mutating a previously shared SVGPaint object makes it unique, so that mutations take affect, but don't affect other renderers which shared the SVGPaint object before. * platform/mac/svg/W3C-SVG-1.1-SE/color-prop-05-t-expected.checksum: Added. * platform/mac/svg/W3C-SVG-1.1-SE/color-prop-05-t-expected.png: Added. * platform/mac/svg/W3C-SVG-1.1-SE/color-prop-05-t-expected.txt: Added. * platform/mac/svg/custom/SVGPaint-mutate-attribute-expected.checksum: Added. * platform/mac/svg/custom/SVGPaint-mutate-attribute-expected.png: Added. * platform/mac/svg/custom/SVGPaint-mutate-attribute-expected.txt: Added. * platform/mac/svg/custom/SVGPaint-mutate-inline-style-expected.checksum: Added. * platform/mac/svg/custom/SVGPaint-mutate-inline-style-expected.png: Added. * platform/mac/svg/custom/SVGPaint-mutate-inline-style-expected.txt: Added. * svg/W3C-SVG-1.1-SE/color-prop-05-t.svg: Added. * svg/animations/animate-color-fill-currentColor-expected.txt: Added. * svg/animations/animate-color-fill-currentColor.html: Added. * svg/animations/script-tests/animate-color-fill-currentColor.js: Added. * svg/custom/SVGPaint-mutate-attribute.svg: Added. * svg/custom/SVGPaint-mutate-inline-style.svg: Added. * svg/dom/SVGColor-expected.txt: SVGColor mutations now reflected in style/computed style. * svg/dom/SVGPaint-expected.txt: Same for SVGPaint. 2011-03-01 Nikolas Zimmermann <nzimmermann@rim.com>; Reviewed by Antti Koivisto. SVG 1.1 2nd Edition color-prop-05-t.svg exposes bug in 'currentColor' handling https://bugs.webkit.org/show_bug.cgi?id=54800 Wrong handling of currentColor on inherit https://bugs.webkit.org/show_bug.cgi?id=38102 Stop storing RefPtr<SVGPaint> objects in the SVGRenderStyle for fill/stroke. These are the last two objects that held references to CSSValues, they're all gone now, aligning better with RenderStyle. It's also dangerous, as a SVGPaint object can be shared by multiple SVGRenderStyles (MappedAttribute will once create a CSSStyleDeclaration for fill="red" and reuse it where possible), and it was easy to accidently mutate the object, affecting multiple styles. Instead store a Color, an URI and a paint type in SVGRenderStyle, enough to create a SVGPaint object, if needed (eg for computed styles). <g color="green"><rect fill="currentColor"/> already worked fine in trunk, but <g fill="currentColor" color="green"><rect color="red"/> procuded a red rectangle. In order to fix to bug we have to resolve all currentColor values for SVGPaint objects, in SVGCSSStyleSelector, as it's already done for SVGColor objects (stop-color, flood-color, etc.) instead of in RenderSVGResource::fill/strokePaintingResource, when trying to use the paint server. The correct "color" value that should be used from the RenderStyle, is directly available in CSSStyleSelector: in applyProperty m_style->color() gives the desired value. In CSSStyleSelector it's handled exactly this way for non-SVG currentColor properties. Also fix computed styles, which did not resolve currentColor for SVGPaint/SVGColor. A previous patch implemented the SVGPaint/SVGColor API. SVG demands these CSSValues to be mutable. Introduce CSSMutableValue, which extends CSSValue by a Node pointer, and let SVGPaint/SVGColor inherit from it. Mutating a SVGPaint/SVGColor object now takes immediate effect, which is reflected in the inline style / computed style. (Note that getPresentationAttribute() already takes care of removing the CSSValue from the mapped attribute cache, so that it's no longer shared.) Add several new tests covering the patch. Tests: svg/W3C-SVG-1.1-SE/color-prop-05-t.svg svg/animations/animate-color-fill-currentColor.html svg/custom/SVGPaint-mutate-attribute.svg svg/custom/SVGPaint-mutate-inline-style.svg * GNUMakefile.am: Add CSSMutableValue.h * WebCore.gypi: Ditto. * WebCore.xcodeproj/project.pbxproj: Ditto. * css/CSSMutableStyleDeclaration.cpp: Reset the Node pointer in all CSSMutableValues belonging to this style declaration. (WebCore::CSSMutableStyleDeclaration::~CSSMutableStyleDeclaration): * css/CSSMutableStyleDeclaration.h: Add destructor. * css/CSSMutableValue.h: Added. (WebCore::CSSMutableValue::CSSMutableValue): (WebCore::CSSMutableValue::~CSSMutableValue): (WebCore::CSSMutableValue::isMutableValue): (WebCore::CSSMutableValue::node): (WebCore::CSSMutableValue::setNode): (WebCore::CSSMutableValue::setNeedsStyleRecalc): * css/CSSStyleDeclaration.cpp: (WebCore::CSSStyleDeclaration::getPropertyCSSValue): Set the Node object of a CSSMutableValue to the Node, this style declaration belongs to. * css/CSSValue.h: (WebCore::CSSValue::isMutableValue): Return false, default. * css/SVGCSSComputedStyleDeclaration.cpp: (WebCore::CSSComputedStyleDeclaration::adjustSVGPaintForCurrentColor): Add helper function, resolving currentColor values for SVGPaint objects. (WebCore::CSSComputedStyleDeclaration::getSVGPropertyCSSValue): Use currentColorOrValidColor/adjustSVGPaintForCurrentColor to resolve SVGColor/SVGPaint values. * css/SVGCSSStyleSelector.cpp: (WebCore::CSSStyleSelector::applySVGProperty): Store fill/stroke uri, color, paint type seperated in SVGRenderStyle, don't store the full SVGPaint object anymore. * rendering/style/SVGRenderStyle.cpp: (WebCore::SVGRenderStyle::diff): Adapt to SVGPaint changes. * rendering/style/SVGRenderStyle.h: Ditto. (WebCore::SVGRenderStyle::initialFillOpacity): (WebCore::SVGRenderStyle::initialFillPaintType): (WebCore::SVGRenderStyle::initialFillPaintColor): (WebCore::SVGRenderStyle::initialFillPaintUri): (WebCore::SVGRenderStyle::initialStrokeOpacity): (WebCore::SVGRenderStyle::initialStrokePaintType): (WebCore::SVGRenderStyle::initialStrokePaintColor): (WebCore::SVGRenderStyle::initialStrokePaintUri): (WebCore::SVGRenderStyle::initialStrokeMiterLimit): (WebCore::SVGRenderStyle::initialStopOpacity): (WebCore::SVGRenderStyle::initialFloodOpacity): (WebCore::SVGRenderStyle::setFillPaint): (WebCore::SVGRenderStyle::setStrokePaint): (WebCore::SVGRenderStyle::fillPaintType): (WebCore::SVGRenderStyle::fillPaintColor): (WebCore::SVGRenderStyle::fillPaintUri): (WebCore::SVGRenderStyle::strokePaintType): (WebCore::SVGRenderStyle::strokePaintColor): (WebCore::SVGRenderStyle::strokePaintUri): (WebCore::SVGRenderStyle::hasStroke): (WebCore::SVGRenderStyle::hasFill): * rendering/style/SVGRenderStyleDefs.cpp: Ditto. (WebCore::StyleFillData::StyleFillData): (WebCore::StyleFillData::operator==): (WebCore::StyleStrokeData::StyleStrokeData): (WebCore::StyleStrokeData::operator==): * rendering/style/SVGRenderStyleDefs.h: Ditto. * rendering/svg/RenderSVGResource.cpp: Ditto. (WebCore::requestPaintingResource): * rendering/svg/RenderSVGResourceClipper.cpp: Ditto. (WebCore::RenderSVGResourceClipper::drawContentIntoMaskImage): * rendering/svg/SVGResources.cpp: Ditto. (WebCore::paintingResourceFromSVGPaint): (WebCore::SVGResources::buildCachedResources): * svg/SVGColor.cpp: Call setNeedsStyleRecalc() after mutating the object. (WebCore::SVGColor::setRGBColor): (WebCore::SVGColor::setRGBColorICCColor): (WebCore::SVGColor::setColor): * svg/SVGColor.h: * svg/SVGPaint.cpp: Ditto. (WebCore::SVGPaint::setUri): (WebCore::SVGPaint::setPaint): * svg/SVGPaint.h: TBR=kerz@chromium.org BUG=80880 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86167

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4447 lines, -603 lines) Patch
D LayoutTests/platform/mac/svg/W3C-SVG-1.1-SE/color-prop-05-t-expected.checksum View 1 chunk +0 lines, -1 line 0 comments Download
D LayoutTests/platform/mac/svg/W3C-SVG-1.1-SE/color-prop-05-t-expected.png View Binary file 0 comments Download
D LayoutTests/platform/mac/svg/W3C-SVG-1.1-SE/color-prop-05-t-expected.txt View 1 chunk +0 lines, -13 lines 0 comments Download
D LayoutTests/platform/mac/svg/custom/SVGPaint-mutate-attribute-expected.checksum View 1 chunk +0 lines, -1 line 0 comments Download
D LayoutTests/platform/mac/svg/custom/SVGPaint-mutate-attribute-expected.png View Binary file 0 comments Download
D LayoutTests/platform/mac/svg/custom/SVGPaint-mutate-attribute-expected.txt View 1 chunk +0 lines, -8 lines 0 comments Download
D LayoutTests/platform/mac/svg/custom/SVGPaint-mutate-inline-style-expected.checksum View 1 chunk +0 lines, -1 line 0 comments Download
D LayoutTests/platform/mac/svg/custom/SVGPaint-mutate-inline-style-expected.png View Binary file 0 comments Download
D LayoutTests/platform/mac/svg/custom/SVGPaint-mutate-inline-style-expected.txt View 1 chunk +0 lines, -8 lines 0 comments Download
D LayoutTests/svg/W3C-SVG-1.1-SE/color-prop-05-t.svg View 1 chunk +0 lines, -60 lines 0 comments Download
D LayoutTests/svg/animations/animate-color-fill-currentColor.html View 1 chunk +0 lines, -15 lines 0 comments Download
D LayoutTests/svg/animations/animate-color-fill-currentColor-expected.txt View 1 chunk +0 lines, -44 lines 0 comments Download
D LayoutTests/svg/animations/script-tests/animate-color-fill-currentColor.js View 1 chunk +0 lines, -74 lines 0 comments Download
D LayoutTests/svg/custom/SVGPaint-mutate-attribute.svg View 1 chunk +0 lines, -25 lines 0 comments Download
D LayoutTests/svg/custom/SVGPaint-mutate-inline-style.svg View 1 chunk +0 lines, -25 lines 0 comments Download
M LayoutTests/svg/dom/SVGColor-expected.txt View 3 chunks +3 lines, -3 lines 0 comments Download
M LayoutTests/svg/dom/SVGPaint-expected.txt View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/WebCore/GNUmakefile.am View 28 chunks +4329 lines, -79 lines 0 comments Download
M Source/WebCore/WebCore.gypi View 1 chunk +1 line, -0 lines 0 comments Download
M Source/WebCore/WebCore.xcodeproj/project.pbxproj View 3 chunks +0 lines, -3 lines 0 comments Download
M Source/WebCore/css/CSSComputedStyleDeclaration.h View 3 chunks +2 lines, -6 lines 0 comments Download
M Source/WebCore/css/CSSMutableStyleDeclaration.h View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/WebCore/css/CSSMutableStyleDeclaration.cpp View 2 chunks +0 lines, -12 lines 0 comments Download
D Source/WebCore/css/CSSMutableValue.h View 1 chunk +0 lines, -54 lines 0 comments Download
M Source/WebCore/css/CSSStyleDeclaration.cpp View 2 chunks +1 line, -25 lines 0 comments Download
M Source/WebCore/css/CSSValue.h View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/WebCore/css/SVGCSSComputedStyleDeclaration.cpp View 4 chunks +5 lines, -14 lines 0 comments Download
M Source/WebCore/css/SVGCSSStyleSelector.cpp View 1 chunk +7 lines, -26 lines 0 comments Download
M Source/WebCore/rendering/style/SVGRenderStyle.h View 5 chunks +17 lines, -33 lines 0 comments Download
M Source/WebCore/rendering/style/SVGRenderStyle.cpp View 2 chunks +2 lines, -4 lines 0 comments Download
M Source/WebCore/rendering/style/SVGRenderStyleDefs.h View 5 chunks +6 lines, -8 lines 0 comments Download
M Source/WebCore/rendering/style/SVGRenderStyleDefs.cpp View 3 chunks +23 lines, -21 lines 0 comments Download
M Source/WebCore/rendering/svg/RenderSVGResource.cpp View 1 chunk +18 lines, -21 lines 0 comments Download
M Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/WebCore/rendering/svg/SVGResources.cpp View 3 chunks +7 lines, -4 lines 0 comments Download
M Source/WebCore/svg/SVGColor.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/WebCore/svg/SVGColor.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M Source/WebCore/svg/SVGPaint.h View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/WebCore/svg/SVGPaint.cpp View 2 chunks +14 lines, -2 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
jamesr
8 years, 1 month ago (2011-05-10 17:37:04 UTC) #1

          

Powered by Google App Engine
This is Rietveld 408576698