Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

We can assume that element() is non-null, and just cast it in the
overriding implementations.
Move the synchronizeAnimatedSVGAttribute(...) calls into actual
attribute collection, so that it applies to all elements in the
inheritance chain.
Also rewrite the lengthy comment, because gradient building has changed
significantly from what it describes, and attribute collection now
precedes the actual Gradient construction. Replicate the new comment to
the similar place for <pattern>s.

BUG=661598
Review-Url: https://codereview.chromium.org/2749593002
Cr-Commit-Position: refs/heads/master@{#456662}
Committed: https://chromium.googlesource.com/chromium/src/+/c9632b65e4048becc539dd103028a01a8d60e55c

[fs, 2017-03-13 16:38:55]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-13 16:39:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2017-03-13 16:41:01]

Description was changed from

==========
Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

We can assume that element() is non-null, and just cast it in the
overriding implementations.
Move the synchronizeAnimatedSVGAttribute(...) calls into actual
attribute collection, so that it applies to all elements in the
inheritance chain.

BUG=661598
==========

to

==========
Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

We can assume that element() is non-null, and just cast it in the
overriding implementations.
Move the synchronizeAnimatedSVGAttribute(...) calls into actual
attribute collection, so that it applies to all elements in the
inheritance chain.
Also drop the lengthy comment, because gradient building has changed
significantly from what it describes, and attribute collection now
precedes the actual Gradient construction.

BUG=661598
==========

[commit-bot: I haz the power, 2017-03-13 18:29:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-13 18:29:19]

Dry run: This issue passed the CQ dry run.

[fs, 2017-03-13 18:34:03]

fs@opera.com changed reviewers:
+ pdr@chromium.org, schenney@chromium.org

[pdr., 2017-03-13 18:36:31]

LGTM

https://codereview.chromium.org/2749593002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp
(left):

https://codereview.chromium.org/2749593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp:55: //
Be sure to synchronize all SVG properties on the gradientElement _before_
Do we have a comment describing the current system anywhere? If not, think you
could add a short one (probably not here, not sure where)?

[fs, 2017-03-13 18:44:47]

https://codereview.chromium.org/2749593002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp
(left):

https://codereview.chromium.org/2749593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp:55: //
Be sure to synchronize all SVG properties on the gradientElement _before_
On 2017/03/13 at 18:36:31, pdr. wrote:
> Do we have a comment describing the current system anywhere? If not, think you
could add a short one (probably not here, not sure where)?

That's a good question, I suspect there is none... I'll see if I can figure
somewhere to put something.

[fs, 2017-03-13 19:52:55]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-13 19:53:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2017-03-13 19:53:46]

Description was changed from

==========
Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

We can assume that element() is non-null, and just cast it in the
overriding implementations.
Move the synchronizeAnimatedSVGAttribute(...) calls into actual
attribute collection, so that it applies to all elements in the
inheritance chain.
Also drop the lengthy comment, because gradient building has changed
significantly from what it describes, and attribute collection now
precedes the actual Gradient construction.

BUG=661598
==========

to

==========
Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

We can assume that element() is non-null, and just cast it in the
overriding implementations.
Move the synchronizeAnimatedSVGAttribute(...) calls into actual
attribute collection, so that it applies to all elements in the
inheritance chain.
Also rewrite the lengthy comment, because gradient building has changed
significantly from what it describes, and attribute collection now
precedes the actual Gradient construction. Replicate the new comment to
the similar place for <pattern>s.

BUG=661598
==========

[fs, 2017-03-13 19:54:45]

https://codereview.chromium.org/2749593002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp
(left):

https://codereview.chromium.org/2749593002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp:55: //
Be sure to synchronize all SVG properties on the gradientElement _before_
On 2017/03/13 at 18:44:47, fs wrote:
> On 2017/03/13 at 18:36:31, pdr. wrote:
> > Do we have a comment describing the current system anywhere? If not, think
you could add a short one (probably not here, not sure where)?
> 
> That's a good question, I suspect there is none... I'll see if I can figure
somewhere to put something.

Kept the new comment in the same place. Added a similar one for the <pattern>
case.

