Refactor SVGTextLayoutEngine::currentLogicalCharacterMetrics

Refactor SVGTextLayoutEngine::currentLogicalCharacterMetrics

The two methods currentLogicalCharacterAttributes and
currentLogicalCharacterMetrics on SVGTextLayoutEngine are very
interdependent, since after calling the former, the latter will be
called.
So fold most of the former into the latter, keeping the bits of the
former that advances to the next layout attribute entry, while
renaming it to nextLogicalAttributes.
The methods are also changed from returning a bool and using out-
variables to return the active SVGTextLayoutAttributes structure
instead.

BUG=607906
Committed: https://crrev.com/cdd3f0264799c6af210ce9e7596b803df92d3cea
Cr-Commit-Position: refs/heads/master@{#390736}

[fs, 2016-04-29 13:16:00]

Description was changed from

==========
Refactor SVGTextLayoutEngine::currentLogicalCharacterMetrics

BUG=
==========

to

==========
Refactor SVGTextLayoutEngine::currentLogicalCharacterMetrics

The two methods currentLogicalCharacterAttributes and
currentLogicalCharacterMetrics on SVGTextLayoutEngine are very
interdependent, since after calling the former, the latter will be
called.
So fold most of the former into the latter, keeping the bits of the
former that advances to the next layout attribute entry, while
renaming it to nextLogicalAttributes.
The methods are also changed from returning a bool and using out-
variables to return the active SVGTextLayoutAttributes structure
instead.

BUG=
==========

[fs, 2016-04-29 13:25:48]

Description was changed from

==========
Refactor SVGTextLayoutEngine::currentLogicalCharacterMetrics

The two methods currentLogicalCharacterAttributes and
currentLogicalCharacterMetrics on SVGTextLayoutEngine are very
interdependent, since after calling the former, the latter will be
called.
So fold most of the former into the latter, keeping the bits of the
former that advances to the next layout attribute entry, while
renaming it to nextLogicalAttributes.
The methods are also changed from returning a bool and using out-
variables to return the active SVGTextLayoutAttributes structure
instead.

BUG=
==========

to

==========
Refactor SVGTextLayoutEngine::currentLogicalCharacterMetrics

The two methods currentLogicalCharacterAttributes and
currentLogicalCharacterMetrics on SVGTextLayoutEngine are very
interdependent, since after calling the former, the latter will be
called.
So fold most of the former into the latter, keeping the bits of the
former that advances to the next layout attribute entry, while
renaming it to nextLogicalAttributes.
The methods are also changed from returning a bool and using out-
variables to return the active SVGTextLayoutAttributes structure
instead.

BUG=607906
==========

[fs, 2016-04-29 14:56:24]

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

[Stephen Chennney, 2016-04-29 17:15:28]

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.h (left):

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.h:78: private:
Why does this get removed?

[fs, 2016-04-29 17:35:12]

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.h (left):

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.h:78: private:
On 2016/04/29 at 17:15:28, Stephen Chennney wrote:
> Why does this get removed?

Because there's one like it just less than 20 lines up.

[f(malita), 2016-04-29 18:06:07]

lgtm

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp (right):

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp:280: {
nit: ASSERT(m_layoutAttributesPosition < m_layoutAttributes.size());

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp:299: //
attributes.
nit: ASSERT(m_logicalCharacterOffset <=
logicalAttributes->context()->textLength());

(I think:)

[Stephen Chennney, 2016-04-29 18:07:18]

On 2016/04/29 18:06:07, f(malita) wrote:
> lgtm
> 
>
https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp
(right):
> 
>
https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp:280: {
> nit: ASSERT(m_layoutAttributesPosition < m_layoutAttributes.size());
> 
>
https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp:299: //
> attributes.
> nit: ASSERT(m_logicalCharacterOffset <=
> logicalAttributes->context()->textLength());
> 
> (I think:)

lgtm from me, given fmalita's comments.

[fs, 2016-04-29 18:56:34]

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp (right):

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp:280: {
On 2016/04/29 at 18:06:07, f(malita) wrote:
> nit: ASSERT(m_layoutAttributesPosition < m_layoutAttributes.size());

Added.

https://codereview.chromium.org/1935493002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/SVGTextLayoutEngine.cpp:299: //
attributes.
On 2016/04/29 at 18:06:07, f(malita) wrote:
> nit: ASSERT(m_logicalCharacterOffset <=
logicalAttributes->context()->textLength());
> 
> (I think:)

Yepp. Added.

[fs, 2016-04-29 20:06:13]

The CQ bit was checked by fs@opera.com

[fs, 2016-04-29 20:06:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
schenney@chromium.org, fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/1935493002/#ps40001
(title: "Add ASSERTs")

[commit-bot: I haz the power, 2016-04-29 20:06:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1935493002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1935493002/40001

[commit-bot: I haz the power, 2016-04-29 20:17:12]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-30 17:28:18]

Patchset 3 (id:??) landed as
https://crrev.com/cdd3f0264799c6af210ce9e7596b803df92d3cea
Cr-Commit-Position: refs/heads/master@{#390736}

[commit-bot: I haz the power, 2016-04-30 18:02:10]

Description was changed from

==========
Refactor SVGTextLayoutEngine::currentLogicalCharacterMetrics

The two methods currentLogicalCharacterAttributes and
currentLogicalCharacterMetrics on SVGTextLayoutEngine are very
interdependent, since after calling the former, the latter will be
called.
So fold most of the former into the latter, keeping the bits of the
former that advances to the next layout attribute entry, while
renaming it to nextLogicalAttributes.
The methods are also changed from returning a bool and using out-
variables to return the active SVGTextLayoutAttributes structure
instead.

BUG=607906
==========

to

==========
Refactor SVGTextLayoutEngine::currentLogicalCharacterMetrics

The two methods currentLogicalCharacterAttributes and
currentLogicalCharacterMetrics on SVGTextLayoutEngine are very
interdependent, since after calling the former, the latter will be
called.
So fold most of the former into the latter, keeping the bits of the
former that advances to the next layout attribute entry, while
renaming it to nextLogicalAttributes.
The methods are also changed from returning a bool and using out-
variables to return the active SVGTextLayoutAttributes structure
instead.

BUG=607906

Committed: https://crrev.com/cdd3f0264799c6af210ce9e7596b803df92d3cea
Cr-Commit-Position: refs/heads/master@{#390736}
==========