Allow the default generic font family settings to find the first available font

chrome/browser/ui/webui/options/font_settings_utils.cc

Index: chrome/browser/ui/webui/options/font_settings_utils.cc
diff --git a/chrome/browser/ui/webui/options/font_settings_utils.cc b/chrome/browser/ui/webui/options/font_settings_utils.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ca2465958767faeaaa0dd5cda1cb1ae4721bee87
--- /dev/null
+++ b/chrome/browser/ui/webui/options/font_settings_utils.cc
@@ -0,0 +1,53 @@
+// Copyright (c) 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/webui/options/font_settings_utils.h"
+
+#include "base/strings/string_split.h"
+#include "third_party/skia/include/core/SkTypeface.h"
+#include "third_party/skia/include/ports/SkFontMgr.h"
+
+namespace options {
+
+bool IsFontFamilyAvailable(const std::string& family, SkFontMgr* fontManager) {
+#if defined(OS_LINUX)
+  sk_sp<SkTypeface> typeface(
+      fontManager->legacyCreateTypeface(family.c_str(), SkFontStyle()));
+  return typeface;
+#else
+  sk_sp<SkFontStyleSet> set(fontManager->matchFamily(family.c_str()));

[msw, 2016/10/27 00:04:18]

How does this non-linux code align with Mac and Windows? It seems like it's
missing the DirectWrite and GDI code for font lookup. Shouldn't that be part of
IsFontFamilyAvailable (run in a nested loop), instead of within
MaybeGetLocalizedFontName, after the loop returns some value. The current
pattern seems like it might miss values that only PlatformFontWin could resolve.
But maybe that is what you actually intend, if the PlatformFontWin would return
a fallback for an invalid font name that you wouldn't want to respect.

For example if |font_name_list| is "OnlyFoundByDirectWrite, GenericFont", then
ResolveFontList would return "GenericFont". Perhaps it should return
"OnlyFoundByDirectWrite" or similar for GDI, but maybe not if those return
values are just going to be fallback fonts... it's hard to say without knowing
more about the caller's intent. I also have no idea about Mac...

[kojii, 2016/10/27 04:50:56]

First, this function needs to return available font visible to Skia, because
chrome setting UI and Blink creates font from the name independently.

Skia doesn't recognize "GenericFont", I added the case to
font_settings_utils_unittest.cc.

MakeFromName does not accept fontManager as a parameter, and matchFamily() is
not supported in Linux (fontconfig), so this looks like the only choice for now.

[msw, 2016/10/27 17:18:11]

Sorry for not being clear, "GenericFont" was meant to be a placeholder for a
very common font name that skia should always find, you don't really need to add
that fake font name to a unit test. I now understand that we don't want to do
separate DirectWrite font lookup, we *just* want fonts that skia can find;
thanks for clarifying.

+  return set && set->count();
+#endif
+}
+
+// Returns the first available font. If there is no available font, returns the
+// first font. Empty entries are ignored.
+std::string FirstAvailableOrFirst(const std::string& font_name_list) {
+  std::vector<std::string> families = base::SplitString(
+      font_name_list, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  if (families.empty())
+    return std::string();
+  sk_sp<SkFontMgr> fm(SkFontMgr::RefDefault());
+  for (auto& family : families) {
+    if (IsFontFamilyAvailable(family, fm.get()))
+      return family;
+  }
+  return families[0];
+}

[Dan Beam, 2016/10/27 02:07:24]

^ why are we duplicating this with the blink code?

[kojii, 2016/10/27 04:50:56]

Two reasons:
1. No good place to share the code. Tried ui::gfx but msw@ disliked in PS12.
Can't find good place in base.
2. Blink string has Latin1/UTF-16 while chrome has UTF-8. Blink calls this
during layout and thus would like to be fast. See eae@'s comment in PS12.

I'm fine to re-try to share the code if you could advice me a good place -- I'm
not very familiar with chromium side yet.

[msw, 2016/10/27 17:18:11]

I'm not sure if ui/gfx makes sense, because this is only used for blink and
webui at the moment, not browser ui... WDYT, Dan?

[Dan Beam, 2016/10/27 17:29:41]

ui/base?

[msw, 2016/10/27 18:24:02]

If ui/ is the right top-level dir, then gfx/ would be the best subdir.
(that matches the location of most skia and text util code in //ui)
Sorry for the churn caused by my uncertainty, I guess ui/gfx is fine.

Perhaps add some explanatory usage comments with these utils, something like
"Used by Blink to pick the primary serif/sans/fixed/etc. fonts from
region-specific IDS lists." (and please correct me if I'm wrong)

[kojii, 2016/10/28 04:59:41]

Blink can't use ui/base (yet?); only ui/gfx and several under base/.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/DEPS

Thank you for the whole discussion, this was really helpful to understand
chromium structures better for me.
You described very accurately, added, thank you again.

+
+std::string FontSettingsUtilities::ResolveFontList(
+    const std::string& font_name_or_list) {
+  if (!font_name_or_list.empty() && font_name_or_list[0] == ',')
+    return FirstAvailableOrFirst(font_name_or_list);
+  return font_name_or_list;
+}
+
+#if !defined(OS_WIN)
+std::string FontSettingsUtilities::MaybeGetLocalizedFontName(
+    const std::string& font_name_or_list) {
+  return ResolveFontList(font_name_or_list);
+}
+#endif
+
+}  // namespace options