Mac: Fix PlatformFontMac::DeriveFont for underline font style.

Mac: Fix PlatformFontMac::DeriveFont for underline font style.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived PlatformFontMac instance.
This is because NSFont does not support underline as a font trait and while
creating the derived PlatformFontMac instance, we fail to set the underline
property in the derived PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404
TEST=On Mac, build and run views_examples_with_content_exe. Select the "Text
Styles" example. Check "Underline". Verify the text gets underlined. It becoming
bold is another issue that will be fixed in a follow-up.

[karandeepb, 2016-07-29 11:16:32]

The CQ bit was checked by karandeepb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 11:16:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[karandeepb, 2016-07-29 11:24:45]

Description was changed from

==========
Mac Font.
==========

to

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived font. This is because
NSFont does not support underline as a font trait and while creating the derived
PlatformFontMac instance, we fail to set the underline property in the derived
PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor.

BUG=605404
==========

[karandeepb, 2016-07-29 11:25:31]

Description was changed from

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived font. This is because
NSFont does not support underline as a font trait and while creating the derived
PlatformFontMac instance, we fail to set the underline property in the derived
PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor.

BUG=605404
==========

to

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived font. This is because
NSFont does not support underline as a font trait and while creating the derived
PlatformFontMac instance, we fail to set the underline property in the derived
PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404
==========

[karandeepb, 2016-07-29 11:26:22]

Description was changed from

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived font. This is because
NSFont does not support underline as a font trait and while creating the derived
PlatformFontMac instance, we fail to set the underline property in the derived
PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404
==========

to

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived font. This is because
NSFont does not support underline as a font trait and while creating the derived
PlatformFontMac instance, we fail to set the underline property in the derived
PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404
==========

[commit-bot: I haz the power, 2016-07-29 12:00:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 12:00:30]

Dry run: This issue passed the CQ dry run.

[karandeepb, 2016-08-01 01:53:22]

Description was changed from

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived font. This is because
NSFont does not support underline as a font trait and while creating the derived
PlatformFontMac instance, we fail to set the underline property in the derived
PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404
==========

to

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived PlatformFontMac instance.
This is because NSFont does not support underline as a font trait and while
creating the derived PlatformFontMac instance, we fail to set the underline
property in the derived PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404
==========

[karandeepb, 2016-08-01 02:03:21]

Description was changed from

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived PlatformFontMac instance.
This is because NSFont does not support underline as a font trait and while
creating the derived PlatformFontMac instance, we fail to set the underline
property in the derived PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404
==========

to

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived PlatformFontMac instance.
This is because NSFont does not support underline as a font trait and while
creating the derived PlatformFontMac instance, we fail to set the underline
property in the derived PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404 
TEST=On Mac, build and run views_examples_with_content_exe. Select
the "Text Styles" example. Check "Underline". Verify the text gets underlined.
==========

[karandeepb, 2016-08-05 04:18:21]

Description was changed from

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived PlatformFontMac instance.
This is because NSFont does not support underline as a font trait and while
creating the derived PlatformFontMac instance, we fail to set the underline
property in the derived PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404 
TEST=On Mac, build and run views_examples_with_content_exe. Select
the "Text Styles" example. Check "Underline". Verify the text gets underlined.
==========

to

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived PlatformFontMac instance.
This is because NSFont does not support underline as a font trait and while
creating the derived PlatformFontMac instance, we fail to set the underline
property in the derived PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404 

TEST=On Mac, build and run views_examples_with_content_exe. Select the "Text
Styles" example. Check "Underline". Verify the text gets underlined. It becoming
bold is another issue that will be fixed in a follow-up.
==========

[karandeepb, 2016-08-05 10:54:34]

Description was changed from

==========
Mac: Fix PlatformFontMac::DeriveFont.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived PlatformFontMac instance.
This is because NSFont does not support underline as a font trait and while
creating the derived PlatformFontMac instance, we fail to set the underline
property in the derived PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404 

TEST=On Mac, build and run views_examples_with_content_exe. Select the "Text
Styles" example. Check "Underline". Verify the text gets underlined. It becoming
bold is another issue that will be fixed in a follow-up.
==========

to

==========
Mac: Fix PlatformFontMac::DeriveFont for underline font style.

PlatformFontMac::DeriveFont derives the font from the existing font descriptor
if only the font size has to be altered. But in this case, if the existing font
has the underline style set, it is lost in the derived PlatformFontMac instance.
This is because NSFont does not support underline as a font trait and while
creating the derived PlatformFontMac instance, we fail to set the underline
property in the derived PlatformFontMac instance.

This CL fixes the issue by passing the underline style to the PlatformFontMac
constructor. A test is also added which fails for the current master on Mac.

BUG=605404 

TEST=On Mac, build and run views_examples_with_content_exe. Select the "Text
Styles" example. Check "Underline". Verify the text gets underlined. It becoming
bold is another issue that will be fixed in a follow-up.
==========

[karandeepb, 2016-08-05 11:03:07]

karandeepb@chromium.org changed reviewers:
+ avi@chromium.org

[karandeepb, 2016-08-05 11:03:07]

PTAL asvitkine@. Thanks.

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

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/font_unittest.cc#new...
ui/gfx/font_unittest.cc:161: TEST_F(FontTest, DeriveFont) {
All the other tests seem to be disabled on Android but the try bots pass. Should
this be disabled on Android as well?

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/font_unittest.cc#new...
ui/gfx/font_unittest.cc:168: EXPECT_EQ(cf.GetStyle() | gfx::Font::UNDERLINE,
This fails on Mac currently. The derived font does not have underline set.

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/platform_font_mac.mm
File ui/gfx/platform_font_mac.mm (right):

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/platform_font_mac.mm...
ui/gfx/platform_font_mac.mm:84: : PlatformFontMac(font_name,
Just a small cleanup.

[karandeepb, 2016-08-05 11:04:05]

Sorry. Pressed enter too quickly. PTAL avi@ :).

[Avi (use Gerrit), 2016-08-05 14:54:59]

lgtm

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

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/font_unittest.cc#new...
ui/gfx/font_unittest.cc:161: TEST_F(FontTest, DeriveFont) {
On 2016/08/05 11:03:07, karandeepb wrote:
> All the other tests seem to be disabled on Android but the try bots pass.
Should
> this be disabled on Android as well?

Perhaps you'll find out on the trybots :)

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/font_unittest.cc#new...
ui/gfx/font_unittest.cc:168: EXPECT_EQ(cf.GetStyle() | gfx::Font::UNDERLINE,
On 2016/08/05 11:03:07, karandeepb wrote:
> This fails on Mac currently. The derived font does not have underline set.

As long as it passes with the other changes in the CL...

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/platform_font_mac.h
File ui/gfx/platform_font_mac.h (right):

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/platform_font_mac.h#...
ui/gfx/platform_font_mac.h:19: explicit PlatformFontMac(NativeFont native_font,
bool underline = false);
If you have two parameters, you don't need to have this marked "explicit" any
more.

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/platform_font_mac.mm
File ui/gfx/platform_font_mac.mm (right):

https://codereview.chromium.org/2189353002/diff/1/ui/gfx/platform_font_mac.mm...
ui/gfx/platform_font_mac.mm:84: : PlatformFontMac(font_name,
On 2016/08/05 11:03:07, karandeepb wrote:
> Just a small cleanup.

Acknowledged.

[karandeepb, 2016-08-09 01:04:40]

Closed in light of crrev.com/2222483002.