Clean up SVGPatternElement::collectPatternAttributes

This patch refactors SVGPatternElement::collectPatternAttributes() by extracting
the setPatternAttributes logic into a new function. This patch also changes the
while loop to be easier to understand and similar to collectGradientAttributes.

A similar refactoring was done to collectGradientAttributes in r165358 [1].

[1] https://src.chromium.org/viewvc/blink?view=rev&revision=165358

BUG=N/A

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165427

[gyuyoung-inactive, 2014-01-20 08:45:09]

Stephen, this cl is to do similar thing as r165358. Could you take a look this ?

[pdr., 2014-01-21 00:33:26]

LGTM but please correct the change description to say what's actually happening
(and reference the correct revision: r165358 not r65358).

I would suggest this:
Clean up SVGPatternElement::collectPatternAttributes.

This patch refactors SVGPatternElement::collectPatternAttributes() by extracting
the setPatternAttributes logic into a new function. This patch also changes the
while loop to be easier to understand and similar to collectGradientAttributes.

A similar refactoring was done to collectGradientAttributes in r165358 [1].

[1] https://src.chromium.org/viewvc/blink?view=rev&revision=165358

https://codereview.chromium.org/137533021/diff/1/Source/core/svg/SVGPatternEl...
File Source/core/svg/SVGPatternElement.cpp (right):

https://codereview.chromium.org/137533021/diff/1/Source/core/svg/SVGPatternEl...
Source/core/svg/SVGPatternElement.cpp:221: return;
I actually think this is harder to follow as there's two return points which is
unusual. That said, this makes the function look like collectGradientAttributes
so I'm okay with it.

[commit-bot: I haz the power, 2014-01-21 00:54:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/137533021/1

[commit-bot: I haz the power, 2014-01-21 00:56:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/137533021/1

[gyuyoung-inactive, 2014-01-21 00:59:41]

On 2014/01/21 00:33:26, pdr wrote:
> LGTM but please correct the change description to say what's actually
happening
> (and reference the correct revision: r165358 not r65358).
> 
> I would suggest this:
> Clean up SVGPatternElement::collectPatternAttributes.

Done.
 
> This patch refactors SVGPatternElement::collectPatternAttributes() by
extracting
> the setPatternAttributes logic into a new function. This patch also changes
the
> while loop to be easier to understand and similar to
collectGradientAttributes.
> 
> A similar refactoring was done to collectGradientAttributes in r165358 [1].
> 
> [1] https://src.chromium.org/viewvc/blink?view=rev&revision=165358
> 
>
https://codereview.chromium.org/137533021/diff/1/Source/core/svg/SVGPatternEl...
> File Source/core/svg/SVGPatternElement.cpp (right):
> 
>
https://codereview.chromium.org/137533021/diff/1/Source/core/svg/SVGPatternEl...
> Source/core/svg/SVGPatternElement.cpp:221: return;
> I actually think this is harder to follow as there's two return points which
is
> unusual. That said, this makes the function look like
collectGradientAttributes
> so I'm okay with it.

[commit-bot: I haz the power, 2014-01-21 01:00:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/137533021/1

[pdr., 2014-01-21 01:00:44]

On 2014/01/21 00:59:41, gyuyoung wrote:
> On 2014/01/21 00:33:26, pdr wrote:
> > LGTM but please correct the change description to say what's actually
> happening
> > (and reference the correct revision: r165358 not r65358).
> > 
> > I would suggest this:
> > Clean up SVGPatternElement::collectPatternAttributes.
> 
> Done.
>  
> > This patch refactors SVGPatternElement::collectPatternAttributes() by
> extracting
> > the setPatternAttributes logic into a new function. This patch also changes
> the
> > while loop to be easier to understand and similar to
> collectGradientAttributes.
> > 
> > A similar refactoring was done to collectGradientAttributes in r165358 [1].
> > 
> > [1] https://src.chromium.org/viewvc/blink?view=rev&revision=165358
> > 
> >
>
https://codereview.chromium.org/137533021/diff/1/Source/core/svg/SVGPatternEl...
> > File Source/core/svg/SVGPatternElement.cpp (right):
> > 
> >
>
https://codereview.chromium.org/137533021/diff/1/Source/core/svg/SVGPatternEl...
> > Source/core/svg/SVGPatternElement.cpp:221: return;
> > I actually think this is harder to follow as there's two return points which
> is
> > unusual. That said, this makes the function look like
> collectGradientAttributes
> > so I'm okay with it.

Sorry, I should have been more clear. Can you use this as the full change
description?
---------
Clean up SVGPatternElement::collectPatternAttributes.

This patch refactors SVGPatternElement::collectPatternAttributes() by extracting
the setPatternAttributes logic into a new function. This patch also changes the
while loop to be easier to understand and similar to collectGradientAttributes.

A similar refactoring was done to collectGradientAttributes in r165358 [1].

[1] https://src.chromium.org/viewvc/blink?view=rev&revision=165358
---------

[gyuyoung-inactive, 2014-01-21 01:24:26]

On 2014/01/21 01:00:44, pdr wrote:
> On 2014/01/21 00:59:41, gyuyoung wrote:
> > On 2014/01/21 00:33:26, pdr wrote:
> > > LGTM but please correct the change description to say what's actually
> > happening
> > > (and reference the correct revision: r165358 not r65358).
> > > 
> > > I would suggest this:
> > > Clean up SVGPatternElement::collectPatternAttributes.
> > 
> > Done.
> >  
> > > This patch refactors SVGPatternElement::collectPatternAttributes() by
> > extracting
> > > the setPatternAttributes logic into a new function. This patch also
changes
> > the
> > > while loop to be easier to understand and similar to
> > collectGradientAttributes.
> > > 
> > > A similar refactoring was done to collectGradientAttributes in r165358
[1].
> > > 
> > > [1] https://src.chromium.org/viewvc/blink?view=rev&revision=165358
> > > 
> > >
> >
>
https://codereview.chromium.org/137533021/diff/1/Source/core/svg/SVGPatternEl...
> > > File Source/core/svg/SVGPatternElement.cpp (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/137533021/diff/1/Source/core/svg/SVGPatternEl...
> > > Source/core/svg/SVGPatternElement.cpp:221: return;
> > > I actually think this is harder to follow as there's two return points
which
> > is
> > > unusual. That said, this makes the function look like
> > collectGradientAttributes
> > > so I'm okay with it.
> 
> Sorry, I should have been more clear. Can you use this as the full change
> description?
> ---------
> Clean up SVGPatternElement::collectPatternAttributes.
> 
> This patch refactors SVGPatternElement::collectPatternAttributes() by
extracting
> the setPatternAttributes logic into a new function. This patch also changes
the
> while loop to be easier to understand and similar to
collectGradientAttributes.
> 
> A similar refactoring was done to collectGradientAttributes in r165358 [1].
> 
> [1] https://src.chromium.org/viewvc/blink?view=rev&revision=165358
> ---------

Sorry about that. My mistake. Modified.

[commit-bot: I haz the power, 2014-01-21 01:24:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gyuyoung.kim@samsung.com/137533021/1

[commit-bot: I haz the power, 2014-01-21 03:27:15]

Change committed as 165427