Disable RTHB on Mac when TOOLKIT_VIEWS isn't defined

Disable RTHB on Mac when TOOLKIT_VIEWS isn't defined

BUG=429371
R=msw

Committed: https://crrev.com/753eb0c4c7b50614d6557b624833d8923e96a6b3
Cr-Commit-Position: refs/heads/master@{#304137}

[msw, 2014-11-12 18:55:21]

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

https://codereview.chromium.org/721843002/diff/1/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:402: return CreateNativeInstance();
Allow HarfBuzz with switches::kEnableHarfBuzzRenderText for testing.

https://codereview.chromium.org/721843002/diff/1/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:404: return CommandLine::ForCurrentProcess()->HasSwitch(
optional nit: cache the command line value in a static local bool.
  static const bool use_harfbuzz = !CommandLine::ForCurrentProcess()->
      HasSwitch(switches::kDisableHarfBuzzRenderText);
  return use_harfbuzz ? new RenderTextHarfBuzz() : CreateNativeInstance();

[ckocagil, 2014-11-12 19:23:01]

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

https://codereview.chromium.org/721843002/diff/1/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:402: return CreateNativeInstance();
On 2014/11/12 18:55:21, msw wrote:
> Allow HarfBuzz with switches::kEnableHarfBuzzRenderText for testing.

Done.

https://codereview.chromium.org/721843002/diff/1/ui/gfx/render_text.cc#newcod...
ui/gfx/render_text.cc:404: return CommandLine::ForCurrentProcess()->HasSwitch(
On 2014/11/12 18:55:21, msw wrote:
> optional nit: cache the command line value in a static local bool.
>   static const bool use_harfbuzz = !CommandLine::ForCurrentProcess()->
>       HasSwitch(switches::kDisableHarfBuzzRenderText);
>   return use_harfbuzz ? new RenderTextHarfBuzz() : CreateNativeInstance();

Done.

[msw, 2014-11-12 19:29:45]

lgtm

[ckocagil, 2014-11-12 19:30:34]

The CQ bit was checked by ckocagil@chromium.org

[commit-bot: I haz the power, 2014-11-12 19:30:58]

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

[commit-bot: I haz the power, 2014-11-12 20:47:05]

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

[commit-bot: I haz the power, 2014-11-12 20:47:08]

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

[ckocagil, 2014-11-13 10:54:38]

ckocagil@chromium.org changed reviewers:
+ derat@chromium.org

[ckocagil, 2014-11-13 19:18:41]

+derat: RenderTextMac fails the RenderTextTest.FontListFallback test.
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...

Do you think we can disable this test on Mac? If so, should we add a TODO?

[Daniel Erat, 2014-11-13 19:23:09]

On 2014/11/13 19:18:41, ckocagil wrote:
> +derat: RenderTextMac fails the RenderTextTest.FontListFallback test.
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...
> 
> Do you think we can disable this test on Mac? If so, should we add a TODO?

i think it's fine to disable it for RenderTextMac -- it's really tied to RTHB's
fallback logic.

[ckocagil, 2014-11-13 19:24:56]

On 2014/11/13 19:23:09, Daniel Erat wrote:
> On 2014/11/13 19:18:41, ckocagil wrote:
> > +derat: RenderTextMac fails the RenderTextTest.FontListFallback test.
> >
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...
> > 
> > Do you think we can disable this test on Mac? If so, should we add a TODO?
> 
> i think it's fine to disable it for RenderTextMac -- it's really tied to
RTHB's
> fallback logic.

Can we make it a RTHB test then? (by directly using a RTHB instance instead of
RenderText)

[Daniel Erat, 2014-11-13 19:30:38]

On 2014/11/13 19:24:56, ckocagil wrote:
> On 2014/11/13 19:23:09, Daniel Erat wrote:
> > On 2014/11/13 19:18:41, ckocagil wrote:
> > > +derat: RenderTextMac fails the RenderTextTest.FontListFallback test.
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...
> > > 
> > > Do you think we can disable this test on Mac? If so, should we add a TODO?
> > 
> > i think it's fine to disable it for RenderTextMac -- it's really tied to
> RTHB's
> > fallback logic.
> 
> Can we make it a RTHB test then? (by directly using a RTHB instance instead of
> RenderText)

that sounds fine to me.

[msw, 2014-11-13 22:39:22]

lgtm

[ckocagil, 2014-11-13 23:18:13]

The CQ bit was checked by ckocagil@chromium.org

[commit-bot: I haz the power, 2014-11-13 23:21:45]

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

[commit-bot: I haz the power, 2014-11-14 01:35:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-14 01:37:34]

Patchset 3 (id:??) landed as
https://crrev.com/753eb0c4c7b50614d6557b624833d8923e96a6b3
Cr-Commit-Position: refs/heads/master@{#304137}