Determine the right SVGTextMetrics using the correct approach

Source/core/layout/svg/SVGTextQuery.cpp

 /*
  * Copyright (C) Research In Motion Limited 2010-2012. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 402 matching lines @@
 struct ExtentOfCharacterData : SVGTextQuery::Data {
     ExtentOfCharacterData(unsigned queryPosition)
         : position(queryPosition)
     {
     }
 
     unsigned position;
     FloatRect extent;
 };
 
+const SVGTextMetrics& findMetricsForCharacter(const Vector<SVGTextMetrics>& textMetricsValues, const SVGTextFragment& fragment, unsigned startInFragment)
+{
+    // Find the text metrics cell that start at or contain the character at |startInFragment|.
+    unsigned textMetricsOffset = fragment.metricsListOffset;
+    unsigned fragmentOffset = 0;
+    while (fragmentOffset < fragment.length) {
+        const SVGTextMetrics& metrics = textMetricsValues[textMetricsOffset++];
+        unsigned glyphEnd = fragmentOffset + metrics.length();
+        if (startInFragment < glyphEnd)
+            break;
+        fragmentOffset = glyphEnd;
+    }
+    return textMetricsValues[textMetricsOffset - 1];

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

Any chance we'll get an OOB index here?

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

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:56]

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

+}
+
 static inline void calculateGlyphBoundaries(SVGTextQuery::Data* queryData, const SVGTextFragment& fragment, int startPosition, FloatRect& extent)
 {
     float scalingFactor = queryData->textLayoutObject->scalingFactor();
     ASSERT(scalingFactor);
 
     FloatPoint glyphPosition = calculateGlyphPositionWithoutTransform(queryData, fragment, startPosition);
     glyphPosition.move(0, -queryData->textLayoutObject->scaledFont().fontMetrics().floatAscent() / scalingFactor);
     extent.setLocation(glyphPosition);
 
     // Use the SVGTextMetrics computed by SVGTextMetricsBuilder (which spends
     // time attempting to compute more correct glyph bounds already, handling
     // cursive scripts to some degree.)
-    Vector<SVGTextMetrics>& textMetricsValues = queryData->textLayoutObject->layoutAttributes()->textMetricsValues();
-    const SVGTextMetrics& metrics = textMetricsValues[fragment.characterOffset + startPosition];
+    const Vector<SVGTextMetrics>& textMetricsValues = queryData->textLayoutObject->layoutAttributes()->textMetricsValues();
+    const SVGTextMetrics& metrics = findMetricsForCharacter(textMetricsValues, fragment, startPosition);
 
     // TODO(fs): Negative glyph extents seems kind of weird to have, but
     // presently it can occur in some cases (like Arabic.)
     FloatSize glyphSize(std::max<float>(metrics.width(), 0), std::max<float>(metrics.height(), 0));
     extent.setSize(glyphSize);
 
     // If RTL, adjust the starting point to align with the LHS of the glyph bounding box.
     if (!queryData->textBox->isLeftToRightDirection()) {
         if (queryData->isVerticalText)
             extent.move(0, -glyphSize.height());
@@ skipping 82 matching lines @@
 int SVGTextQuery::characterNumberAtPosition(const FloatPoint& position) const
 {
     CharacterNumberAtPositionData data(position);
     if (!executeQuery(&data, &SVGTextQuery::characterNumberAtPositionCallback))
         return -1;
 
     return data.processedCharacters;
 }
 
 }