Simplify range mapping computation in SVGTextQuery/SVGInlineTextBox

Simplify range mapping computation in SVGTextQuery/SVGInlineTextBox

Reduce SVGInlineTextBox::mapStartEndPositionsIntoFragmentCoordinates to
a simple offset/base adjustment and an intersection (which is what it
did - just in an unnecessarily verbose way.)

In SVGTextQuery::mapStartEndPositionsIntoFragmentCoordinates, drop the
intersection code, and leave that to the SVGInlineTextBox method.
Move the call to modifyStartEndPositionsRespectingLigatures() after the
intersection, because we don't want to do any adjustments on an empty
range. This is ok because modifyStartEndPositionsRespectingLigatures()
should never expand the range outside of the fragment it operating on.
Moving the call means it needs to be adjusted to work on offsets within
the fragment, so do that.

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

[fs, 2015-03-31 13:06:05]

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

[fs, 2015-04-01 08:57:09]

This diff looks more intimidating that it is - I swear! =)

[Erik Dahlström (inactive), 2015-04-01 14:04:21]

https://codereview.chromium.org/1049673002/diff/1/Source/core/layout/svg/line...
File Source/core/layout/svg/line/SVGInlineTextBox.cpp (right):

https://codereview.chromium.org/1049673002/diff/1/Source/core/layout/svg/line...
Source/core/layout/svg/line/SVGInlineTextBox.cpp:207: // Compute positions
relative to the fragment.
This used to leave startPosition and endPosition untouched in some cases, are
the callers of this method expecting that the values will now potentially always
change?

[fs, 2015-04-01 14:16:25]

https://codereview.chromium.org/1049673002/diff/1/Source/core/layout/svg/line...
File Source/core/layout/svg/line/SVGInlineTextBox.cpp (right):

https://codereview.chromium.org/1049673002/diff/1/Source/core/layout/svg/line...
Source/core/layout/svg/line/SVGInlineTextBox.cpp:207: // Compute positions
relative to the fragment.
On 2015/04/01 14:04:20, Erik Dahlström wrote:
> This used to leave startPosition and endPosition untouched in some cases, are
> the callers of this method expecting that the values will now potentially
always
> change?

The common pattern in callers are:

int startPos = fullStartPos;
int endPos = fullEndPos;
if (!mapStartEndPositionsIntoFragmentCoordinates(fragment, startPos, endPos))
    continue;
<use startPos, endPos>

the single user where that's not the case instead uses the returned bool (and a
"range empty" test). So, no, there's no caller that uses the values for anything
if false was returned.

[Erik Dahlström (inactive), 2015-04-01 14:53:02]

On 2015/04/01 14:16:25, fs wrote:
>
https://codereview.chromium.org/1049673002/diff/1/Source/core/layout/svg/line...
> File Source/core/layout/svg/line/SVGInlineTextBox.cpp (right):
> 
>
https://codereview.chromium.org/1049673002/diff/1/Source/core/layout/svg/line...
> Source/core/layout/svg/line/SVGInlineTextBox.cpp:207: // Compute positions
> relative to the fragment.
> On 2015/04/01 14:04:20, Erik Dahlström wrote:
> > This used to leave startPosition and endPosition untouched in some cases,
are
> > the callers of this method expecting that the values will now potentially
> always
> > change?
> 
> The common pattern in callers are:
> 
> int startPos = fullStartPos;
> int endPos = fullEndPos;
> if (!mapStartEndPositionsIntoFragmentCoordinates(fragment, startPos, endPos))
>     continue;
> <use startPos, endPos>
> 
> the single user where that's not the case instead uses the returned bool (and
a
> "range empty" test). So, no, there's no caller that uses the values for
anything
> if false was returned.

Ok, having looked at the callers I think this is fine. LGTM.

[fs, 2015-04-02 08:18:25]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2015-04-02 08:18:42]

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

[commit-bot: I haz the power, 2015-04-02 09:39:49]

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