Require certain faimilies to contain certain styles

content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc

Index: content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc
diff --git a/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc b/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc
index eccace7f0d93d58ab1ba688f6f02d0eff6909b67..53d423008ae6cb447e81f84d68e2c98c8bb72018 100644
--- a/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc
+++ b/content/browser/renderer_host/dwrite_font_proxy_message_filter_win.cc
@@ -41,6 +41,7 @@ enum DirectWriteFontLoaderType {
   FILE_SYSTEM_FONT_DIR = 0,
   FILE_OUTSIDE_SANDBOX = 1,
   OTHER_LOADER = 2,
+  FONT_WITH_MISSING_REQUIRED_STYLES = 3,
 
   FONT_LOADER_TYPE_MAX_VALUE
 };
@@ -131,6 +132,61 @@ const base::Feature kForceCustomFonts {
   "ForceDirectWriteCustomFonts", base::FEATURE_DISABLED_BY_DEFAULT
 };
 
+struct RequiredFontStyle {
+  const wchar_t* family_name;
+  DWRITE_FONT_WEIGHT required_weight;
+  DWRITE_FONT_STRETCH required_stretch;
+  DWRITE_FONT_STYLE required_style;
+};
+
+const RequiredFontStyle kRequiredStyles[] = {
+    {L"open sans", DWRITE_FONT_WEIGHT_NORMAL, DWRITE_FONT_STRETCH_NORMAL,
+     DWRITE_FONT_STYLE_NORMAL},
+    {L"helvetica", DWRITE_FONT_WEIGHT_NORMAL, DWRITE_FONT_STRETCH_NORMAL,
+     DWRITE_FONT_STYLE_NORMAL},

[ananta, 2016/08/19 21:19:49]

Is this list likely to grow?. Do IE and Firefox do something like this

[Ilya Kulshin, 2016/08/19 21:29:42]

I think we will want to add 'Gill Sans' to the list, since we already have a
similar mechanism that excludes certain Gill Sans styles completely (that
approach is more difficult to implement in this case, since I don't want to
blanket-ban Helvetica Bold and Open Sans Condensed, and I don't know the full
set of Helvetica and Open Sans styles anyway). I didn't make that change in this
patch to keep this straightforward. I did an informal survey on the forums and
did not get any reports of anything other than Open Sans and Helvetica (and
Arial, but that's a different bug).

Longer term, I want to do a more comprehensive solution similar to
https://codereview.chromium.org/2248693003/ so I don't anticipate this list
growing much.

Don't know about IE/Firefox, but Edge somehow also manages to exclude Open Sans
if it is only available in the condensed style.

+};
+
+// As a workaround for crbug.com/635932, refuse to load some common fonts that
+// do not contain certain styles. We found that sometimes these fonts are
+// installed only in specialized styles ('Open Sans' might only be available in
+// the condensed light variant, or Helvetica might only be available in bold).
+// That results in a poor user experience because websites that use those fonts
+// usually expect them to be rendered in the regular variant.
+bool CheckRequiredStylesPresent(IDWriteFontCollection* collection,
+                                const base::string16& family_name,
+                                uint32_t family_index) {
+  for (const auto& font_style : kRequiredStyles) {
+    if (base::EqualsCaseInsensitiveASCII(family_name, font_style.family_name)) {
+      mswr::ComPtr<IDWriteFontFamily> family;
+      if (FAILED(collection->GetFontFamily(family_index, &family))) {
+        DCHECK(false);
+        return true;
+      }
+      mswr::ComPtr<IDWriteFont> font;
+      if (FAILED(family->GetFirstMatchingFont(
+          font_style.required_weight, font_style.required_stretch,
+          font_style.required_style, &font))) {
+        DCHECK(false);
+        return true;
+      }
+
+      // GetFirstMatchingFont doesn't require strict style matching, so check
+      // the actual font that we got.
+      if (font->GetWeight() != font_style.required_weight ||
+          font->GetStretch() != font_style.required_stretch ||
+          font->GetStyle() != font_style.required_style) {
+        // Not really a loader type, but good to have telemetry on how often
+        // fonts like these are encountered, and the data can be compared with
+        // the other loader types.
+        LogLoaderType(FONT_WITH_MISSING_REQUIRED_STYLES);
+        return false;
+      }
+      break;
+    }
+  }
+  return true;
+}
+
 }  // namespace
 
 DWriteFontProxyMessageFilter::DWriteFontProxyMessageFilter()
@@ -178,8 +234,10 @@ void DWriteFontProxyMessageFilter::OnFindFamily(
     UINT32 index = UINT32_MAX;
     HRESULT hr =
         collection_->FindFamilyName(family_name.data(), &index, &exists);
-    if (SUCCEEDED(hr) && exists)
+    if (SUCCEEDED(hr) && exists &&
+        CheckRequiredStylesPresent(collection_.Get(), family_name, index)) {
       *family_index = index;
+    }
   }
 }