Guard against invalid glyph shaping results

third_party/WebKit/Source/platform/fonts/shaping/CachingWordShaper.cpp

 /*
  * Copyright (C) 2015 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 105 matching lines @@
     return buffer.getCharacterRange(run.direction(), totalWidth, from, to);
 }
 
 Vector<CharacterRange> CachingWordShaper::individualCharacterRanges(
     const Font* font, const TextRun& run)
 {
     ShapeResultBuffer buffer;
     float totalWidth = shapeResultsForRun(m_shapeCache, font, run, nullptr,
         &buffer);
 
-    return buffer.individualCharacterRanges(run.direction(), totalWidth);
+    auto ranges = buffer.individualCharacterRanges(run.direction(), totalWidth);
+    // The shaper can fail to return glyph metrics for all characters (see
+    // crbug.com/613915) so add empty ranges to ensure all characters have an
+    // associated range.
+    while (ranges.size() < static_cast<unsigned>(run.length()))

[fs, 2016/05/28 14:08:31]

So this will do all the padding "at the end", while I guess there could be
multiple failures in a set of shape results? Or will Harfbuzz fail on the first
so that there'll only ever be one failure? (I.e, ought the character offset be
tracked down in the ShapeResultBuffer methods so that padding can be applied in
a way so that the "fixups" line up with the errors.)

Anyway, this seems unlikely enough that applying a band-aid is hopefully fine,
and potential follow-on errors higher up will be benign.

(Also, should this be CharacterRange(ranges.last().end, ranges.last().end)?
doesn't matter either ATM because we only use width() IIRC.)

[pdr., 2016/05/28 22:58:16]

Great questions.

Ultimately I think this should be fixed down in HarfBuzzShaper.cpp instead of
doing too much work to fixup the results up here. I think the code added in this
patch will still be a useful guard against bad shaping results even after that
is fixed though, since this is the first code that directly uses shape results
which exposed this bug. I've filed https://crbug.com/615661 for the HarfBuzz
fix.

Does that sound reasonable to you?

[fs, 2016/05/29 14:35:55]

Yes, I do prefer less pairs of belts and suspenders. SGTM.

+        ranges.append(CharacterRange(0, 0));
+    return ranges;
 }
 
 }; // namespace blink