Implement fallback cache

content/child/dwrite_font_proxy/font_fallback_win.cc

Index: content/child/dwrite_font_proxy/font_fallback_win.cc
diff --git a/content/child/dwrite_font_proxy/font_fallback_win.cc b/content/child/dwrite_font_proxy/font_fallback_win.cc
index 46588c7066a5c89efb61887c0eff125bd4e97b02..8a53bccbcca2dfcc2abe626a19b6aa3a45adf320 100644
--- a/content/child/dwrite_font_proxy/font_fallback_win.cc
+++ b/content/child/dwrite_font_proxy/font_fallback_win.cc
@@ -4,8 +4,11 @@
 
 #include "content/child/dwrite_font_proxy/font_fallback_win.h"
 
-#include "base/strings/string16.h"
+#include <algorithm>
 
+#include "base/metrics/histogram_macros.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversion_utils.h"
 #include "content/child/dwrite_font_proxy/dwrite_font_proxy_win.h"
 #include "content/common/dwrite_font_proxy_messages.h"
 #include "content/public/child/child_thread.h"
@@ -15,6 +18,29 @@ namespace mswr = Microsoft::WRL;
 
 namespace content {
 
+namespace {
+
+const size_t kMaxFamilyCacheSize = 10;
+
+// This enum is used to define the buckets for an enumerated UMA histogram.
+// Hence,
+//   (a) existing enumerated constants should never be deleted or reordered, and
+//   (b) new constants should only be appended at the end of the enumeration.
+enum DirectWriteFontFallbackResult {
+  FAILED_NO_FONT = 0,
+  SUCCESS_CACHE = 1,
+  SUCCESS_IPC = 2,
+
+  FONT_FALLBACK_RESULT_MAX_VALUE
+};
+
+void LogFallbackResult(DirectWriteFontFallbackResult fallback_result) {
+  UMA_HISTOGRAM_ENUMERATION("DirectWrite.Fonts.Proxy.FallbackResult",
+                            fallback_result, FONT_FALLBACK_RESULT_MAX_VALUE);
+}
+
+}  // namespace
+
 FontFallback::FontFallback() = default;
 FontFallback::~FontFallback() = default;
 
@@ -39,7 +65,19 @@ HRESULT FontFallback::MapCharacters(IDWriteTextAnalysisSource* source,
     DCHECK(false);
     return E_FAIL;
   }
-  base::string16 text_chunk(text, chunk_length);
+  base::string16 text_chunk(text, std::min(chunk_length, text_length));
+
+  base_family_name = base_family_name ? base_family_name : L"";
+
+  if (GetCachedFont(text_chunk, base_family_name, base_weight, base_style,

[scottmg, 2016/04/21 22:45:49]

It looks like if text_chunk.size() == 0, this will return false. Can that
happen? and/or is that what you want?

[Ilya Kulshin, 2016/04/22 00:48:37]

That would mean we were asked to map an empty string, and doesn't really make
sense. Added a check to bail on the whole thing if it happens.

+                    base_stretch, mapped_font, mapped_length)) {
+    DCHECK(*mapped_font);
+    DCHECK_GT(*mapped_length, 0u);

[scottmg, 2016/04/21 22:45:49]

You didn't cache or return *scale here. Are you certain that's not necessary?

[Ilya Kulshin, 2016/04/22 00:48:37]

I've never (and by never I mean "in the past month or so") seen a scale other
than 1, and Skia ignores |scale| anyway (in fact, it ignores everything other
than the family name it obtains from the font). So while we could cache |scale|,
I don't think the added complexity is justified. In the meantime, I added a
DCHECK and a comment.

+    LogFallbackResult(SUCCESS_CACHE);
+    return S_OK;
+  }
+
+  TRACE_EVENT0("dwrite", "FontFallback::MapCharacters (IPC)");
 
   const WCHAR* locale = nullptr;
   // |locale_text_length| is actually the length of text with the locale, not
@@ -48,8 +86,7 @@ HRESULT FontFallback::MapCharacters(IDWriteTextAnalysisSource* source,
   source->GetLocaleName(text_position /*textPosition*/, &locale_text_length,
                         &locale);
 
-  if (locale == nullptr)
-    locale = L"";
+  locale = locale ? locale : L"";
 
   DWriteFontStyle style;
   style.font_weight = base_weight;
@@ -62,14 +99,18 @@ HRESULT FontFallback::MapCharacters(IDWriteTextAnalysisSource* source,
       sender_override_ ? sender_override_ : ChildThread::Get();
   if (!sender->Send(new DWriteFontProxyMsg_MapCharacters(
           text_chunk, style, locale, source->GetParagraphReadingDirection(),
-          base_family_name ? base_family_name : L"", &result)))
+          base_family_name, &result))) {
+    DCHECK(false);
     return E_FAIL;
+  }
 
   *mapped_length = result.mapped_length;
   *scale = result.scale;
 
