Makes gfx::FontList carry UNDERLINE flag.

Makes gfx::FontList carry UNDERLINE flag.

I found that gfx::FontList doesn't carry UNDERLINE flag properly.  Even though gfx::RenderText draws underline in a speciall manner, gfx::FontList should properly hold the underline flag.  Otherwise, underline info will be dropped somewhere.

TEST=Run ui_unittests.

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

[Yuki, 2013-12-20 15:01:21]

Could you review this CL?

I found this fix is necessary for clean-up of gfx::Font -> gfx::FontList.

[Alexei Svitkine (slow), 2013-12-20 16:28:09]

https://codereview.chromium.org/119993002/diff/1/ui/gfx/font_list.cc
File ui/gfx/font_list.cc (right):

https://codereview.chromium.org/119993002/diff/1/ui/gfx/font_list.cc#newcode47
ui/gfx/font_list.cc:47: else if (!styles_size[i].compare("Underline"))
So the description string comes from a format used by Pango, etc..

Does that format support "Underline"? If it doesn't, we shouldn't be adding it
to the string or parsing it out of the string.

[Yuki, 2013-12-20 17:34:52]

Thanks for the comment.  Could you take another look?

https://codereview.chromium.org/119993002/diff/1/ui/gfx/font_list.cc
File ui/gfx/font_list.cc (right):

https://codereview.chromium.org/119993002/diff/1/ui/gfx/font_list.cc#newcode47
ui/gfx/font_list.cc:47: else if (!styles_size[i].compare("Underline"))
On 2013/12/20 16:28:09, Alexei Svitkine wrote:
> So the description string comes from a format used by Pango, etc..
> 
> Does that format support "Underline"? If it doesn't, we shouldn't be adding it
> to the string or parsing it out of the string.

You're right.  Since underline is NOT considered as part of font in Pango, we
shouldn't add "Underline".  I've reverted this part.

Instead I found an issue in GetFonts() method, and fixed it.

[Alexei Svitkine (slow), 2013-12-20 17:52:19]

lgtm with a nit

https://codereview.chromium.org/119993002/diff/130001/ui/gfx/font_list.cc
File ui/gfx/font_list.cc (right):

https://codereview.chromium.org/119993002/diff/130001/ui/gfx/font_list.cc#new...
ui/gfx/font_list.cc:257: // It's possible that gfx::Font::UNDERLINE is specified
and it has already
Nit: it has -> it's

[Yuki, 2013-12-24 05:55:17]

https://codereview.chromium.org/119993002/diff/130001/ui/gfx/font_list.cc
File ui/gfx/font_list.cc (right):

https://codereview.chromium.org/119993002/diff/130001/ui/gfx/font_list.cc#new...
ui/gfx/font_list.cc:257: // It's possible that gfx::Font::UNDERLINE is specified
and it has already
On 2013/12/20 17:52:19, Alexei Svitkine wrote:
> Nit: it has -> it's

Done.

[commit-bot: I haz the power, 2013-12-24 06:44:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yukishiino@chromium.org/119993002/220001

[commit-bot: I haz the power, 2013-12-24 13:22:59]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[commit-bot: I haz the power, 2013-12-24 15:33:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yukishiino@chromium.org/119993002/220001

[commit-bot: I haz the power, 2013-12-25 05:33:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yukishiino@chromium.org/119993002/630001

[commit-bot: I haz the power, 2013-12-25 07:55:20]

Change committed as 242486