Move PathTraversalState::m_segmentIndex to SVGPathTraversalStateBuilder

Move PathTraversalState::m_segmentIndex to SVGPathTraversalStateBuilder

The latter is the only one to manipulate the value.

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

[fs, 2015-03-23 13:54:08]

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

[fs, 2015-03-23 13:54:09]

Not a big deal really [1], just looked a bit odd to me...

[1] Since the latter is the only user/thing derived from the former.

[f(malita), 2015-03-23 14:37:06]

I believe the CL is correct, but the current semantics are a bit leaky:
SVGPathTraversalStateBuilder doesn't own the PathTraversalState, so
theoretically the same state could be passed to multiple builders.

At a quick glance, the state always has the same scope as the builder (and is
never used outside of the builder).  Could we make that implicit (relocate the
state into builder instead of a ref to an external object)?  I don't see any
cases where the state outlives the builder so I think that should be trivial.

[fs, 2015-03-23 14:50:15]

On 2015/03/23 14:37:06, f(malita) wrote:
> I believe the CL is correct, but the current semantics are a bit leaky:
> SVGPathTraversalStateBuilder doesn't own the PathTraversalState, so
> theoretically the same state could be passed to multiple builders.

Yes... Maybe at some point someone considered reusing the PathTraversalState to
amortize the cost across multiple calls.
 
> At a quick glance, the state always has the same scope as the builder (and is
> never used outside of the builder).  Could we make that implicit (relocate the
> state into builder instead of a ref to an external object)?  I don't see any
> cases where the state outlives the builder so I think that should be trivial.

Turned the ref into a non-ref.

[f(malita), 2015-03-23 15:00:54]

On 2015/03/23 14:50:15, fs wrote:
> On 2015/03/23 14:37:06, f(malita) wrote:
> > At a quick glance, the state always has the same scope as the builder (and
is
> > never used outside of the builder).  Could we make that implicit (relocate
the
> > state into builder instead of a ref to an external object)?  I don't see any
> > cases where the state outlives the builder so I think that should be
trivial.
> 
> Turned the ref into a non-ref.

Awesome, thanks!

LGTM

[f(malita), 2015-03-23 15:00:58]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2015-03-23 15:01:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1015833004/20001

[fs, 2015-03-23 15:03:29]

On 2015/03/23 15:00:54, f(malita) wrote:
> On 2015/03/23 14:50:15, fs wrote:
> > On 2015/03/23 14:37:06, f(malita) wrote:
> > > At a quick glance, the state always has the same scope as the builder (and
> is
> > > never used outside of the builder).  Could we make that implicit (relocate
> the
> > > state into builder instead of a ref to an external object)?  I don't see
any
> > > cases where the state outlives the builder so I think that should be
> trivial.
> > 
> > Turned the ref into a non-ref.
> 
> Awesome, thanks!
> 
> LGTM

Thanks. I was almost starting to get second thoughts on this one... Don't be
surprised if a new CL shows up soonish that turns this balloon dog into a
balloon unicorns ;-)

[commit-bot: I haz the power, 2015-03-23 16:08:08]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192370