Use DirectWrite to populate list of avalible fonts

content/common/font_list_win.cc

Index: content/common/font_list_win.cc
diff --git a/content/common/font_list_win.cc b/content/common/font_list_win.cc
index bc6c1dcd1ed296bc5f336dce9b4c7b9f8d17121b..5d346905877d82893f14f8711ebe2ed9d4e63aad 100644
--- a/content/common/font_list_win.cc
+++ b/content/common/font_list_win.cc
@@ -5,11 +5,13 @@
 #include "content/common/font_list.h"
 
 #include <windows.h>
+#include <dwrite.h>
 
 #include <set>
 
 #include "base/strings/string16.h"
 #include "base/values.h"
+#include "content/common/sandbox_win.h"
 
 namespace content {
 
@@ -29,7 +31,7 @@ static int CALLBACK EnumFontFamExProc(ENUMLOGFONTEXW* logical_font,
   return 1;
 }
 
-scoped_ptr<base::ListValue> GetFontList_SlowBlocking() {
+static void GetFontListInternal_GDI(scoped_ptr<base::ListValue>& font_list) {

[scottmg, 2014/06/26 22:45:27]

no & here

   std::set<base::string16> font_names;
 
   LOGFONTW logfont;
@@ -41,7 +43,6 @@ scoped_ptr<base::ListValue> GetFontList_SlowBlocking() {
                         (LPARAM)&font_names, 0);
   ::ReleaseDC(NULL, hdc);
 
-  scoped_ptr<base::ListValue> font_list(new base::ListValue);
   std::set<base::string16>::iterator iter;
   for (iter = font_names.begin(); iter != font_names.end(); ++iter) {
     base::ListValue* font_item = new base::ListValue();
@@ -49,6 +50,96 @@ scoped_ptr<base::ListValue> GetFontList_SlowBlocking() {
     font_item->Append(new base::StringValue(*iter));
     font_list->Append(font_item);
   }
+}
+
+static void AddLocalizedFontFamily(scoped_ptr<base::ListValue>& font_list,

[scottmg, 2014/06/26 22:45:27]

no & here

+                                   IDWriteFontFamily* font_family,
+                                   wchar_t* user_locale) {
+  IDWriteLocalizedStrings* family_names = NULL;
+  if (SUCCEEDED(font_family->GetFamilyNames(&family_names))) {

[scottmg, 2014/06/26 22:45:27]

gdi skips fonts starting with @ (vertical orientation or something?), do we need
to do that here?

[eae, 2014/06/26 23:14:57]

We go out of our way to check for the @ prefix in blink code regardless of
rendering path so yeah we should probably do the same here. Good call.

+    UINT32 index = 0;
+    BOOL exists = false;
+
+    // Try to get name of font in the users locale first, failing that fall back
+    // on US English and then the first name listed.
+    family_names->FindLocaleName(user_locale, &index, &exists);
+    if (!exists)
+      family_names->FindLocaleName(L"en-us", &index, &exists);
+    if (!exists)
+      index = 0;
+
+    UINT32 len = 0;
+    if (SUCCEEDED(family_names->GetStringLength(index, &len))) {
+      wchar_t* name = new wchar_t[len + 1];
+      if (name) {
+        if (SUCCEEDED(family_names->GetString(index, name, len + 1))) {
+          base::ListValue* font_item = new base::ListValue();
+          font_item->Append(new base::StringValue(name));

[scottmg, 2014/06/26 22:45:27]

i assume 2x is correct since gdi does too, maybe a comment as to why we pass the
same thing twice

+          font_item->Append(new base::StringValue(name));
+          font_list->Append(font_item);
+        }
+        delete[] name;
+      }
+    }
+  }
+  if (family_names)

[scottmg, 2014/06/26 22:45:27]

could this just not go inside the if (SUCCEEDED(GetFamilyNames)) block?

[eae, 2014/06/26 23:14:57]

The documentation for GetFamilyNames is unclear on whether a failure  can result
in the output parameter being constructed. I'd rather err or the side of
caution.

+    family_names->Release();
+}
+
+static void CreateDirectWriteFactory(IDWriteFactory** factory) {
+  typedef decltype(DWriteCreateFactory) * DWriteCreateFactoryProc;
+  HMODULE dwrite_dll = LoadLibraryW(L"dwrite.dll");
+  if (dwrite_dll) {
+    DWriteCreateFactoryProc dwrite_create_factory_proc =
+        reinterpret_cast<DWriteCreateFactoryProc>(
+            GetProcAddress(dwrite_dll, "DWriteCreateFactory"));
+    if (dwrite_create_factory_proc) {
+      dwrite_create_factory_proc(DWRITE_FACTORY_TYPE_ISOLATED,
+                                 __uuidof(IDWriteFactory),
+                                 reinterpret_cast<IUnknown**>(factory));
+    }
+  }
+}
+
+static void GetFontListInternal_DirectWrite(
+    scoped_ptr<base::ListValue>& font_list) {

[scottmg, 2014/06/26 22:45:27]

no & here

+  wchar_t user_locale[LOCALE_NAME_MAX_LENGTH];
+  GetUserDefaultLocaleName(user_locale, LOCALE_NAME_MAX_LENGTH);
+
+  IDWriteFactory* factory = NULL;
+  CreateDirectWriteFactory(&factory);
+
+  IDWriteFontCollection* font_collection = NULL;
+  BOOL checkForUpdates = false;

[scottmg, 2014/06/26 22:45:27]

checkForUpdates -> check_for_updates

[eae, 2014/06/26 23:14:57]

Thanks, haven't written chromium style code in awhile.

+  if (factory && SUCCEEDED(factory->GetSystemFontCollection(&font_collection,
+                                                            checkForUpdates))) {
+    UINT32 family_count = font_collection->GetFontFamilyCount();
+    for (UINT32 i = 0; i < family_count; i++) {
+      IDWriteFontFamily* font_family = NULL;
+      if (SUCCEEDED(font_collection->GetFontFamily(i, &font_family)))
+        AddLocalizedFontFamily(font_list, font_family, user_locale);
+      if (font_family)
+        font_family->Release();

[scottmg, 2014/06/26 22:45:27]

move inside if (SUCCEEDED())?

[eae, 2014/06/26 23:14:57]

Again, it is unclear whether it is guaranteed not to be constructor on failure.

+    }
+  }
+
+  if (font_collection)

[scottmg, 2014/06/26 22:45:27]

can this move inside the GetSystemFontCollection block and be unconditional?

+    font_collection->Release();
+  if (factory)
+    factory->Release();
+}
+
+scoped_ptr<base::ListValue> GetFontList_SlowBlocking() {
+  scoped_ptr<base::ListValue> font_list(new base::ListValue);
+
+  if (ShouldUseDirectWrite())
+    GetFontListInternal_DirectWrite(font_list);
+
+  // Fallback on GDI if DirectWrite isn't enabled or if it failed to populate
+  // the font list.
+  if (font_list->GetSize() < 1)

[scottmg, 2014/06/26 22:45:27]

nit (optional); == 0 seems clearer than < 1 to me

[eae, 2014/06/26 23:14:57]

I agree.

+    GetFontListInternal_GDI(font_list);
+
   return font_list.Pass();
 }