Mac: Fix PlatformFontMac::DeriveFont.

Mac: Fix PlatformFontMac::DeriveFont.

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font (See Potential API Pitfalls
   section in the WWDC talk Introducing the New System Fonts. Link -
   https://developer.apple.com/videos/play/wwdc2015/804/).

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved. Also, added two tests which fail on
the current master and demonstrate the problem.

BUG=605404
TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.

Committed: https://crrev.com/f7e5e8283b6a43f3371381fedd81beea941cef48
Cr-Commit-Position: refs/heads/master@{#410525}

[karandeepb, 2016-08-05 10:44:03]

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

[commit-bot: I haz the power, 2016-08-05 10:44:15]

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

[commit-bot: I haz the power, 2016-08-05 11:50:48]

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

[commit-bot: I haz the power, 2016-08-05 11:50:49]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[karandeepb, 2016-08-08 01:33:29]

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

[commit-bot: I haz the power, 2016-08-08 01:33:39]

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

[commit-bot: I haz the power, 2016-08-08 02:01:51]

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

[commit-bot: I haz the power, 2016-08-08 02:01:52]

Dry run: This issue passed the CQ dry run.

[karandeepb, 2016-08-08 10:53:16]

Description was changed from

==========
PlatformFontMac fixes.
==========

to

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font.

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved.

BUG=605404

TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.
==========

[karandeepb, 2016-08-08 10:56:33]

Description was changed from

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font.

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved.

BUG=605404

TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.
==========

to

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font.

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved. Also, added two tests which fail on 
the current master and demonstrate the problem.

BUG=605404
TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.
==========

[karandeepb, 2016-08-08 11:25:47]

Patchset #1 (id:1) has been deleted

[karandeepb, 2016-08-08 11:25:55]

Patchset #1 (id:20001) has been deleted

[karandeepb, 2016-08-08 11:26:31]

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

[commit-bot: I haz the power, 2016-08-08 11:26:47]

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

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

Description was changed from

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font.

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved. Also, added two tests which fail on 
the current master and demonstrate the problem.

BUG=605404
TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.
==========

to

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font (See Potential API Pitfalls 
   section in the WWDC talk Introducing the New System Fonts. Link - 
   https://developer.apple.com/videos/play/wwdc2015/804/).

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved. Also, added two tests which fail on 
the current master and demonstrate the problem.

BUG=605404
TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.
==========

[karandeepb, 2016-08-08 12:07:23]

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

[karandeepb, 2016-08-08 12:07:24]

PTAL avi@. Please disregard https://codereview.chromium.org/2189353002/. I have
since found a better way to structure the code. This fixes the issue fixed in
that CL, plus another issue with DeriveFont.

https://codereview.chromium.org/2222483002/diff/120001/ui/gfx/platform_font_m...
File ui/gfx/platform_font_mac.mm (right):

https://codereview.chromium.org/2222483002/diff/120001/ui/gfx/platform_font_m...
ui/gfx/platform_font_mac.mm:102: // unreliable. Hence use the NSFontManager.
The NSFontDescriptor API's like fontDescriptorWithSymbolicTraits: do not always
seem to work correctly. Hence using the NSFontManager API below.

https://codereview.chromium.org/2222483002/diff/120001/ui/gfx/platform_font_m...
ui/gfx/platform_font_mac.mm:119: return Font(new PlatformFontMac(derived_font,
font_name_,
It may happen that the native NSFont instance is not bold, even when the bold
trait was requested. My understanding of the Font/PlatformFont API is that the
font weight should still be returned as bold for the Font instance even though
the underlying native font is not bold. Can you confirm that this is correct?

[commit-bot: I haz the power, 2016-08-08 12:08:57]

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

[commit-bot: I haz the power, 2016-08-08 12:08:58]

Dry run: This issue passed the CQ dry run.

[Avi (use Gerrit), 2016-08-08 15:50:58]

lgtm

https://codereview.chromium.org/2222483002/diff/120001/ui/gfx/platform_font_m...
File ui/gfx/platform_font_mac.mm (right):

https://codereview.chromium.org/2222483002/diff/120001/ui/gfx/platform_font_m...
ui/gfx/platform_font_mac.mm:119: return Font(new PlatformFontMac(derived_font,
font_name_,
On 2016/08/08 12:07:24, karandeepb wrote:
> It may happen that the native NSFont instance is not bold, even when the bold
> trait was requested. My understanding of the Font/PlatformFont API is that the
> font weight should still be returned as bold for the Font instance even though
> the underlying native font is not bold. Can you confirm that this is correct?

I don't know; I don't think I've ever really played with fonts. You probably
know more than I do in this area by now :)

[karandeepb, 2016-08-09 01:02:29]

The CQ bit was checked by karandeepb@chromium.org

[commit-bot: I haz the power, 2016-08-09 01:03:08]

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

[commit-bot: I haz the power, 2016-08-09 02:18:29]

Description was changed from

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font (See Potential API Pitfalls 
   section in the WWDC talk Introducing the New System Fonts. Link - 
   https://developer.apple.com/videos/play/wwdc2015/804/).

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved. Also, added two tests which fail on 
the current master and demonstrate the problem.

BUG=605404
TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.
==========

to

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font (See Potential API Pitfalls 
   section in the WWDC talk Introducing the New System Fonts. Link - 
   https://developer.apple.com/videos/play/wwdc2015/804/).

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved. Also, added two tests which fail on 
the current master and demonstrate the problem.

BUG=605404
TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.
==========

[commit-bot: I haz the power, 2016-08-09 02:18:30]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2016-08-09 02:20:33]

Description was changed from

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font (See Potential API Pitfalls 
   section in the WWDC talk Introducing the New System Fonts. Link - 
   https://developer.apple.com/videos/play/wwdc2015/804/).

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved. Also, added two tests which fail on 
the current master and demonstrate the problem.

BUG=605404
TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.
==========

to

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

1. Currently, when we derive a font from an existing PlatformFontMac instance
   with only the size changed, we fail to persist the Font::UNDERLINE style in
   the derived font.

2. Also in all the other cases, instead of using the native_font_ instance to
   derive the font, we create a new PlatformFontMac instance from the font_name
   and other parameters. This can lead to unexpected results when the font from
   which we are deriving is a system font, since Apple explicitly forbids
   using a system font name to create a new font (See Potential API Pitfalls
   section in the WWDC talk Introducing the New System Fonts. Link -
   https://developer.apple.com/videos/play/wwdc2015/804/).

For example, in the Text Styles example of views_examples_with_content_exe, on
clicking underline, the derived font does not have underline due to 1 and is
bold because of 2, even though we didn't specify the bold font trait.

This CL modifies PlatformFontMac::DeriveFont to use the NSFontManager to create
a new derived NSfont. Since the new PlatformFontMac instance created has the
passed font style, 1 is solved. Also, since we no longer use the font name to
create the derived font, 2 is also solved. Also, added two tests which fail on
the current master and demonstrate the problem.

BUG=605404
TEST=Build and run views_examples_with_content_exe. Select the "Text Styles"
example. Check "Underline". Verify the text gets underlined. Verify the font is
not bold. Play around with different combinations and verify that they work as
expected.

Committed: https://crrev.com/f7e5e8283b6a43f3371381fedd81beea941cef48
Cr-Commit-Position: refs/heads/master@{#410525}
==========

[commit-bot: I haz the power, 2016-08-09 02:20:35]

Patchset 6 (id:??) landed as
https://crrev.com/f7e5e8283b6a43f3371381fedd81beea941cef48
Cr-Commit-Position: refs/heads/master@{#410525}