Suppress GetFallbackFontFamilies spam on Mac

Suppress GetFallbackFontFamilies spam on Mac

BUG=439039
Committed: https://crrev.com/97ad0b4aa3fd6b3705f9a2c6d20e95527bc88f6f
Cr-Commit-Position: refs/heads/master@{#306848}

[Jiang Jiang, 2014-12-04 14:09:56]

jiangj@opera.com changed reviewers:
+ asvitkine@chromium.org, derat@chromium.org

[Jiang Jiang, 2014-12-04 14:10:26]

PTAL

[Alexei Svitkine (slow), 2014-12-04 14:33:57]

lgtm, though I thought the Mac code doesn't use this by default now? (i.e. it
should still use RenderTextMac which doesn't call this)

Am I wrong?

[Daniel Erat, 2014-12-04 15:08:57]

On 2014/12/04 14:33:57, Alexei Svitkine wrote:
> lgtm, though I thought the Mac code doesn't use this by default now? (i.e. it
> should still use RenderTextMac which doesn't call this)
> 
> Am I wrong?

alexie, i don't think you're wrong (assuming that we don't build mac with
TOOLKIT_VIEWS). from ui/gfx/render_text.cc:

RenderText* RenderText::CreateInstance() {
#if defined(OS_MACOSX) && !defined(TOOLKIT_VIEWS)
  static const bool use_harfbuzz = CommandLine::ForCurrentProcess()->
      HasSwitch(switches::kEnableHarfBuzzRenderText);
#else
  static const bool use_harfbuzz = !CommandLine::ForCurrentProcess()->
      HasSwitch(switches::kDisableHarfBuzzRenderText);
#endif
  return use_harfbuzz ? new RenderTextHarfBuzz : CreateNativeInstance();
}

jiang, do you know where this code is getting called?

[Jiang Jiang, 2014-12-04 15:45:10]

On 2014/12/04 15:08:57, Daniel Erat wrote:
> On 2014/12/04 14:33:57, Alexei Svitkine wrote:
> > lgtm, though I thought the Mac code doesn't use this by default now? (i.e.
it
> > should still use RenderTextMac which doesn't call this)
> > 
> > Am I wrong?
> 
> alexie, i don't think you're wrong (assuming that we don't build mac with
> TOOLKIT_VIEWS). from ui/gfx/render_text.cc:
> 
> RenderText* RenderText::CreateInstance() {
> #if defined(OS_MACOSX) && !defined(TOOLKIT_VIEWS)
>   static const bool use_harfbuzz = CommandLine::ForCurrentProcess()->
>       HasSwitch(switches::kEnableHarfBuzzRenderText);
> #else
>   static const bool use_harfbuzz = !CommandLine::ForCurrentProcess()->
>       HasSwitch(switches::kDisableHarfBuzzRenderText);
> #endif
>   return use_harfbuzz ? new RenderTextHarfBuzz : CreateNativeInstance();
> }
> 
> jiang, do you know where this code is getting called?

Did it get reverted at some point? I was pretty certain that HarfBuzz backend
got switched on for Mac earlier, even saw a PSA email on chromium-dev.

[Alexei Svitkine (slow), 2014-12-04 15:46:27]

Yes, it did.

On Thu, Dec 4, 2014 at 10:45 AM, <jiangj@opera.com> wrote:

> On 2014/12/04 15:08:57, Daniel Erat wrote:
>
>> On 2014/12/04 14:33:57, Alexei Svitkine wrote:
>> > lgtm, though I thought the Mac code doesn't use this by default now?
>> (i.e.
>>
> it
>
>> > should still use RenderTextMac which doesn't call this)
>> >
>> > Am I wrong?
>>
>
>  alexie, i don't think you're wrong (assuming that we don't build mac with
>> TOOLKIT_VIEWS). from ui/gfx/render_text.cc:
>>
>
>  RenderText* RenderText::CreateInstance() {
>> #if defined(OS_MACOSX) && !defined(TOOLKIT_VIEWS)
>>    static const bool use_harfbuzz = CommandLine::ForCurrentProcess()->
>>        HasSwitch(switches::kEnableHarfBuzzRenderText);
>> #else
>>    static const bool use_harfbuzz = !CommandLine::ForCurrentProcess()->
>>        HasSwitch(switches::kDisableHarfBuzzRenderText);
>> #endif
>>    return use_harfbuzz ? new RenderTextHarfBuzz : CreateNativeInstance();
>> }
>>
>
>  jiang, do you know where this code is getting called?
>>
>
> Did it get reverted at some point? I was pretty certain that HarfBuzz
> backend
> got switched on for Mac earlier, even saw a PSA email on chromium-dev.
>
> https://codereview.chromium.org/767223003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ckocagil, 2014-12-04 15:47:02]

On 2014/12/04 15:45:10, Jiang Jiang wrote:
> Did it get reverted at some point? I was pretty certain that HarfBuzz backend
> got switched on for Mac earlier, even saw a PSA email on chromium-dev.

Yes. I didn't see it important enough to notify chromium-dev, but we disabled
RTHB on Mac.

[Jiang Jiang, 2014-12-04 15:47:06]

On 2014/12/04 15:45:10, Jiang Jiang wrote:
> On 2014/12/04 15:08:57, Daniel Erat wrote:
> > On 2014/12/04 14:33:57, Alexei Svitkine wrote:
> > > lgtm, though I thought the Mac code doesn't use this by default now? (i.e.
> it
> > > should still use RenderTextMac which doesn't call this)
> > > 
> > > Am I wrong?
> > 
> > alexie, i don't think you're wrong (assuming that we don't build mac with
> > TOOLKIT_VIEWS). from ui/gfx/render_text.cc:
> > 
> > RenderText* RenderText::CreateInstance() {
> > #if defined(OS_MACOSX) && !defined(TOOLKIT_VIEWS)
> >   static const bool use_harfbuzz = CommandLine::ForCurrentProcess()->
> >       HasSwitch(switches::kEnableHarfBuzzRenderText);
> > #else
> >   static const bool use_harfbuzz = !CommandLine::ForCurrentProcess()->
> >       HasSwitch(switches::kDisableHarfBuzzRenderText);
> > #endif
> >   return use_harfbuzz ? new RenderTextHarfBuzz : CreateNativeInstance();
> > }
> > 
> > jiang, do you know where this code is getting called?
> 
> Did it get reverted at some point? I was pretty certain that HarfBuzz backend
> got switched on for Mac earlier, even saw a PSA email on chromium-dev.

Looks like ckocagil reverted it recently for non-views build:
https://codereview.chromium.org/721843002

I don't absolutely need this patch as we don't make views build yet, but I still
think it's nice to have if just for the people developing MacViews code to see
less warning. What do you guys think?

[Daniel Erat, 2014-12-04 15:54:16]

lgtm

seems reasonable to me

[Alexei Svitkine (slow), 2014-12-04 15:55:30]

likewise, still lgtm

On Thu, Dec 4, 2014 at 10:54 AM, <derat@chromium.org> wrote:

> lgtm
>
> seems reasonable to me
>
> https://codereview.chromium.org/767223003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Jiang Jiang, 2014-12-04 16:08:09]

The CQ bit was checked by jiangj@opera.com

[commit-bot: I haz the power, 2014-12-04 16:08:46]

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

[commit-bot: I haz the power, 2014-12-04 17:30:38]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-12-04 17:31:20]

Patchset 1 (id:??) landed as
https://crrev.com/97ad0b4aa3fd6b3705f9a2c6d20e95527bc88f6f
Cr-Commit-Position: refs/heads/master@{#306848}