Return HRESULT from font file utility functions

Preparation for UMA logging failure reasons on why extracting font file
and name information failed for individual indexed font files.

Bug: 1009402
Change-Id: Ia6133d2646b9f1610edd5f1e8b4a43a3a0143486
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875106Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709140}

content/browser/renderer_host/dwrite_font_file_util_win.cc

@@ -17,7 +17,7 @@
 
 namespace content {
 
-bool FontFilePathAndTtcIndex(IDWriteFont* font,
+HRESULT FontFilePathAndTtcIndex(IDWriteFont* font,
                                 base::string16& file_path,
                                 uint32_t& ttc_index) {
   Microsoft::WRL::ComPtr<IDWriteFontFace> font_face;
@@ -28,12 +28,12 @@ bool FontFilePathAndTtcIndex(IDWriteFont* font,
                              hr);
     LogMessageFilterError(
         MessageFilterError::ADD_FILES_FOR_FONT_CREATE_FACE_FAILED);
-    return false;
+    return hr;
   }
   return FontFilePathAndTtcIndex(font_face.Get(), file_path, ttc_index);
 }
 
-bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
+HRESULT FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
                                 base::string16& file_path,
                                 uint32_t& ttc_index) {
   TRACE_EVENT0("dwrite,fonts",
@@ -44,7 +44,7 @@ bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
   if (FAILED(hr)) {
     LogMessageFilterError(
         MessageFilterError::ADD_FILES_FOR_FONT_GET_FILE_COUNT_FAILED);
-    return false;
+    return hr;
   }
 
   // We've learned from the DirectWrite team at MS that the number of font files
@@ -58,7 +58,7 @@ bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
   if (file_count > 1) {
     LogMessageFilterError(
         MessageFilterError::GET_FILE_COUNT_INVALID_NUMBER_OF_FILES);
-    return false;
+    return kErrorFontFileUtilTooManyFilesPerFace;
   }
 
   Microsoft::WRL::ComPtr<IDWriteFontFile> font_file;
@@ -66,7 +66,7 @@ bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
   if (FAILED(hr)) {
     LogMessageFilterError(
         MessageFilterError::ADD_FILES_FOR_FONT_GET_FILES_FAILED);
-    return false;
+    return hr;
   }
 
   Microsoft::WRL::ComPtr<IDWriteFontFileLoader> loader;
@@ -74,7 +74,7 @@ bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
   if (FAILED(hr)) {
     LogMessageFilterError(
         MessageFilterError::ADD_FILES_FOR_FONT_GET_LOADER_FAILED);
-    return false;
+    return hr;
   }
 
   Microsoft::WRL::ComPtr<IDWriteLocalFontFileLoader> local_loader;
@@ -92,10 +92,10 @@ bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
     // we could proxy the stream reads directly instead.
     LogLoaderType(DirectWriteFontLoaderType::OTHER_LOADER);
     DCHECK(false);
-    return false;
+    return hr;
   } else if (FAILED(hr)) {
     LogMessageFilterError(MessageFilterError::ADD_FILES_FOR_FONT_QI_FAILED);
-    return false;
+    return hr;
   }
 
   const void* key;
@@ -104,7 +104,7 @@ bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
   if (FAILED(hr)) {
     LogMessageFilterError(
         MessageFilterError::ADD_LOCAL_FILE_GET_REFERENCE_KEY_FAILED);
-    return false;
+    return hr;
   }
 
   UINT32 path_length = 0;
@@ -112,7 +112,7 @@ bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
   if (FAILED(hr)) {
     LogMessageFilterError(
         MessageFilterError::ADD_LOCAL_FILE_GET_PATH_LENGTH_FAILED);
-    return false;
+    return hr;
   }
   base::string16 retrieve_file_path;
   retrieve_file_path.resize(
@@ -121,36 +121,37 @@ bool FontFilePathAndTtcIndex(IDWriteFontFace* font_face,
                                         path_length);
   if (FAILED(hr)) {
     LogMessageFilterError(MessageFilterError::ADD_LOCAL_FILE_GET_PATH_FAILED);
-    return false;
+    return hr;
   }
   // No need for the null-terminator in base::string16.
   retrieve_file_path.resize(--path_length);
 
   uint32_t retrieve_ttc_index = font_face->GetIndex();
   if (FAILED(hr)) {
-    return false;
+    return hr;
   }
 
   file_path = retrieve_file_path;
   ttc_index = retrieve_ttc_index;
 
-  return true;
+  return S_OK;
 }
 
-bool AddFilesForFont(IDWriteFont* font,
+HRESULT AddFilesForFont(IDWriteFont* font,
                         const base::string16& windows_fonts_path,
                         std::set<base::string16>* path_set,
                         std::set<base::string16>* custom_font_path_set,
                         uint32_t* ttc_index) {
   base::string16 file_path;
-  if (!FontFilePathAndTtcIndex(font, file_path, *ttc_index)) {
-    return false;
+  HRESULT hr = FontFilePathAndTtcIndex(font, file_path, *ttc_index);
+  if (FAILED(hr)) {
+    return hr;
   }
 
   base::string16 file_path_folded = base::i18n::FoldCase(file_path);
 
   if (!file_path_folded.size())
-    return false;
+    return kErrorFontFileUtilEmptyFilePath;
 
   if (!base::StartsWith(file_path_folded, windows_fonts_path,
                         base::CompareCase::SENSITIVE)) {
@@ -160,7 +161,7 @@ bool AddFilesForFont(IDWriteFont* font,
     LogLoaderType(DirectWriteFontLoaderType::FILE_SYSTEM_FONT_DIR);
     path_set->insert(file_path);
   }
-  return true;
+  return S_OK;
 }
 
 base::string16 GetWindowsFontsPath() {

content/browser/renderer_host/dwrite_font_file_util_win.h

@@ -15,13 +15,20 @@
 
 namespace content {
 
-bool FontFilePathAndTtcIndex(IDWriteFontFace* font,
+// Custom error codes potentially emitted from FontFilePathAndTtcIndex and
+// AddFilesForFont.
+const HRESULT kErrorFontFileUtilTooManyFilesPerFace =
+    MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, 0xD001);
+const HRESULT kErrorFontFileUtilEmptyFilePath =
+    MAKE_HRESULT(SEVERITY_ERROR, FACILITY_ITF, 0xD002);
+
+HRESULT FontFilePathAndTtcIndex(IDWriteFontFace* font,
                                 base::string16& file_path,
                                 uint32_t& ttc_index);
-bool FontFilePathAndTtcIndex(IDWriteFont* font,
+HRESULT FontFilePathAndTtcIndex(IDWriteFont* font,
                                 base::string16& file_path,
                                 uint32_t& ttc_index);
-bool AddFilesForFont(IDWriteFont* font,
+HRESULT AddFilesForFont(IDWriteFont* font,
                         const base::string16& windows_fonts_path,
                         std::set<base::string16>* path_set,
                         std::set<base::string16>* custom_font_path_set,

content/browser/renderer_host/dwrite_font_lookup_table_builder_win.cc

@@ -469,8 +469,9 @@ DWriteFontLookupTableBuilder::ExtractPathAndNamesFromFamily(
     {
       base::ScopedBlockingCall scoped_blocking_call(
           FROM_HERE, base::BlockingType::MAY_BLOCK);
-      if (!AddFilesForFont(font.Get(), *windows_fonts_path, &path_set,
-                           &custom_font_path_set, &ttc_index)) {
+      hr = AddFilesForFont(font.Get(), *windows_fonts_path, &path_set,
+                           &custom_font_path_set, &ttc_index);
+      if (FAILED(hr)) {
         // It's possible to not be able to retrieve a font file for a font that
         // is in the system font collection, see https://crbug.com/922183. If we
         // were not able to retrieve a file for a registered font, we do not

content/browser/renderer_host/dwrite_font_proxy_impl_win.cc

@@ -473,10 +473,10 @@ void DWriteFontProxyImpl::MatchUniqueFont(
 
   base::string16 font_file_pathname;
   uint32_t ttc_index;
-  bool result = FontFilePathAndTtcIndex(first_font_face.Get(),
-                                        font_file_pathname, ttc_index);
-  if (!result)
+  if (FAILED(FontFilePathAndTtcIndex(first_font_face.Get(), font_file_pathname,
+                                     ttc_index))) {
     return;
+  }
 
   base::FilePath path(base::UTF16ToWide(font_file_pathname));
   std::move(callback).Run(path, ttc_index);