Commit ea6dba31 authored by Fredrik Söderquist's avatar Fredrik Söderquist Committed by Commit Bot

Align <pattern> and <*Gradient> attribute collection

Currently we stop collection of the pattern attributes if one of the
<pattern>s that is inherited from is not in the layout tree (but still
use the result). For gradients this cause the generation of the paint
server to fail (triggering fallback if available).

So consider a missing LayoutObject the same as a missing element in the
various Collect...Attributes implementations, essentially ignoring the
reference.
This also aligns our behavior with that of Gecko.

Add const qualification to the CollectGradientAttributes() methods.

Bug: 109212

Change-Id: Ie8a22664c4dc37ebc10fb02f18e1176a9d41a0e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/968924Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#743900}
parent dd489d36
...@@ -70,8 +70,7 @@ std::unique_ptr<GradientData> LayoutSVGResourceGradient::BuildGradientData( ...@@ -70,8 +70,7 @@ std::unique_ptr<GradientData> LayoutSVGResourceGradient::BuildGradientData(
// currently working on. Preferably the state validation should have // currently working on. Preferably the state validation should have
// no side-effects though. // no side-effects though.
if (should_collect_gradient_attributes_) { if (should_collect_gradient_attributes_) {
if (!CollectGradientAttributes()) CollectGradientAttributes();
return gradient_data;
should_collect_gradient_attributes_ = false; should_collect_gradient_attributes_ = false;
} }
......
...@@ -47,7 +47,7 @@ class LayoutSVGResourceGradient : public LayoutSVGResourcePaintServer { ...@@ -47,7 +47,7 @@ class LayoutSVGResourceGradient : public LayoutSVGResourcePaintServer {
protected: protected:
virtual SVGUnitTypes::SVGUnitType GradientUnits() const = 0; virtual SVGUnitTypes::SVGUnitType GradientUnits() const = 0;
virtual AffineTransform CalculateGradientTransform() const = 0; virtual AffineTransform CalculateGradientTransform() const = 0;
virtual bool CollectGradientAttributes() = 0; virtual void CollectGradientAttributes() = 0;
virtual scoped_refptr<Gradient> BuildGradient() const = 0; virtual scoped_refptr<Gradient> BuildGradient() const = 0;
static GradientSpreadMethod PlatformSpreadMethodFromSVGType( static GradientSpreadMethod PlatformSpreadMethodFromSVGType(
......
...@@ -33,10 +33,10 @@ LayoutSVGResourceLinearGradient::LayoutSVGResourceLinearGradient( ...@@ -33,10 +33,10 @@ LayoutSVGResourceLinearGradient::LayoutSVGResourceLinearGradient(
LayoutSVGResourceLinearGradient::~LayoutSVGResourceLinearGradient() = default; LayoutSVGResourceLinearGradient::~LayoutSVGResourceLinearGradient() = default;
bool LayoutSVGResourceLinearGradient::CollectGradientAttributes() { void LayoutSVGResourceLinearGradient::CollectGradientAttributes() {
DCHECK(GetElement()); DCHECK(GetElement());
attributes_wrapper_->Set(LinearGradientAttributes()); attributes_wrapper_->Set(LinearGradientAttributes());
return To<SVGLinearGradientElement>(GetElement()) To<SVGLinearGradientElement>(GetElement())
->CollectGradientAttributes(MutableAttributes()); ->CollectGradientAttributes(MutableAttributes());
} }
......
...@@ -47,7 +47,7 @@ class LayoutSVGResourceLinearGradient final : public LayoutSVGResourceGradient { ...@@ -47,7 +47,7 @@ class LayoutSVGResourceLinearGradient final : public LayoutSVGResourceGradient {
AffineTransform CalculateGradientTransform() const override { AffineTransform CalculateGradientTransform() const override {
return Attributes().GradientTransform(); return Attributes().GradientTransform();
} }
bool CollectGradientAttributes() override; void CollectGradientAttributes() override;
scoped_refptr<Gradient> BuildGradient() const override; scoped_refptr<Gradient> BuildGradient() const override;
FloatPoint StartPoint(const LinearGradientAttributes&) const; FloatPoint StartPoint(const LinearGradientAttributes&) const;
......
...@@ -34,10 +34,10 @@ LayoutSVGResourceRadialGradient::LayoutSVGResourceRadialGradient( ...@@ -34,10 +34,10 @@ LayoutSVGResourceRadialGradient::LayoutSVGResourceRadialGradient(
LayoutSVGResourceRadialGradient::~LayoutSVGResourceRadialGradient() = default; LayoutSVGResourceRadialGradient::~LayoutSVGResourceRadialGradient() = default;
bool LayoutSVGResourceRadialGradient::CollectGradientAttributes() { void LayoutSVGResourceRadialGradient::CollectGradientAttributes() {
DCHECK(GetElement()); DCHECK(GetElement());
attributes_wrapper_->Set(RadialGradientAttributes()); attributes_wrapper_->Set(RadialGradientAttributes());
return To<SVGRadialGradientElement>(GetElement()) To<SVGRadialGradientElement>(GetElement())
->CollectGradientAttributes(MutableAttributes()); ->CollectGradientAttributes(MutableAttributes());
} }
......
...@@ -47,7 +47,7 @@ class LayoutSVGResourceRadialGradient final : public LayoutSVGResourceGradient { ...@@ -47,7 +47,7 @@ class LayoutSVGResourceRadialGradient final : public LayoutSVGResourceGradient {
AffineTransform CalculateGradientTransform() const override { AffineTransform CalculateGradientTransform() const override {
return Attributes().GradientTransform(); return Attributes().GradientTransform();
} }
bool CollectGradientAttributes() override; void CollectGradientAttributes() override;
scoped_refptr<Gradient> BuildGradient() const override; scoped_refptr<Gradient> BuildGradient() const override;
FloatPoint CenterPoint(const RadialGradientAttributes&) const; FloatPoint CenterPoint(const RadialGradientAttributes&) const;
......
...@@ -110,8 +110,8 @@ static void SetGradientAttributes(const SVGGradientElement& element, ...@@ -110,8 +110,8 @@ static void SetGradientAttributes(const SVGGradientElement& element,
attributes.SetY2(linear.y2()->CurrentValue()); attributes.SetY2(linear.y2()->CurrentValue());
} }
bool SVGLinearGradientElement::CollectGradientAttributes( void SVGLinearGradientElement::CollectGradientAttributes(
LinearGradientAttributes& attributes) { LinearGradientAttributes& attributes) const {
DCHECK(GetLayoutObject()); DCHECK(GetLayoutObject());
VisitedSet visited; VisitedSet visited;
...@@ -123,12 +123,14 @@ bool SVGLinearGradientElement::CollectGradientAttributes( ...@@ -123,12 +123,14 @@ bool SVGLinearGradientElement::CollectGradientAttributes(
visited.insert(current); visited.insert(current);
current = current->ReferencedElement(); current = current->ReferencedElement();
if (!current || visited.Contains(current))
// Ignore the referenced gradient element if it is not attached.
if (!current || !current->GetLayoutObject())
break;
// Cycle detection.
if (visited.Contains(current))
break; break;
if (!current->GetLayoutObject())
return false;
} }
return true;
} }
bool SVGLinearGradientElement::SelfHasRelativeLengths() const { bool SVGLinearGradientElement::SelfHasRelativeLengths() const {
......
...@@ -35,7 +35,7 @@ class SVGLinearGradientElement final : public SVGGradientElement { ...@@ -35,7 +35,7 @@ class SVGLinearGradientElement final : public SVGGradientElement {
public: public:
explicit SVGLinearGradientElement(Document&); explicit SVGLinearGradientElement(Document&);
bool CollectGradientAttributes(LinearGradientAttributes&); void CollectGradientAttributes(LinearGradientAttributes&) const;
SVGAnimatedLength* x1() const { return x1_.Get(); } SVGAnimatedLength* x1() const { return x1_.Get(); }
SVGAnimatedLength* y1() const { return y1_.Get(); } SVGAnimatedLength* y1() const { return y1_.Get(); }
......
...@@ -266,10 +266,10 @@ void SVGPatternElement::CollectPatternAttributes( ...@@ -266,10 +266,10 @@ void SVGPatternElement::CollectPatternAttributes(
// from that element to override values this pattern didn't set. // from that element to override values this pattern didn't set.
current = current->ReferencedElement(); current = current->ReferencedElement();
// Only consider attached SVG pattern elements. // Ignore the referenced pattern element if it is not attached.
if (!current || !current->GetLayoutObject()) if (!current || !current->GetLayoutObject())
break; break;
// Cycle detection // Cycle detection.
if (processed_patterns.Contains(current)) if (processed_patterns.Contains(current))
break; break;
} }
......
...@@ -130,8 +130,8 @@ static void SetGradientAttributes(const SVGGradientElement& element, ...@@ -130,8 +130,8 @@ static void SetGradientAttributes(const SVGGradientElement& element,
attributes.SetFr(radial.fr()->CurrentValue()); attributes.SetFr(radial.fr()->CurrentValue());
} }
bool SVGRadialGradientElement::CollectGradientAttributes( void SVGRadialGradientElement::CollectGradientAttributes(
RadialGradientAttributes& attributes) { RadialGradientAttributes& attributes) const {
DCHECK(GetLayoutObject()); DCHECK(GetLayoutObject());
VisitedSet visited; VisitedSet visited;
...@@ -143,10 +143,12 @@ bool SVGRadialGradientElement::CollectGradientAttributes( ...@@ -143,10 +143,12 @@ bool SVGRadialGradientElement::CollectGradientAttributes(
visited.insert(current); visited.insert(current);
current = current->ReferencedElement(); current = current->ReferencedElement();
if (!current || visited.Contains(current)) // Ignore the referenced gradient element if it is not attached.
if (!current || !current->GetLayoutObject())
break;
// Cycle detection.
if (visited.Contains(current))
break; break;
if (!current->GetLayoutObject())
return false;
} }
// Handle default values for fx/fy // Handle default values for fx/fy
...@@ -155,8 +157,6 @@ bool SVGRadialGradientElement::CollectGradientAttributes( ...@@ -155,8 +157,6 @@ bool SVGRadialGradientElement::CollectGradientAttributes(
if (!attributes.HasFy()) if (!attributes.HasFy())
attributes.SetFy(attributes.Cy()); attributes.SetFy(attributes.Cy());
return true;
} }
bool SVGRadialGradientElement::SelfHasRelativeLengths() const { bool SVGRadialGradientElement::SelfHasRelativeLengths() const {
......
...@@ -35,7 +35,7 @@ class SVGRadialGradientElement final : public SVGGradientElement { ...@@ -35,7 +35,7 @@ class SVGRadialGradientElement final : public SVGGradientElement {
public: public:
explicit SVGRadialGradientElement(Document&); explicit SVGRadialGradientElement(Document&);
bool CollectGradientAttributes(RadialGradientAttributes&); void CollectGradientAttributes(RadialGradientAttributes&) const;
SVGAnimatedLength* cx() const { return cx_.Get(); } SVGAnimatedLength* cx() const { return cx_.Get(); }
SVGAnimatedLength* cy() const { return cy_.Get(); } SVGAnimatedLength* cy() const { return cy_.Get(); }
......
<svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink"
xmlns:h="http:/www.w3.org/1999/xhtml">
<title>&#x3c;linearGradient&#x3e; (without content) inheriting from a &#x3c;linearGradient&#x3e; in an invalid context</title>
<h:link rel="help" href="https://svgwg.org/svg2-draft/pservers.html#StopNotes"/>
<h:link rel="match" href="reference/green-100x100.svg"/>
<linearGradient id="linearGradient1" xlink:href="#linearGradient0"/>
<rect width="100" height="100" fill="green"/>
<rect width="100" height="100" fill="url(#linearGradient1) yellow"/>
<text>
<linearGradient id="linearGradient0">
<stop stop-color="orange"/>
</linearGradient>
</text>
</svg>
<svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink"
xmlns:h="http:/www.w3.org/1999/xhtml">
<title>&#x3c;linearGradient&#x3e; (with content) inheriting from a &#x3c;linearGradient&#x3e; in an invalid context</title>
<h:link rel="help" href="https://svgwg.org/svg2-draft/pservers.html#StopNotes"/>
<h:link rel="match" href="reference/green-100x100.svg"/>
<linearGradient id="linearGradient1" xlink:href="#linearGradient0">
<stop stop-color="green"/>
</linearGradient>
<rect width="100" height="100" fill="url(#linearGradient1) yellow"/>
<text>
<linearGradient id="linearGradient0">
<stop stop-color="orange"/>
</linearGradient>
</text>
</svg>
<svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink"
xmlns:h="http:/www.w3.org/1999/xhtml">
<title>&#x3c;pattern&#x3e; (without content) inheriting from a &#x3c;pattern&#x3e; in an invalid context</title>
<h:link rel="match" href="reference/green-100x100.svg"/>
<pattern id="pattern1" xlink:href="#pattern0" width="1" height="1"/>
<rect width="100" height="100" fill="url(#pattern1) green"/>
<text>
<pattern id="pattern0">
<rect width="100" height="100" fill="orange"/>
</pattern>
</text>
</svg>
<svg xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink"
xmlns:h="http:/www.w3.org/1999/xhtml">
<title>&#x3c;pattern&#x3e; (with content) inheriting from a &#x3c;pattern&#x3e; in an invalid context</title>
<h:link rel="match" href="reference/green-100x100.svg"/>
<pattern id="pattern1" xlink:href="#pattern0" width="1" height="1">
<rect width="100" height="100" fill="green"/>
</pattern>
<rect width="100" height="100" fill="url(#pattern1) yellow"/>
<text>
<pattern id="pattern0">
<rect width="100" height="100" fill="orange"/>
</pattern>
</text>
</svg>
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg
xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink"
version="1.0"
width="200"
height="200">
<defs>
<linearGradient id="linearGradient1" xlink:href="#linearGradient0" />
</defs>
<rect width="200" height="200" style="fill:url(#linearGradient1) green;" id="rect0" />
<text id="text0" x="10" y="20" style="fill:green">
PASS
<linearGradient id="linearGradient0">
<stop offset="0" stop-color="white"/>
<stop offset="1" stop-color="black"/>
</linearGradient>
</text>
</svg>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment