Determine the right SVGTextMetrics using the correct approach

Determine the right SVGTextMetrics using the correct approach

https://codereview.chromium.org/1041693002/ had a bug with how it
determined the SVGTextMetrics cell to use for a certain character/glyph.
We need to use SVGTextFragment::metricsListOffset and iterate.

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

[Erik Dahlström (inactive), 2015-03-30 12:28:45]

ed@opera.com changed reviewers:
+ ed@opera.com

[Erik Dahlström (inactive), 2015-03-30 12:28:46]

If this was buggy before, adding a test seems like a good thing to do.

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

https://codereview.chromium.org/1045793002/diff/1/Source/core/layout/svg/SVGT...
Source/core/layout/svg/SVGTextQuery.cpp:435: return
textMetricsValues[textMetricsOffset - 1];
Any chance we'll get an OOB index here?

[fs, 2015-03-30 13:26:40]

On 2015/03/30 12:28:46, Erik Dahlström wrote:
> If this was buggy before, adding a test seems like a good thing to do.

Well, this should really have been included in the CL mentioned in the
description... Adding a test is not entirely without complication since it would
seem to require a font with certain properties (ligatures). Maybe whitespace
could do it too...

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

https://codereview.chromium.org/1045793002/diff/1/Source/core/layout/svg/SVGT...
Source/core/layout/svg/SVGTextQuery.cpp:435: return
textMetricsValues[textMetricsOffset - 1];
On 2015/03/30 12:28:45, Erik Dahlström wrote:
> Any chance we'll get an OOB index here?

Only if the invariants don't hold. (And OOB here means RELEASE_ASSERT.) There's
a lot of different ways to write this function - none of them which is great
overall (IMHO).

[fs, 2015-03-30 14:00:57]

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

https://codereview.chromium.org/1045793002/diff/1/Source/core/layout/svg/SVGT...
Source/core/layout/svg/SVGTextQuery.cpp:435: return
textMetricsValues[textMetricsOffset - 1];
On 2015/03/30 13:26:40, fs wrote:
> On 2015/03/30 12:28:45, Erik Dahlström wrote:
> > Any chance we'll get an OOB index here?
> 
> Only if the invariants don't hold. (And OOB here means RELEASE_ASSERT.)
There's
> a lot of different ways to write this function - none of them which is great
> overall (IMHO).

Non-BMP works too. Musical symbols to the rescue! (Test added.)

[fs, 2015-03-30 16:15:23]

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

[Erik Dahlström (inactive), 2015-03-31 08:36:02]

lgtm

[fs, 2015-03-31 09:19:37]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2015-03-31 09:19:48]

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

[commit-bot: I haz the power, 2015-03-31 09:23:01]

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