FontAccess: Ensure there are no duplicates in enumeration

The use of the system APIs sometimes return duplicates.
This change adds measures to prevent duplicates from occurring in the
enumerations and measures their incidence.

This CL treats fonts having the same postscript name as unique. This
follows what Mac OS does:
https://developer.apple.com/fonts/TrueType-Reference-Manual/RM06/Chap6name.html

And also mirrors the matching behavior in @font-face src: local matching.

Bug: 1043306
Change-Id: I9c8cdd26a47e0f54c4c5c35c2df80f1bed46e4bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438875
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Auto-Submit: Olivier Yiptong <oyiptong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812651}

content/browser/BUILD.gn

@@ -929,10 +929,6 @@ source_set("browser") {
     "font_access/font_access_manager_impl.h",
     "font_access/font_enumeration_cache.cc",
     "font_access/font_enumeration_cache.h",
-    "font_access/font_enumeration_cache_mac.h",
-    "font_access/font_enumeration_cache_mac.mm",
-    "font_access/font_enumeration_cache_win.cc",
-    "font_access/font_enumeration_cache_win.h",
     "font_list_async.cc",
     "generic_sensor/sensor_provider_proxy_impl.cc",
     "generic_sensor/sensor_provider_proxy_impl.h",
@@ -2176,6 +2172,8 @@ source_set("browser") {
     sources += [
       "../app_shim_remote_cocoa/web_contents_ns_view_bridge.h",
       "../app_shim_remote_cocoa/web_contents_ns_view_bridge.mm",
+      "font_access/font_enumeration_cache_mac.h",
+      "font_access/font_enumeration_cache_mac.mm",
       "gpu/ca_transaction_gpu_coordinator.cc",
       "gpu/ca_transaction_gpu_coordinator.h",
       "sandbox_support_mac_impl.h",
@@ -2251,6 +2249,8 @@ source_set("browser") {
 
   if (is_win) {
     sources += [
+      "font_access/font_enumeration_cache_win.cc",
+      "font_access/font_enumeration_cache_win.h",
       "installedapp/installed_app_provider_impl_win.cc",
       "installedapp/installed_app_provider_impl_win.h",
       "renderer_host/virtual_keyboard_controller_win.cc",

content/browser/font_access/font_enumeration_cache_fontconfig.cc

@@ -7,7 +7,7 @@
 #include <fontconfig/fontconfig.h>
 
 #include "base/feature_list.h"
-#include "base/metrics/histogram_macros.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/no_destructor.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
@@ -83,7 +83,7 @@ void FontEnumerationCacheFontconfig::PrepareFontEnumerationCache() {
   // Metrics.
   const base::ElapsedTimer start_timer;
   int incomplete_count = 0;
-  int dupe_count = 0;
+  int duplicate_count = 0;
 
   auto font_enumeration_table = std::make_unique<blink::FontEnumerationTable>();
 
@@ -94,7 +94,7 @@ void FontEnumerationCacheFontconfig::PrepareFontEnumerationCache() {
   std::unique_ptr<FcFontSet, decltype(&FcFontSetDestroy)> fontset(
       ListFonts(object_set.get()), FcFontSetDestroy);
 
-  UMA_HISTOGRAM_CUSTOM_COUNTS(
+  base::UmaHistogramCustomCounts(
       "Fonts.AccessAPI.EnumerationCache.Fontconfig.FontCount", fontset->nfont,
       1, 5000, 50);
 
@@ -120,7 +120,7 @@ void FontEnumerationCacheFontconfig::PrepareFontEnumerationCache() {
     }
 
     if (fonts_seen.count(postscript_name) != 0) {
-      ++dupe_count;
+      ++duplicate_count;
       // Skip duplicates.
       continue;
     }
@@ -137,17 +137,16 @@ void FontEnumerationCacheFontconfig::PrepareFontEnumerationCache() {
     *added_font_meta = metadata;
   }
 
-  UMA_HISTOGRAM_COUNTS_100(
+  base::UmaHistogramCounts100(
       "Fonts.AccessAPI.EnumerationCache.Fontconfig.IncompleteFontCount",
       incomplete_count);
-  UMA_HISTOGRAM_COUNTS_100(
-      "Fonts.AccessAPI.EnumerationCache.Fontconfig.DuplicateFontCount",
-      dupe_count);
+  base::UmaHistogramCounts100(
+      "Fonts.AccessAPI.EnumerationCache.DuplicateFontCount", duplicate_count);
 
   BuildEnumerationCache(std::move(font_enumeration_table));
 
-  UMA_HISTOGRAM_MEDIUM_TIMES("Fonts.AccessAPI.EnumerationTime",
-                             start_timer.Elapsed());
+  base::UmaHistogramMediumTimes("Fonts.AccessAPI.EnumerationTime",
+                                start_timer.Elapsed());
   // Respond to pending and future requests.
   StartCallbacksTaskQueue();
 }

content/browser/font_access/font_enumeration_cache_mac.mm

@@ -9,7 +9,7 @@
 
 #include "base/feature_list.h"
 #include "base/mac/foundation_util.h"
-#include "base/metrics/histogram_macros.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/no_destructor.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/task/task_traits.h"
@@ -81,6 +81,10 @@ void FontEnumerationCacheMac::PrepareFontEnumerationCache() {
     base::ScopedCFTypeRef<CFArrayRef> font_descs(
         CTFontCollectionCreateMatchingFontDescriptors(collection));
 
+    // Used to filter duplicates.
+    std::set<std::string> fonts_seen;
+    int duplicate_count = 0;
+
     for (CFIndex i = 0; i < CFArrayGetCount(font_descs); ++i) {
       CTFontDescriptorRef fd = base::mac::CFCast<CTFontDescriptorRef>(
           CFArrayGetValueAtIndex(font_descs, i));
@@ -91,9 +95,18 @@ void FontEnumerationCacheMac::PrepareFontEnumerationCache() {
       base::ScopedCFTypeRef<CFStringRef> cf_family =
           GetLocalizedString(fd, kCTFontFamilyNameAttribute);
 
+      std::string postscript_name =
+          base::SysCFStringRefToUTF8(cf_postscript_name.get());
+
+      if (fonts_seen.count(postscript_name) != 0) {
+        ++duplicate_count;
+        // Skip duplicates.
+        continue;
+      }
+      fonts_seen.insert(postscript_name);
+
       blink::FontEnumerationTable_FontMetadata metadata;
-      metadata.set_postscript_name(
-          base::SysCFStringRefToUTF8(cf_postscript_name.get()).c_str());
+      metadata.set_postscript_name(postscript_name.c_str());
       metadata.set_full_name(
           base::SysCFStringRefToUTF8(cf_full_name.get()).c_str());
       metadata.set_family(base::SysCFStringRefToUTF8(cf_family.get()).c_str());
@@ -105,8 +118,10 @@ void FontEnumerationCacheMac::PrepareFontEnumerationCache() {
 
     BuildEnumerationCache(std::move(font_enumeration_table));
 
-    UMA_HISTOGRAM_MEDIUM_TIMES("Fonts.AccessAPI.EnumerationTime",
-                               start_timer.Elapsed());
+    base::UmaHistogramCounts100(
+        "Fonts.AccessAPI.EnumerationCache.DuplicateFontCount", duplicate_count);
+    base::UmaHistogramMediumTimes("Fonts.AccessAPI.EnumerationTime",
+                                  start_timer.Elapsed());
     // Respond to pending and future requests.
     StartCallbacksTaskQueue();
   }

content/browser/font_access/font_enumeration_cache_win.cc

@@ -248,7 +248,7 @@ void FontEnumerationCacheWin::PrepareFontEnumerationCache() {
 
     outstanding_family_results_ = collection_->GetFontFamilyCount();
 
-    UMA_HISTOGRAM_CUSTOM_COUNTS(
+    base::UmaHistogramCustomCounts(
         "Fonts.AccessAPI.EnumerationCache.Dwrite.FamilyCount",
         outstanding_family_results_, 1, 5000, 50);
   }
@@ -282,7 +282,20 @@ void FontEnumerationCacheWin::AppendFontDataAndFinalizeIfNeeded(
   if (FAILED(family_data_result->exit_hresult))
     enumeration_errors_[family_data_result->exit_hresult]++;
 
+  // Used to filter duplicates.
+  std::set<std::string> fonts_seen;
+  int duplicate_count = 0;
+
   for (const auto& font_meta : family_data_result->fonts) {
+    const std::string& postscript_name = font_meta.postscript_name();
+
+    if (fonts_seen.count(postscript_name) != 0) {
+      ++duplicate_count;
+      // Skip duplicates.
+      continue;
+    }
+    fonts_seen.insert(postscript_name);
+
     blink::FontEnumerationTable_FontMetadata* added_font_meta =
         font_enumeration_table_->add_fonts();
     *added_font_meta = font_meta;
@@ -291,6 +304,9 @@ void FontEnumerationCacheWin::AppendFontDataAndFinalizeIfNeeded(
   if (!outstanding_family_results_) {
     FinalizeEnumerationCache();
   }
+
+  base::UmaHistogramCounts100(
+      "Fonts.AccessAPI.EnumerationCache.DuplicateFontCount", duplicate_count);
 }
 
 void FontEnumerationCacheWin::FinalizeEnumerationCache() {
@@ -316,8 +332,8 @@ void FontEnumerationCacheWin::FinalizeEnumerationCache() {
       std::move(font_enumeration_table_));
   BuildEnumerationCache(std::move(enumeration_table));
 
-  UMA_HISTOGRAM_MEDIUM_TIMES("Fonts.AccessAPI.EnumerationTime",
-                             enumeration_timer_->Elapsed());
+  base::UmaHistogramMediumTimes("Fonts.AccessAPI.EnumerationTime",
+                                enumeration_timer_->Elapsed());
   enumeration_timer_.reset();
 
   // Respond to pending and future requests.

tools/metrics/histograms/histograms_xml/others/histograms.xml

@@ -5133,6 +5133,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <summary>Time to retrieve the fallback fonts on the system.</summary>
 </histogram>
 
+<histogram name="Fonts.AccessAPI.EnumerationCache.DuplicateFontCount"
+    units="count" expires_after="M90">
+  <owner>oyiptong@chromium.org</owner>
+  <owner>storage-dev@chromium.org</owner>
+  <summary>
+    The number of duplicate fonts returned by system APIs. This is recorded when
+    fonts are enumerated to be returned to script as a result of a call to the
+    Font Access API.
+  </summary>
+</histogram>
+
 <histogram name="Fonts.AccessAPI.EnumerationCache.Dwrite.FamilyCount"
     units="families" expires_after="M90">
   <owner>oyiptong@chromium.org</owner>
@@ -5171,6 +5182,10 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 <histogram
     name="Fonts.AccessAPI.EnumerationCache.Fontconfig.DuplicateFontCount"
     units="count" expires_after="M90">
+  <obsolete>
+    Removed in M86 in favor of
+    Fonts.AccessAPI.EnumerationCache.DuplicateFontCount.
+  </obsolete>
   <owner>oyiptong@chromium.org</owner>
   <owner>storage-dev@chromium.org</owner>
   <summary>The number of duplicate fonts returned by fontconfig.</summary>