-  if (result.family_index == UINT32_MAX)
+  if (result.family_index == UINT32_MAX) {
+    LogFallbackResult(FAILED_NO_FONT);
     return S_OK;
+  }
 
   mswr::ComPtr<IDWriteFontFamily> family;
   // It would be nice to find a way to determine at runtime if |collection_| is
@@ -93,7 +134,8 @@ HRESULT FontFallback::MapCharacters(IDWriteTextAnalysisSource* source,
   }
 
   DCHECK(*mapped_font);
-
+  AddCachedFamily(std::move(family), base_family_name);
+  LogFallbackResult(SUCCESS_IPC);
   return S_OK;
 }
 
@@ -105,4 +147,71 @@ FontFallback::RuntimeClassInitialize(DWriteFontCollectionProxy* collection,
   return S_OK;
 }
 
+bool FontFallback::GetCachedFont(const base::string16& text,
+                                 const wchar_t* base_family_name,
+                                 DWRITE_FONT_WEIGHT base_weight,
+                                 DWRITE_FONT_STYLE base_style,
+                                 DWRITE_FONT_STRETCH base_stretch,
+                                 IDWriteFont** font,
+                                 uint32_t* mapped_length) {
+  std::map<base::string16, std::list<mswr::ComPtr<IDWriteFontFamily>>>::iterator
+      it = fallback_family_cache_.find(base_family_name);
+  if (it == fallback_family_cache_.end())
+    return false;
+
+  TRACE_EVENT0("dwrite", "FontFallback::GetCachedFont");
+
+  std::list<mswr::ComPtr<IDWriteFontFamily>>& family_list = it->second;
+  std::list<mswr::ComPtr<IDWriteFontFamily>>::iterator family_iterator;
+  for (family_iterator = family_list.begin();
+       family_iterator != family_list.end(); ++family_iterator) {
+    mswr::ComPtr<IDWriteFont> matched_font;
+    (*family_iterator)->GetFirstMatchingFont(base_weight, base_stretch,
+                                             base_style, &matched_font);
+
+    // |character_index| tracks how much of the string we have read. This is
+    // different from |mapped_length| because ReadUnicodeCharacter can andvance

[scottmg, 2016/04/21 22:45:49]

andvance -> advance

[Ilya Kulshin, 2016/04/22 00:48:37]

Done.

+    // |character_index| even if the character cannot be mapped (invalid
+    // surrogate pair or font does not contain a matching glyph).
+    int32_t character_index = 0;
+    uint32_t length = 0;  // How much of the text can actually be mapped.
+    while (static_cast<uint32_t>(character_index) < text.length()) {
+      BOOL exists = false;
+      uint32_t character = 0;
+      if (!base::ReadUnicodeCharacter(text.c_str(), text.length(),
+                                      &character_index, &character))
+        break;
+      if (FAILED(matched_font->HasCharacter(character, &exists)) || !exists)
+        break;
+      character_index++;
+      length = character_index;
+    }
+
+    if (length > 0) {
+      // Move the current family to the front of the list
+      family_list.splice(family_list.begin(), family_list, family_iterator);
+
+      matched_font.CopyTo(font);
+      *mapped_length = length;
+      return true;
+    }
+  }
+
+  return false;
+}
+
+void FontFallback::AddCachedFamily(
+    Microsoft::WRL::ComPtr<IDWriteFontFamily> family,
+    const wchar_t* base_family_name) {
+  std::list<mswr::ComPtr<IDWriteFontFamily>>& family_list =
+      fallback_family_cache_[base_family_name];
+  family_list.push_front(std::move(family));
+
+  UMA_HISTOGRAM_COUNTS_100("DirectWrite.Fonts.Proxy.Fallback.CacheSize",
+                           family_list.size());
+
+  if (family_list.size() > kMaxFamilyCacheSize)
+    family_list.resize(kMaxFamilyCacheSize);

[scottmg, 2016/04/21 22:45:49]

Can this just be pop_back()? I know the size is only 10 currently, but O(N)
resize() makes me jumpy.

[Ilya Kulshin, 2016/04/22 00:48:37]

Done.

+}
+
 }  // namespace content