Mac: Implement GetFallbackFontFamilies for Harfbuzz

Mac: Implement GetFallbackFontFamilies for Harfbuzz

From 10.8 onwards, CoreText provides
CTFontCopyDefaultCascadeListForLanguages() which is exactly what we
want. For 10.6 and 10.7, use an older private interface,
CTFontCopyDefaultCascadeList().

Adds tests to ensure the feature detection and API behavior is sane
across all OSX versions we support.

BUG=439039, 462477
NOPRESUBMIT=true

Committed: https://crrev.com/2daf5448d99882ffc6957c773a83a2e7efdb73e2
Cr-Commit-Position: refs/heads/master@{#319176}

[tapted, 2015-03-03 05:12:44]

Patchset #6 (id:100001) has been deleted

[tapted, 2015-03-03 05:16:44]

Patchset #5 (id:80001) has been deleted

[tapted, 2015-03-03 05:27:50]

tapted@chromium.org changed reviewers:
+ rsesek@chromium.org

[tapted, 2015-03-03 05:29:48]

tapted@chromium.org changed reviewers:
+ thakis@chromium.org
- rsesek@chromium.org

[tapted, 2015-03-03 05:39:11]

tapted@chromium.org changed reviewers:
+ asvitkine@chromium.org

[tapted, 2015-03-03 07:23:18]

Hi Alexei and Nico - please take a look.

Nico for foundation_util owners and for all the crazy Mac voodoo [I *think* this
is the way to do it....] and Alexei for HarfBuzz.

The 10.6 trybot is in a funny state, but I've tested locally:
 - compiled on 10.6 [10.6 SDK]
 - compiled on 10.9 [10.9 SDK]
 - compiled on 10.9 [10.6 SDK]

There's a bit more history in http://crbug.com/439039#c8

Thanksb

[Alexei Svitkine (slow), 2015-03-03 23:12:33]

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
File ui/gfx/font_fallback_mac.mm (right):

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:1: // Copyright 2014 The Chromium Authors. All
rights reserved.
2015

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:27: CTFontRef font_ref, CFArrayRef
language_pref_list) {
Nit: 1 param per line if first param wraps.

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:29: static const MountainLionPrototype prototype =
Nit: This isn't really a prototype - it's a function pointer. How about
|function_pointer| or |copy_default_cascade_list_function|.

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:32: DCHECK(prototype ||
base::mac::IsOSLionOrEarlier());
Nit: Can't this just be moved below the if and just test the second cond?

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:82: const CFIndex fallback_count =
CFArrayGetCount(cascade_list);
Nit: Move this to be right above the loop.

[tapted, 2015-03-03 23:41:14]

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
File ui/gfx/font_fallback_mac.mm (right):

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:1: // Copyright 2014 The Chromium Authors. All
rights reserved.
On 2015/03/03 23:12:33, Alexei Svitkine wrote:
> 2015

Done (I guess it's pretty different to font_fallback_mac.cc now :).

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:27: CTFontRef font_ref, CFArrayRef
language_pref_list) {
On 2015/03/03 23:12:33, Alexei Svitkine wrote:
> Nit: 1 param per line if first param wraps.

Done.

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:29: static const MountainLionPrototype prototype =
On 2015/03/03 23:12:33, Alexei Svitkine wrote:
> Nit: This isn't really a prototype - it's a function pointer. How about
> |function_pointer| or |copy_default_cascade_list_function|.

Done. Went with `cascade_with_languages_function` to distinguish it from the
non-languages function that we fall back to.

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:32: DCHECK(prototype ||
base::mac::IsOSLionOrEarlier());
On 2015/03/03 23:12:33, Alexei Svitkine wrote:
> Nit: Can't this just be moved below the if and just test the second cond?

Yep - that has the same effect.

https://codereview.chromium.org/971673002/diff/120001/ui/gfx/font_fallback_ma...
ui/gfx/font_fallback_mac.mm:82: const CFIndex fallback_count =
CFArrayGetCount(cascade_list);
On 2015/03/03 23:12:33, Alexei Svitkine wrote:
> Nit: Move this to be right above the loop.

Done.

[Alexei Svitkine (slow), 2015-03-04 16:23:02]

lgtm % comment

https://codereview.chromium.org/971673002/diff/140001/ui/gfx/render_text_unit...
File ui/gfx/render_text_unittest.cc (right):

https://codereview.chromium.org/971673002/diff/140001/ui/gfx/render_text_unit...
ui/gfx/render_text_unittest.cc:2596: std::vector<std::string> fallback_families
= GetFallbackFontFamilies("Arial");
Sounds like this should be in font_fallback_mac_unittest.cc.

[Nico, 2015-03-04 18:06:10]

lgtm

asvitkine's comment on the test makes sense to me

[tapted, 2015-03-04 23:44:35]

Thanks all!

https://codereview.chromium.org/971673002/diff/140001/ui/gfx/render_text_unit...
File ui/gfx/render_text_unittest.cc (right):

https://codereview.chromium.org/971673002/diff/140001/ui/gfx/render_text_unit...
ui/gfx/render_text_unittest.cc:2596: std::vector<std::string> fallback_families
= GetFallbackFontFamilies("Arial");
On 2015/03/04 16:23:02, Alexei Svitkine wrote:
> Sounds like this should be in font_fallback_mac_unittest.cc.

Done.

[tapted, 2015-03-04 23:46:03]

The CQ bit was checked by tapted@chromium.org

[tapted, 2015-03-04 23:46:03]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/971673002/#ps160001
(title: "Move to font_fallback_mac_unittest")

[commit-bot: I haz the power, 2015-03-04 23:47:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/971673002/160001

[commit-bot: I haz the power, 2015-03-05 00:09:27]

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

[commit-bot: I haz the power, 2015-03-05 00:09:28]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[tapted, 2015-03-05 00:18:01]

On 2015/03/05 00:09:28, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

ug. This is

** Presubmit Warnings **
|const NSClass*| is wrong, see http://dev.chromium.org/developers/clang-mac
  base/mac/foundation_util.mm

which came across as just an upload warning, and doesn't give a "continue?"
prompt.

The code is some distant stuff in foundation_util.mm that's not changing

NSFont* CFToNSCast(CTFontRef cf_val) {
  NSFont* ns_val =
      const_cast<NSFont*>(reinterpret_cast<const NSFont*>(cf_val));
..


I think there is 

NOPRESUBMIT=true

for this. But let me know if there's anything else I should follow-up on.

[tapted, 2015-03-05 00:19:08]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2015-03-05 00:20:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/971673002/160001

[commit-bot: I haz the power, 2015-03-05 00:42:22]

Committed patchset #7 (id:160001)

[commit-bot: I haz the power, 2015-03-05 00:43:13]

Patchset 7 (id:??) landed as
https://crrev.com/2daf5448d99882ffc6957c773a83a2e7efdb73e2
Cr-Commit-Position: refs/heads/master@{#319176}