Consolidate fetching Windows system fonts into platform_font_win.

Half of requested cleanup following text scaling issues with Windows
Text Zoom accessibility feature.

Removes calls to GetNonClientMetrics() sprinkled around the code, as
well as most uses of the Windows LOGFONT struct outside of
platform_font_win and its callbacks.

The remainder of the cleanup will be in a followup CL, which will
consolidate all of the fiddly scaling and other uses of LOGFONT into
platform_font_win.

Bug: 888788
Change-Id: Iff47d2037b6a70210bd1eb0f24ab2f0430c59f86
Reviewed-on: https://chromium-review.googlesource.com/1246824Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594889}

chrome/browser/chrome_browser_main_win.cc

@@ -464,8 +464,8 @@ ChromeBrowserMainPartsWin::~ChromeBrowserMainPartsWin() {
 
 void ChromeBrowserMainPartsWin::ToolkitInitialized() {
   ChromeBrowserMainParts::ToolkitInitialized();
-  gfx::PlatformFontWin::adjust_font_callback = &AdjustUIFont;
-  gfx::PlatformFontWin::get_minimum_font_size_callback = &GetMinimumFontSize;
+  gfx::PlatformFontWin::SetAdjustFontCallback(&AdjustUIFont);
+  gfx::PlatformFontWin::SetGetMinimumFontSizeCallback(&GetMinimumFontSize);
   ui::CursorLoaderWin::SetCursorResourceModule(chrome::kBrowserResourcesDll);
 }
 

content/browser/renderer_host/render_view_host_impl.cc

@@ -96,8 +96,6 @@
 #include "url/url_constants.h"
 
 #if defined(OS_WIN)
-#include "base/win/win_client_metrics.h"
-#include "ui/display/win/dpi.h"
 #include "ui/display/win/screen_win.h"
 #include "ui/gfx/geometry/dip_util.h"
 #include "ui/gfx/platform_font_win.h"
@@ -122,42 +120,33 @@ using RoutingIDViewMap = base::hash_map<RenderViewHostID, RenderViewHostImpl*>;
 base::LazyInstance<RoutingIDViewMap>::Leaky g_routing_id_view_map =
     LAZY_INSTANCE_INITIALIZER;
 
+#if defined(OS_WIN)
+// Fetches the name and font size of a particular Windows system font.
+void GetFontInfo(gfx::PlatformFontWin::SystemFont system_font,
+                 base::string16* name,
+                 int32_t* size) {
+  const gfx::Font& font = gfx::PlatformFontWin::GetSystemFont(system_font);
+  *name = base::UTF8ToUTF16(font.GetFontName());
+  *size = font.GetFontSize();
+}
+#endif  // OS_WIN
+
 void GetPlatformSpecificPrefs(RendererPreferences* prefs) {
 #if defined(OS_WIN)
-  NONCLIENTMETRICS_XP metrics = {0};
-  base::win::GetNonClientMetrics(&metrics);
-
-  // Render process has the same problem with Windows applying text scaling
-  // before Chrome DPI + accessibility scaling is applied, resulting in double
-  // scaling. These fonts are only used for very specific CSS styles, but to
-  // remain consistent with the rest of chrome we will make sure they aren't
-  // incorrectly scaled:
-  display::win::AdjustFontForAccessibility(&metrics.lfCaptionFont);
-  display::win::AdjustFontForAccessibility(&metrics.lfSmCaptionFont);
-  display::win::AdjustFontForAccessibility(&metrics.lfMenuFont);
-  display::win::AdjustFontForAccessibility(&metrics.lfStatusFont);
-  display::win::AdjustFontForAccessibility(&metrics.lfMessageFont);
-
-  // Store the preferred font faces and sizes:
-  prefs->caption_font_family_name = metrics.lfCaptionFont.lfFaceName;
-  prefs->caption_font_height = gfx::PlatformFontWin::GetFontSize(
-      metrics.lfCaptionFont);
-
-  prefs->small_caption_font_family_name = metrics.lfSmCaptionFont.lfFaceName;
-  prefs->small_caption_font_height = gfx::PlatformFontWin::GetFontSize(
-      metrics.lfSmCaptionFont);
-
-  prefs->menu_font_family_name = metrics.lfMenuFont.lfFaceName;
-  prefs->menu_font_height = gfx::PlatformFontWin::GetFontSize(
-      metrics.lfMenuFont);
-
-  prefs->status_font_family_name = metrics.lfStatusFont.lfFaceName;
-  prefs->status_font_height = gfx::PlatformFontWin::GetFontSize(
-      metrics.lfStatusFont);
-
-  prefs->message_font_family_name = metrics.lfMessageFont.lfFaceName;
-  prefs->message_font_height = gfx::PlatformFontWin::GetFontSize(
-      metrics.lfMessageFont);
+  // Note that what is called "height" in this struct is actually the font size;
+  // font "height" typically includes ascender, descender, and padding and is
+  // often a third or so larger than the given font size.
+  GetFontInfo(gfx::PlatformFontWin::SystemFont::kCaption,
+              &prefs->caption_font_family_name, &prefs->caption_font_height);
+  GetFontInfo(gfx::PlatformFontWin::SystemFont::kSmallCaption,
+              &prefs->small_caption_font_family_name,
+              &prefs->small_caption_font_height);
+  GetFontInfo(gfx::PlatformFontWin::SystemFont::kMenu,
+              &prefs->menu_font_family_name, &prefs->menu_font_height);
+  GetFontInfo(gfx::PlatformFontWin::SystemFont::kMessage,
+              &prefs->message_font_family_name, &prefs->message_font_height);
+  GetFontInfo(gfx::PlatformFontWin::SystemFont::kStatus,
+              &prefs->status_font_family_name, &prefs->status_font_height);
 
   prefs->vertical_scroll_bar_width_in_dips =
       display::win::ScreenWin::GetSystemMetricsInDIP(SM_CXVSCROLL);

ui/gfx/font_unittest.cc

@@ -21,33 +21,6 @@ namespace {
 
 using FontTest = testing::Test;
 
-#if defined(OS_WIN)
-class ScopedMinimumFontSizeCallback {
- public:
-  explicit ScopedMinimumFontSizeCallback(int minimum_size) {
-    minimum_size_ = minimum_size;
-    old_callback_ = PlatformFontWin::get_minimum_font_size_callback;
-    PlatformFontWin::get_minimum_font_size_callback = &GetMinimumFontSize;
-  }
-
-  ~ScopedMinimumFontSizeCallback() {
-    PlatformFontWin::get_minimum_font_size_callback = old_callback_;
-  }
-
- private:
-  static int GetMinimumFontSize() {
-    return minimum_size_;
-  }
-
-  PlatformFontWin::GetMinimumFontSizeCallback old_callback_;
-  static int minimum_size_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedMinimumFontSizeCallback);
-};
-
-int ScopedMinimumFontSizeCallback::minimum_size_ = 0;
-#endif  // defined(OS_WIN)
-
 TEST_F(FontTest, LoadArial) {
   Font cf(kTestFontName, 16);
 #if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_IOS)
@@ -151,8 +124,7 @@ TEST_F(FontTest, DeriveFont) {
 #if defined(OS_WIN)
 TEST_F(FontTest, DeriveResizesIfSizeTooSmall) {
   Font cf(kTestFontName, 8);
-  // The minimum font size is set to 5 in browser_main.cc.
-  ScopedMinimumFontSizeCallback minimum_size(5);
+  PlatformFontWin::SetGetMinimumFontSizeCallback([] { return 5; });
 
   Font derived_font = cf.Derive(-4, cf.GetStyle(), cf.GetWeight());
   EXPECT_EQ(5, derived_font.GetFontSize());
@@ -160,8 +132,7 @@ TEST_F(FontTest, DeriveResizesIfSizeTooSmall) {
 
 TEST_F(FontTest, DeriveKeepsOriginalSizeIfHeightOk) {
   Font cf(kTestFontName, 8);
-  // The minimum font size is set to 5 in browser_main.cc.
-  ScopedMinimumFontSizeCallback minimum_size(5);
+  PlatformFontWin::SetGetMinimumFontSizeCallback([] { return 5; });
 
   Font derived_font = cf.Derive(-2, cf.GetStyle(), cf.GetWeight());
   EXPECT_EQ(6, derived_font.GetFontSize());

ui/gfx/platform_font_win.cc

@@ -13,10 +13,13 @@
 #include <wrl/client.h>
 
 #include <algorithm>
+#include <utility>
 
+#include "base/containers/flat_map.h"
 #include "base/debug/alias.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/no_destructor.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
@@ -35,11 +38,15 @@
 
 namespace {
 
+gfx::PlatformFontWin::AdjustFontCallback g_adjust_font_callback = nullptr;
+gfx::PlatformFontWin::GetMinimumFontSizeCallback
+    g_get_minimum_font_size_callback = nullptr;
+
 // Returns the minimum font size, using the minimum size callback, if set.
 int GetMinimumFontSize() {
   int min_font_size = 0;
-  if (gfx::PlatformFontWin::get_minimum_font_size_callback)
-    min_font_size = gfx::PlatformFontWin::get_minimum_font_size_callback();
+  if (g_get_minimum_font_size_callback)
+    min_font_size = g_get_minimum_font_size_callback();
   return min_font_size;
 }
 
@@ -247,6 +254,60 @@ HRESULT GetMatchingDirectWriteFont(LOGFONT* font_info,
   return hr;
 }
 
+class SystemFonts {
+ public:
+  SystemFonts() {
+    NONCLIENTMETRICS_XP metrics;
+    base::win::GetNonClientMetrics(&metrics);
+
+    AddFont(gfx::PlatformFontWin::SystemFont::kCaption, &metrics.lfCaptionFont);
+    AddFont(gfx::PlatformFontWin::SystemFont::kSmallCaption,
+            &metrics.lfSmCaptionFont);
+    AddFont(gfx::PlatformFontWin::SystemFont::kMenu, &metrics.lfMenuFont);
+    AddFont(gfx::PlatformFontWin::SystemFont::kMessage, &metrics.lfMessageFont);
+    AddFont(gfx::PlatformFontWin::SystemFont::kStatus, &metrics.lfStatusFont);
+
+    is_initialized_ = true;
+  }
+
+  const gfx::Font& GetFont(gfx::PlatformFontWin::SystemFont system_font) const {
+    auto it = system_fonts_.find(system_font);
+    DCHECK(it != system_fonts_.end())
+        << "System font #" << static_cast<int>(system_font) << " not found!";
+    DCHECK(it->second.GetNativeFont())
+        << "Font for system font #" << static_cast<int>(system_font)
+        << " has invalid handle.";
+    return it->second;
+  }
+
+  static SystemFonts* Instance() {
+    static base::NoDestructor<SystemFonts> instance;
+    return instance.get();
+  }
+
+  static bool IsInitialized() { return is_initialized_; }
+
+ private:
+  void AddFont(gfx::PlatformFontWin::SystemFont system_font, LOGFONT* logfont) {
+    if (g_adjust_font_callback)
+      g_adjust_font_callback(logfont);
+    logfont->lfHeight = AdjustFontSize(logfont->lfHeight, 0);
+    HFONT font = CreateFontIndirect(logfont);
+    DLOG_ASSERT(font);
+    system_fonts_.emplace(system_font, gfx::Font(font));
+  }
+
+  // Use a flat map for faster lookups.
+  base::flat_map<gfx::PlatformFontWin::SystemFont, gfx::Font> system_fonts_;
+
+  static bool is_initialized_;
+
+  DISALLOW_COPY_AND_ASSIGN(SystemFonts);
+};
+
+// static
+bool SystemFonts::is_initialized_ = false;
+
 }  // namespace
 
 namespace gfx {
@@ -254,12 +315,6 @@ namespace gfx {
 // static
 PlatformFontWin::HFontRef* PlatformFontWin::base_font_ref_;
 
-// static
-PlatformFontWin::AdjustFontCallback
-    PlatformFontWin::adjust_font_callback = nullptr;
-PlatformFontWin::GetMinimumFontSizeCallback
-    PlatformFontWin::get_minimum_font_size_callback = NULL;
-
 IDWriteFactory* PlatformFontWin::direct_write_factory_ = nullptr;
 
 // TODO(ananta)
@@ -303,6 +358,37 @@ PlatformFontWin::PlatformFontWin(const std::string& font_name,
   InitWithFontNameAndSize(font_name, font_size);
 }
 
+// static
+void PlatformFontWin::SetGetMinimumFontSizeCallback(
+    GetMinimumFontSizeCallback callback) {
+  DCHECK(!SystemFonts::IsInitialized());
+  g_get_minimum_font_size_callback = callback;
+}
+
+// static
+void PlatformFontWin::SetAdjustFontCallback(AdjustFontCallback callback) {
+  DCHECK(!SystemFonts::IsInitialized());
+  g_adjust_font_callback = callback;
+}
+
+// static
+void PlatformFontWin::SetDirectWriteFactory(IDWriteFactory* factory) {
+  // We grab a reference on the DirectWrite factory. This reference is
+  // leaked, which is ok because skia leaks it as well.
+  factory->AddRef();
+  direct_write_factory_ = factory;
+}
+
+// static
+bool PlatformFontWin::IsDirectWriteEnabled() {
+  return direct_write_factory_ != nullptr;
+}
+
+// static
+const Font& PlatformFontWin::GetSystemFont(SystemFont system_font) {
+  return SystemFonts::Instance()->GetFont(system_font);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // PlatformFontWin, PlatformFont implementation:
 
@@ -386,40 +472,6 @@ NativeFont PlatformFontWin::GetNativeFont() const {
   return font_ref_->hfont();
 }
 
-// static
-void PlatformFontWin::SetDirectWriteFactory(IDWriteFactory* factory) {
-  // We grab a reference on the DirectWrite factory. This reference is
-  // leaked, which is ok because skia leaks it as well.
-  factory->AddRef();
-  direct_write_factory_ = factory;
-}
-
-// static
-bool PlatformFontWin::IsDirectWriteEnabled() {
-  return direct_write_factory_ != nullptr;
-}
-
-// static
-void PlatformFontWin::GetTextMetricsForFont(HDC hdc,
-                                            HFONT font,
-                                            TEXTMETRIC* text_metrics) {
-  base::win::ScopedSelectObject scoped_font(hdc, font);
-  GetTextMetrics(hdc, text_metrics);
-}
-
-// static
-int PlatformFontWin::GetFontSize(const LOGFONT& font_info) {
-  if (font_info.lfHeight < 0)
-    return -font_info.lfHeight;
-
-  base::win::ScopedGetDC screen_dc(NULL);
-  base::win::ScopedGDIObject<HFONT> font(CreateFontIndirect(&font_info));
-
-  TEXTMETRIC font_metrics = {0};
-  PlatformFontWin::GetTextMetricsForFont(screen_dc, font.get(), &font_metrics);
-  return font_metrics.tmAscent;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // Font, private:
 
@@ -443,20 +495,21 @@ void PlatformFontWin::InitWithFontNameAndSize(const std::string& font_name,
 }
 
 // static
-PlatformFontWin::HFontRef* PlatformFontWin::GetBaseFontRef() {
-  if (base_font_ref_ == NULL) {
-    NONCLIENTMETRICS_XP metrics;
-    base::win::GetNonClientMetrics(&metrics);
+void PlatformFontWin::GetTextMetricsForFont(HDC hdc,
+                                            HFONT font,
+                                            TEXTMETRIC* text_metrics) {
+  base::win::ScopedSelectObject scoped_font(hdc, font);
+  GetTextMetrics(hdc, text_metrics);
+}
 
-    if (adjust_font_callback)
-      adjust_font_callback(&metrics.lfMessageFont);
-    metrics.lfMessageFont.lfHeight =
-        AdjustFontSize(metrics.lfMessageFont.lfHeight, 0);
-    HFONT font = CreateFontIndirect(&metrics.lfMessageFont);
-    DLOG_ASSERT(font);
-    base_font_ref_ = PlatformFontWin::CreateHFontRef(font);
-    // base_font_ref_ is global, up the ref count so it's never deleted.
-    base_font_ref_->AddRef();
+// static
+PlatformFontWin::HFontRef* PlatformFontWin::GetBaseFontRef() {
+  if (base_font_ref_ == nullptr) {
+    // We'll delegate to our SystemFonts instance to give us the default
+    // message font.
+    PlatformFontWin* message_font = static_cast<PlatformFontWin*>(
+        SystemFonts::Instance()->GetFont(SystemFont::kMessage).platform_font());
+    base_font_ref_ = message_font->font_ref_.get();
   }
   return base_font_ref_;
 }

ui/gfx/platform_font_win.h

@@ -5,6 +5,8 @@
 #ifndef UI_GFX_PLATFORM_FONT_WIN_H_
 #define UI_GFX_PLATFORM_FONT_WIN_H_
 
+#include <windows.h>
+
 #include <string>
 
 #include "base/compiler_specific.h"
@@ -14,8 +16,6 @@
 #include "ui/gfx/gfx_export.h"
 #include "ui/gfx/platform_font.h"
 
-#include <windows.h>
-
 struct IDWriteFactory;
 struct IDWriteFont;
 
@@ -23,6 +23,14 @@ namespace gfx {
 
 class GFX_EXPORT PlatformFontWin : public PlatformFont {
  public:
+  enum class SystemFont : int {
+    kCaption = 0,
+    kSmallCaption,
+    kMenu,
+    kStatus,
+    kMessage
+  };
+
   PlatformFontWin();
   explicit PlatformFontWin(NativeFont native_font);
   PlatformFontWin(const std::string& font_name, int font_size);
@@ -39,14 +47,15 @@ class GFX_EXPORT PlatformFontWin : public PlatformFont {
   // Callback that returns the minimum height that should be used for
   // gfx::Fonts. Optional. If not specified, the minimum font size is 0.
   typedef int (*GetMinimumFontSizeCallback)();
-  static GetMinimumFontSizeCallback get_minimum_font_size_callback;
+  static void SetGetMinimumFontSizeCallback(
+      GetMinimumFontSizeCallback callback);
 
   // Callback that adjusts a LOGFONT to meet suitability requirements of the
   // embedding application. Optional. If not specified, no adjustments are
   // performed other than clamping to a minimum font height if
   // |get_minimum_font_size_callback| is specified.
   typedef void (*AdjustFontCallback)(LOGFONT* lf);
-  static AdjustFontCallback adjust_font_callback;
+  static void SetAdjustFontCallback(AdjustFontCallback callback);
 
   // Returns the font name for the system locale. Some fonts, particularly
   // East Asian fonts, have different names per locale. If the localized font
@@ -75,13 +84,7 @@ class GFX_EXPORT PlatformFontWin : public PlatformFont {
 
   static bool IsDirectWriteEnabled();
 
-  // Returns the GDI metrics for the font passed in.
-  static void GetTextMetricsForFont(HDC hdc,
-                                    HFONT font,
-                                    TEXTMETRIC* text_metrics);
-
-  // Returns the size of the font based on the font information passed in.
-  static int GetFontSize(const LOGFONT& font_info);
+  static const Font& GetSystemFont(SystemFont system_font);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(RenderTextHarfBuzzTest, HarfBuzz_UniscribeFallback);
@@ -168,6 +171,11 @@ class GFX_EXPORT PlatformFontWin : public PlatformFont {
   void InitWithFontNameAndSize(const std::string& font_name,
                                int font_size);
 
+  // Returns the GDI metrics for the font passed in.
+  static void GetTextMetricsForFont(HDC hdc,
+                                    HFONT font,
+                                    TEXTMETRIC* text_metrics);
+
   // Returns the base font ref. This should ONLY be invoked on the
   // UI thread.
   static HFontRef* GetBaseFontRef();

ui/views/controls/menu/menu_config_win.cc

@@ -10,10 +10,8 @@
 
 #include "base/logging.h"
 #include "base/win/scoped_gdi_object.h"
-#include "base/win/win_client_metrics.h"
-#include "ui/base/l10n/l10n_util_win.h"
-#include "ui/display/win/dpi.h"
 #include "ui/gfx/color_utils.h"
+#include "ui/gfx/platform_font_win.h"
 #include "ui/native_theme/native_theme_win.h"
 
 using ui::NativeTheme;
@@ -22,16 +20,9 @@ namespace views {
 
 void MenuConfig::Init() {
   arrow_color = color_utils::GetSysSkColor(COLOR_MENUTEXT);
+  font_list = gfx::FontList(gfx::PlatformFontWin::GetSystemFont(
+      gfx::PlatformFontWin::SystemFont::kMenu));
 
-  NONCLIENTMETRICS_XP metrics;
-  base::win::GetNonClientMetrics(&metrics);
-  display::win::AdjustFontForAccessibility(&metrics.lfMenuFont);
-  l10n_util::AdjustUIFont(&(metrics.lfMenuFont));
-  {
-    base::win::ScopedHFONT new_font(CreateFontIndirect(&metrics.lfMenuFont));
-    DLOG_ASSERT(new_font.is_valid());
-    font_list = gfx::FontList(gfx::Font(new_font.get()));
-  }
   NativeTheme::ExtraParams extra;
   gfx::Size arrow_size = NativeTheme::GetInstanceForNativeUi()->GetPartSize(
       NativeTheme::kMenuPopupArrow, NativeTheme::kNormal, extra);

ui/views/widget/native_widget_aura.cc

@@ -4,6 +4,9 @@
 
 #include "ui/views/widget/native_widget_aura.h"
 
+#include <memory>
+#include <utility>
+
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
@@ -57,9 +60,7 @@
 
 #if defined(OS_WIN)
 #include "base/win/scoped_gdi_object.h"
-#include "base/win/win_client_metrics.h"
-#include "ui/base/l10n/l10n_util_win.h"
-#include "ui/display/win/dpi.h"
+#include "ui/gfx/platform_font_win.h"
 #include "ui/views/widget/desktop_aura/desktop_window_tree_host_win.h"
 #endif
 
@@ -1227,12 +1228,8 @@ void NativeWidgetPrivate::ReparentNativeView(gfx::NativeView native_view,
 // static
 gfx::FontList NativeWidgetPrivate::GetWindowTitleFontList() {
 #if defined(OS_WIN)
-  NONCLIENTMETRICS_XP ncm;
-  base::win::GetNonClientMetrics(&ncm);
-  display::win::AdjustFontForAccessibility(&(ncm.lfCaptionFont));
-  l10n_util::AdjustUIFont(&(ncm.lfCaptionFont));
-  base::win::ScopedHFONT caption_font(CreateFontIndirect(&(ncm.lfCaptionFont)));
-  return gfx::FontList(gfx::Font(caption_font.get()));
+  return gfx::FontList(gfx::PlatformFontWin::GetSystemFont(
+      gfx::PlatformFontWin::SystemFont::kCaption));
 #else
   return gfx::FontList();
 #endif