Applying M-68 milestone per email discussion with ellyjones@.

MacViews triage: spqchan@, can you look into this? Let's fix this by M-68 :)

Just to clarify: this should ALWAYS return harfbuzz on MAC_VIEWS_BROWSER builds?

nope -- only when Typesetter::PLATFORM is passed. 

Typesetter::NATIVE is still needed for tooltips and NSMenu (or potentially other places we must still render text on Mac with an NSAttributedString rather than gfx::RenderText)

Thanks for the clarification!

Sorry, I'm still confused about this. There's no Typesetter::PLATFORM.

Also, what do you mean by "switches::kEnableHarfBuzzRenderText should always be set"?
Should I remove kEnableHarfBuzzRenderText and always return Harfbuzz in Mac if Typesetter:NATIVE is not passed? Thanks!

ah - my mistake. It was renamed to BROWSER. Other stuff has shifted about since this bug was filed too. I think we just need RenderText::CreateFor to consider kViewsBrowserWindows

something like

// static
std::unique_ptr<RenderText> RenderText::CreateFor(Typesetter typesetter) {
#if defined(OS_MACOSX)
  if (typesetter == Typesetter::NATIVE)
    return std::make_unique<RenderTextMac>();

  if (typesetter == Typesetter::HARFBUZZ)
    return CreateHarfBuzzInstance();

  static const bool use_native =
      !base::FeatureList::IsEnabled(features::kViewsBrowserWindows) &&
      !base::CommandLine::ForCurrentProcess()->HasSwitch(
          switches::kEnableHarfBuzzRenderText);
  if (use_native)
    return std::make_unique<RenderTextMac>();
#endif  // defined(OS_MACOSX)
  return CreateHarfBuzzInstance();
}

Thanks for the clarification! Sounds good. This will be a bit tricky since we can't put ui_base stuff into ui/gfx but I'll see if there's a workaround.

** Bulk Edit **

FYI: Starting 04/13 M68 will be in canary, M68 Dev promotion will be on 04/26.

Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.

MacViews triage: over to weili@ for M69 :)

https://chromium-review.googlesource.com/c/chromium/src/+/1248263

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a5abaaad7eb9693282982a3aadd5bd63357d032

commit 5a5abaaad7eb9693282982a3aadd5bd63357d032
Author: Trent Apted <tapted@chromium.org>
Date: Thu Sep 27 15:34:46 2018

Remove [gfx] switches::kEnableHarfBuzzRenderText.

It's effectively the default. Only a few places using
gfx::Typesetter::NATIVE explicitly should be using RenderTextMac now.

Bug:  798944 , 889152
Change-Id: I3318676b14b938607de9413130627dc1581b2289
Reviewed-on: https://chromium-review.googlesource.com/1248263
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594732}
[modify] https://crrev.com/5a5abaaad7eb9693282982a3aadd5bd63357d032/chrome/browser/about_flags.cc
[modify] https://crrev.com/5a5abaaad7eb9693282982a3aadd5bd63357d032/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/5a5abaaad7eb9693282982a3aadd5bd63357d032/ui/gfx/render_text.cc
[modify] https://crrev.com/5a5abaaad7eb9693282982a3aadd5bd63357d032/ui/gfx/switches.cc
[modify] https://crrev.com/5a5abaaad7eb9693282982a3aadd5bd63357d032/ui/gfx/switches.h
[modify] https://crrev.com/5a5abaaad7eb9693282982a3aadd5bd63357d032/ui/views/controls/label_unittest.cc