Perform SVG text chunk processing on-the-fly

Perform SVG text chunk processing on-the-fly

Instead of building a vector of chunks, and then process that vector,
process the chunks as they are formed. The chunks themselves are
independent units, so this is reasonably straight-forward. The main
change is that instead of creating and adding an SVGTextChunk to a
vector, the data used to create the chunk is passed to a handler
method. Because of how <textPath> is handled, this handler currently
needs to be a virtual method. Because of that, a SVGTextChunkBuilder
is subclassed to SVGTextPathChunkBuilder for the text-on-path
processing.

Using the PerfTestRunner.measurePageLoadTime harness with
separate-x.svg from the bug give the following results locally:

       Before:     After:
avg    100.5 ms -> 89.5 ms
median 100.5 ms -> 88.7 ms
stdev    2.1 ms ->  3.2 ms

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

[fs, 2015-05-27 14:39:40]

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

[fs, 2015-05-27 14:39:41]

Bombardement!

[f(malita), 2015-05-27 15:19:13]

https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
File Source/core/layout/svg/SVGTextChunkBuilder.cpp (right):

https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
Source/core/layout/svg/SVGTextChunkBuilder.cpp:73: static SVGTextChunk
createTextChunk(const Vector<SVGInlineTextBox*>& lineLayoutBoxes, unsigned
boxStart, unsigned boxCount)
Can we fold this into the SVGTextChunk ctor and just use that instead?

https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
File Source/core/layout/svg/SVGTextChunkBuilder.h (right):

https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
Source/core/layout/svg/SVGTextChunkBuilder.h:47: virtual void
handleTextChunk(const Vector<SVGInlineTextBox*>&, unsigned boxStart, unsigned
boxEnd);
The first thing both impls do is create a chunk based on these explicit args.
Would it make more sense to pass a chunk instead?

  handleTextChunk(const SVGTextChunk&)

then maybe clients could construct those on the fly:

  handleTextChunk(SVGTextChunk(lineLayoutBoxes, lastChunkStartPosition,
boxPosition));

[fs, 2015-05-27 15:28:07]

https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
File Source/core/layout/svg/SVGTextChunkBuilder.h (right):

https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
Source/core/layout/svg/SVGTextChunkBuilder.h:47: virtual void
handleTextChunk(const Vector<SVGInlineTextBox*>&, unsigned boxStart, unsigned
boxEnd);
On 2015/05/27 15:19:13, f(malita) wrote:
> The first thing both impls do is create a chunk based on these explicit args.
> Would it make more sense to pass a chunk instead?
> 
>   handleTextChunk(const SVGTextChunk&)
> 
> then maybe clients could construct those on the fly:
> 
>   handleTextChunk(SVGTextChunk(lineLayoutBoxes, lastChunkStartPosition,
> boxPosition));
>      
> 
> 

I should probably have mentioned this somewhere, but I plan to get rid of
SVGTextChunk [1], and then doing this way means slightly less back-and-forth...
(Next up is killing SVGTextChunk::m_boxes, which means we get
createTextChunk(boxes[boxStart]) instead in the handleTextChunk impls.)

[1] After all, it's just a slice of the full Vector<...> and some properties
that could as well be collected/checked/read when needed instead.

[f(malita), 2015-05-27 15:29:12]

On 2015/05/27 15:28:07, fs wrote:
>
https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
> File Source/core/layout/svg/SVGTextChunkBuilder.h (right):
> 
>
https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
> Source/core/layout/svg/SVGTextChunkBuilder.h:47: virtual void
> handleTextChunk(const Vector<SVGInlineTextBox*>&, unsigned boxStart, unsigned
> boxEnd);
> On 2015/05/27 15:19:13, f(malita) wrote:
> > The first thing both impls do is create a chunk based on these explicit
args.
> > Would it make more sense to pass a chunk instead?
> > 
> >   handleTextChunk(const SVGTextChunk&)
> > 
> > then maybe clients could construct those on the fly:
> > 
> >   handleTextChunk(SVGTextChunk(lineLayoutBoxes, lastChunkStartPosition,
> > boxPosition));
> >      
> > 
> > 
> 
> I should probably have mentioned this somewhere, but I plan to get rid of
> SVGTextChunk [1], and then doing this way means slightly less
back-and-forth...
> (Next up is killing SVGTextChunk::m_boxes, which means we get
> createTextChunk(boxes[boxStart]) instead in the handleTextChunk impls.)
> 
> [1] After all, it's just a slice of the full Vector<...> and some properties
> that could as well be collected/checked/read when needed instead.

SGTM/LGTM

[fs, 2015-05-27 15:31:47]

On 2015/05/27 15:29:12, f(malita) wrote:
> On 2015/05/27 15:28:07, fs wrote:
> >
>
https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
> > File Source/core/layout/svg/SVGTextChunkBuilder.h (right):
> > 
> >
>
https://codereview.chromium.org/1155293005/diff/1/Source/core/layout/svg/SVGT...
> > Source/core/layout/svg/SVGTextChunkBuilder.h:47: virtual void
> > handleTextChunk(const Vector<SVGInlineTextBox*>&, unsigned boxStart,
unsigned
> > boxEnd);
> > On 2015/05/27 15:19:13, f(malita) wrote:
> > > The first thing both impls do is create a chunk based on these explicit
> args.
> > > Would it make more sense to pass a chunk instead?
> > > 
> > >   handleTextChunk(const SVGTextChunk&)
> > > 
> > > then maybe clients could construct those on the fly:
> > > 
> > >   handleTextChunk(SVGTextChunk(lineLayoutBoxes, lastChunkStartPosition,
> > > boxPosition));
> > >      
> > > 
> > > 
> > 
> > I should probably have mentioned this somewhere, but I plan to get rid of
> > SVGTextChunk [1], and then doing this way means slightly less
> back-and-forth...
> > (Next up is killing SVGTextChunk::m_boxes, which means we get
> > createTextChunk(boxes[boxStart]) instead in the handleTextChunk impls.)
> > 
> > [1] After all, it's just a slice of the full Vector<...> and some properties
> > that could as well be collected/checked/read when needed instead.
> 
> SGTM/LGTM

(This is also, indirectly, related to "clearing a path" to be able to fix
crbug.com/375258 - more on that later though...)

[fs, 2015-05-28 07:44:14]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2015-05-28 07:44:20]

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

[commit-bot: I haz the power, 2015-05-28 07:48:12]

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