Merge HarfBuzz kerning support from Blink

Merge HarfBuzz kerning support from Blink

Original CL: https://codereview.chromium.org/69513002

BUG=321868
R=msw@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273181

[msw, 2014-05-27 22:00:58]

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
ui/gfx/render_text_harfbuzz.cc:129: hb_position_t
GetGlyphHorizontalKerning(hb_font_t*,
nit: give names to even unused parameters (this and last void*).

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
ui/gfx/render_text_harfbuzz.cc:142: int32_t kerningAdjustments[1] = { 0 };
nit: |kerning_adjustments|

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
ui/gfx/render_text_harfbuzz.cc:160: if (!font_data->paint_.isVerticalText()) {
This is the only logical difference from the function above. I understand that
parity with Blink is valuable, but perhaps we could have both of our functions
call a common helper? (Blink could do the same, but that's not in scope)

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
ui/gfx/render_text_harfbuzz.cc:167: int32_t kerningAdjustments[1] = { 0 };
ditto nit: |kerning_adjustments|

[ckocagil, 2014-05-28 01:07:09]

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
ui/gfx/render_text_harfbuzz.cc:129: hb_position_t
GetGlyphHorizontalKerning(hb_font_t*,
On 2014/05/27 22:00:59, msw wrote:
> nit: give names to even unused parameters (this and last void*).

Done.

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
ui/gfx/render_text_harfbuzz.cc:142: int32_t kerningAdjustments[1] = { 0 };
On 2014/05/27 22:00:59, msw wrote:
> nit: |kerning_adjustments|

Done.

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
ui/gfx/render_text_harfbuzz.cc:160: if (!font_data->paint_.isVerticalText()) {
On 2014/05/27 22:00:59, msw wrote:
> This is the only logical difference from the function above. I understand that
> parity with Blink is valuable, but perhaps we could have both of our functions
> call a common helper? (Blink could do the same, but that's not in scope)

Done, added a common helper.

https://codereview.chromium.org/290993011/diff/20001/ui/gfx/render_text_harfb...
ui/gfx/render_text_harfbuzz.cc:167: int32_t kerningAdjustments[1] = { 0 };
On 2014/05/27 22:00:59, msw wrote:
> ditto nit: |kerning_adjustments|

Done.

[msw, 2014-05-28 01:12:39]

LGTM

[ckocagil, 2014-05-28 01:13:01]

The CQ bit was checked by ckocagil@chromium.org

[commit-bot: I haz the power, 2014-05-28 01:13:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ckocagil@chromium.org/290993011/40001

[commit-bot: I haz the power, 2014-05-28 05:13:05]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-28 08:40:15]

Change committed as 273181

[sof, 2014-05-28 10:03:39]

Seeing a compilation error with g++-4.7 due to this CL,

../../ui/gfx/render_text_harfbuzz.cc: In function ‘hb_position_t
gfx::{anonymous}::GetGlyphKerning(gfx::{anonymous}::FontData*, hb_codepoint_t,
hb_codepoint_t)’:
../../ui/gfx/render_text_harfbuzz.cc:134:58: error: narrowing conversion of
‘first_glyph’ from ‘hb_codepoint_t {aka unsigned int}’ to ‘const uint16_t {aka
const short unsigned int}’ inside { } is ill-formed in C++11 [-Werror=narrowing]
../../ui/gfx/render_text_harfbuzz.cc:134:58: error: narrowing conversion of
‘second_glyph’ from ‘hb_codepoint_t {aka unsigned int}’ to ‘const uint16_t {aka
const short unsigned int}’ inside { } is ill-formed in C++11 [-Werror=narrowing]

[Michael van Ouwerkerk, 2014-05-28 10:56:25]

A revert of this CL has been created in
https://codereview.chromium.org/299313009/ by mvanouwerkerk@chromium.org.

The reason for reverting is: It broke this build:
http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28amd6...

Sorry for the trouble..