[pdr., 2017-03-13 20:03:08]

On 2017/03/13 at 19:54:45, fs wrote:
>
https://codereview.chromium.org/2749593002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp
(left):
> 
>
https://codereview.chromium.org/2749593002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.cpp:55: //
Be sure to synchronize all SVG properties on the gradientElement _before_
> On 2017/03/13 at 18:44:47, fs wrote:
> > On 2017/03/13 at 18:36:31, pdr. wrote:
> > > Do we have a comment describing the current system anywhere? If not, think
you could add a short one (probably not here, not sure where)?
> > 
> > That's a good question, I suspect there is none... I'll see if I can figure
somewhere to put something.
> 
> Kept the new comment in the same place. Added a similar one for the <pattern>
case.

LGTM! Thanks

[fs, 2017-03-13 22:29:13]

The CQ bit was unchecked by fs@opera.com

[fs, 2017-03-13 22:29:18]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2017-03-13 22:29:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-13 23:30:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-13 23:31:00]

Failed to apply patch for
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h:
While running git apply --index -p1;
  error: patch failed:
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h:57
  error: third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h:
patch does not apply

Patch:      
third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h
Index: third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h
diff --git
a/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h
b/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h
index
63d68c91b30812a2cc35e386a392785aed1d23e4..179bd44b903b9765a558a082ddd9759eb3379af6
100644
--- a/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h
+++ b/third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceGradient.h
@@ -24,7 +24,6 @@
 
 #include "core/layout/svg/LayoutSVGResourcePaintServer.h"
 #include "core/svg/SVGGradientElement.h"
-#include "platform/geometry/FloatRect.h"
 #include "platform/graphics/Gradient.h"
 #include "platform/transforms/AffineTransform.h"
 #include "wtf/HashMap.h"
@@ -57,7 +56,7 @@ class LayoutSVGResourceGradient : public
LayoutSVGResourcePaintServer {
 
   virtual SVGUnitTypes::SVGUnitType gradientUnits() const = 0;
   virtual AffineTransform calculateGradientTransform() const = 0;
-  virtual bool collectGradientAttributes(SVGGradientElement*) = 0;
+  virtual bool collectGradientAttributes() = 0;
   virtual PassRefPtr<Gradient> buildGradient() const = 0;
 
   static GradientSpreadMethod platformSpreadMethodFromSVGType(

[fs, 2017-03-14 08:31:40]

The CQ bit was checked by fs@opera.com

[fs, 2017-03-14 08:31:41]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2749593002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2017-03-14 08:32:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-14 09:58:16]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489480300934410,
"parent_rev": "90f556922c9607672d7d977ebd7390c729c65ca9", "commit_rev":
"c9632b65e4048becc539dd103028a01a8d60e55c"}

[commit-bot: I haz the power, 2017-03-14 09:58:55]

Description was changed from

==========
Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

We can assume that element() is non-null, and just cast it in the
overriding implementations.
Move the synchronizeAnimatedSVGAttribute(...) calls into actual
attribute collection, so that it applies to all elements in the
inheritance chain.
Also rewrite the lengthy comment, because gradient building has changed
significantly from what it describes, and attribute collection now
precedes the actual Gradient construction. Replicate the new comment to
the similar place for <pattern>s.

BUG=661598
==========

to

==========
Remove argument to LayoutSVGResourceGradient::collectGradientAttributes

We can assume that element() is non-null, and just cast it in the
overriding implementations.
Move the synchronizeAnimatedSVGAttribute(...) calls into actual
attribute collection, so that it applies to all elements in the
inheritance chain.
Also rewrite the lengthy comment, because gradient building has changed
significantly from what it describes, and attribute collection now
precedes the actual Gradient construction. Replicate the new comment to
the similar place for <pattern>s.

BUG=661598

Review-Url: https://codereview.chromium.org/2749593002
Cr-Commit-Position: refs/heads/master@{#456662}
Committed:
https://chromium.googlesource.com/chromium/src/+/c9632b65e4048becc539dd103028...
==========

[commit-bot: I haz the power, 2017-03-14 09:58:57]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/c9632b65e4048becc539dd103028...