Rebuild layout attributes on layout instead of on layout tree updates

Rebuild layout attributes on layout instead of on layout tree updates

What layout attributes are used (for a text node; LayoutSVGInlineText),
depends on how many "characters" precedes the node in question.
Layout attributes were updated on insertions and removals on the layout
tree, by find the node to update, and update the surrounding nodes.
It were however trying to depend on the order in which nodes were being
attached, which meant that a sequence of updates could lead to incorrect
layout attribute (indices) being computed. The process per node is also
essentially O(n) (albeit a fairly cheap such.)
Instead of updating on add/remove/update of nodes, just mark the position
data as invalid, and update on the next layout of the <text> root. This
also has the side-effect of simplifying the code quite significantly,
and should avoid repeatedly resolving the layout attribute indices.

Also take the opportunity to pass LayoutSVGText references and simplify
related code a bit.

BUG=405966, 594058
Committed: https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3
Cr-Commit-Position: refs/heads/master@{#385149}

[fs, 2016-04-04 16:19:38]

Description was changed from

==========
Rebuild layout attributes on layout instead of on layout tree updates

What layout attributes are used (for a text node; LayoutSVGInlineText),
depends on how many "characters" precedes the node in question.
Layout attributes were updated on insertions and removals on the layout
tree, by find the node to update, and update the surrounding nodes.
It were however trying to depend on the order in which nodes were being
attached, which meant that a sequence of updates could lead to incorrect
layout attribute (indices) being computed. The process per node is also
essentially O(n) (albeit a fairly cheap such.)
Instead of updating on add/remove/update of nodes, jsut mark the position
data as invalid, and update on the next layout of the <text> root. This
also has the side-effect of simplifying the code quite significantly,
and should avoid repeatedly resolving the layout attribute indices.

Also take the opportunity to pass LayoutSVGText references and simplify
related code a bit.

BUG=405966, 594058
==========

to

==========
Rebuild layout attributes on layout instead of on layout tree updates

What layout attributes are used (for a text node; LayoutSVGInlineText),
depends on how many "characters" precedes the node in question.
Layout attributes were updated on insertions and removals on the layout
tree, by find the node to update, and update the surrounding nodes.
It were however trying to depend on the order in which nodes were being
attached, which meant that a sequence of updates could lead to incorrect
layout attribute (indices) being computed. The process per node is also
essentially O(n) (albeit a fairly cheap such.)
Instead of updating on add/remove/update of nodes, just mark the position
data as invalid, and update on the next layout of the <text> root. This
also has the side-effect of simplifying the code quite significantly,
and should avoid repeatedly resolving the layout attribute indices.

Also take the opportunity to pass LayoutSVGText references and simplify
related code a bit.

BUG=405966, 594058
==========

[fs, 2016-04-04 16:21:06]

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

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

https://codereview.chromium.org/1854123002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp (right):

https://codereview.chromium.org/1854123002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp:228:
m_layoutAttributes.clear();
Should probably just do this directly "on invalidation" - that ought to be a bit
more robust (and make the code even simpler.) Can squeeze that into this, or do
as a follow-up.

[pdr., 2016-04-04 22:33:13]

LGTM

Nice, net -137 LOCC (lines of crazy code).

https://codereview.chromium.org/1854123002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp (right):

https://codereview.chromium.org/1854123002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/svg/LayoutSVGText.cpp:228:
m_layoutAttributes.clear();
On 2016/04/04 at 20:04:13, fs wrote:
> Should probably just do this directly "on invalidation" - that ought to be a
bit more robust (and make the code even simpler.) Can squeeze that into this, or
do as a follow-up.

Followup sounds great

[fs, 2016-04-05 08:09:43]

On 2016/04/04 at 22:33:13, pdr wrote:
...
> Nice, net -137 LOCC (lines of crazy code).

(I thought  that would be LoCO according to the LocoMo model...)

[fs, 2016-04-05 10:37:36]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-04-05 10:37:41]

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

[commit-bot: I haz the power, 2016-04-05 11:29:06]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-04-05 11:30:17]

Description was changed from

==========
Rebuild layout attributes on layout instead of on layout tree updates

What layout attributes are used (for a text node; LayoutSVGInlineText),
depends on how many "characters" precedes the node in question.
Layout attributes were updated on insertions and removals on the layout
tree, by find the node to update, and update the surrounding nodes.
It were however trying to depend on the order in which nodes were being
attached, which meant that a sequence of updates could lead to incorrect
layout attribute (indices) being computed. The process per node is also
essentially O(n) (albeit a fairly cheap such.)
Instead of updating on add/remove/update of nodes, just mark the position
data as invalid, and update on the next layout of the <text> root. This
also has the side-effect of simplifying the code quite significantly,
and should avoid repeatedly resolving the layout attribute indices.

Also take the opportunity to pass LayoutSVGText references and simplify
related code a bit.

BUG=405966, 594058
==========

to

==========
Rebuild layout attributes on layout instead of on layout tree updates

What layout attributes are used (for a text node; LayoutSVGInlineText),
depends on how many "characters" precedes the node in question.
Layout attributes were updated on insertions and removals on the layout
tree, by find the node to update, and update the surrounding nodes.
It were however trying to depend on the order in which nodes were being
attached, which meant that a sequence of updates could lead to incorrect
layout attribute (indices) being computed. The process per node is also
essentially O(n) (albeit a fairly cheap such.)
Instead of updating on add/remove/update of nodes, just mark the position
data as invalid, and update on the next layout of the <text> root. This
also has the side-effect of simplifying the code quite significantly,
and should avoid repeatedly resolving the layout attribute indices.

Also take the opportunity to pass LayoutSVGText references and simplify
related code a bit.

BUG=405966, 594058

Committed: https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3
Cr-Commit-Position: refs/heads/master@{#385149}
==========

[commit-bot: I haz the power, 2016-04-05 11:30:19]

Patchset 1 (id:??) landed as
https://crrev.com/63680ba154b5839ee37ad744caf251ebe41bd3a3
Cr-Commit-Position: refs/heads/master@{#385149}

[pdr., 2016-04-05 18:15:47]

On 2016/04/05 at 08:09:43, fs wrote:
> On 2016/04/04 at 22:33:13, pdr wrote:
> ...
> > Nice, net -137 LOCC (lines of crazy code).
> 
> (I thought  that would be LoCO according to the LocoMo model...)

haha XD