Implement support for loading font files from outside system directory

content/child/dwrite_font_proxy/dwrite_font_proxy_win.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/child/dwrite_font_proxy/dwrite_font_proxy_win.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <utility>
 
 #include "base/debug/crash_logging.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/scoped_handle.h"
 #include "content/child/dwrite_font_proxy/dwrite_localized_strings_win.h"
 #include "content/common/dwrite_font_proxy_messages.h"
 #include "content/public/child/child_thread.h"
+#include "ipc/ipc_platform_file.h"
 #include "ipc/ipc_sender.h"
 
 namespace mswr = Microsoft::WRL;
 
 namespace content {
 
 namespace {
 
 // This enum is used to define the buckets for an enumerated UMA histogram.
 // Hence,
@@ skipping 13 matching lines @@
 // 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 FontProxyError {
   FIND_FAMILY_SEND_FAILED = 0,
   GET_FAMILY_COUNT_SEND_FAILED = 1,
   COLLECTION_KEY_INVALID = 2,
   FAMILY_INDEX_OUT_OF_RANGE = 3,
   GET_FONT_FILES_SEND_FAILED = 4,
   MAPPED_FILE_FAILED = 5,
+  DUPLICATE_HANDLE_FAILED = 6,
 
   FONT_PROXY_ERROR_MAX_VALUE
 };
 
 const char kFontKeyName[] = "font_key_name";
 
 void LogLoadFamilyResult(DirectWriteLoadFamilyResult result) {
   UMA_HISTOGRAM_ENUMERATION("DirectWrite.Fonts.Proxy.LoadFamilyResult", result,
                             LOAD_FAMILY_MAX_VALUE);
 }
@@ skipping 121 matching lines @@
 
   if (*family_index >= GetFontFamilyCount()) {
     LogFontProxyError(FAMILY_INDEX_OUT_OF_RANGE);
     return E_INVALIDARG;
   }
 
   // If we already loaded the family we should reuse the existing collection.
   DCHECK(!families_[*family_index]->IsLoaded());
 
   std::vector<base::string16> file_names;
-  if (!GetSender()->Send(
-          new DWriteFontProxyMsg_GetFontFiles(*family_index, &file_names))) {
+  std::vector<IPC::PlatformFileForTransit> file_handles;
+  if (!GetSender()->Send(new DWriteFontProxyMsg_GetFontFiles(
+          *family_index, &file_names, &file_handles))) {
     LogFontProxyError(GET_FONT_FILES_SEND_FAILED);
     return E_FAIL;
   }
 
+  std::vector<HANDLE> handles;
+  file_handles.reserve(file_names.size() + file_handles.size());
+  for (const base::string16& file_name : file_names) {
+    // This leaks the handles, since they are used as the reference key to
+    // CreateFontFileStream, and DirectWrite requires the reference keys to

[ananta, 2016/07/25 22:21:39]

How was this working previously. Comparing the old code, it looks like the
file_names are not being leaked?

[Ilya Kulshin, 2016/07/25 22:38:52]

First, I should point out that we aren't leaking the *memory* for storing the
handles, we're leaking the underlying operating system objects. That set should
be bounded in size, since we only load families that Skia asks for, we only load
each family once, and all the files are unique within the family (there are some
cases when a single file is needed for multiple families - in that case, each
family would create a reference to that file).

As for how it was working before...
As far as I can tell, when CreateStreamFromKey is called DirectWrite internally
copies the fontFileReferenceKeySize bytes pointed to by fontFileReferenceKey.
Since the data was previously just strings, there is no additional cleanup
required and the memory (from our side) could be freed immediately after the
CreateStreamFromKey call. With handles, closing the handle would invalidate it,
causing future attempts by DirectWrite to create the stream to fail (yes, I've
seen calls when sometimes DirectWrite asks the loader for multiple streams per
CreateStreamFromKey invocation). We could free the handles in
DWriteFontCollectionProxy::Unregister(), but that isn't called until process
shutdown and at that point we might as well let the OS handle it, but I can add
the code if you think that's worthwhile.

(no, the IDWriteFontFileLoader does not have a "FreeKey" API)

[ananta, 2016/07/25 23:08:20]

Thanks. Please add some comments in the patchset and here, which indicate that
having font handles outside windows\fonts opened by the browser avoids file
access denied errors in the renderer as the default policies won't allow these
file opens.

+    // remain valid for the lifetime of the loader. The loader is the font
+    // collection proxy, which remains alive for the lifetime of the renderer.
+    HANDLE handle =
+        CreateFile(file_name.c_str(), GENERIC_READ, FILE_SHARE_READ, NULL,
+                   OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+    if (handle != INVALID_HANDLE_VALUE)
+      handles.push_back(handle);
+  }
+  for (const IPC::PlatformFileForTransit& file_handle : file_handles) {
+    handles.push_back(IPC::PlatformFileForTransitToPlatformFile(file_handle));
+  }
+
   HRESULT hr = mswr::MakeAndInitialize<FontFileEnumerator>(
-      font_file_enumerator, factory, this, &file_names);
+      font_file_enumerator, factory, this, &handles);
 
   if (!SUCCEEDED(hr)) {
     DCHECK(false);
     return E_FAIL;
   }
 
   return S_OK;
 }
 
 HRESULT DWriteFontCollectionProxy::CreateStreamFromKey(
     const void* font_file_reference_key,
     UINT32 font_file_reference_key_size,
     IDWriteFontFileStream** font_file_stream) {
-  if (!font_file_reference_key) {
-    return E_FAIL;
-  }
-
-  const base::char16* file_name =
-      reinterpret_cast<const base::char16*>(font_file_reference_key);
-  DCHECK_EQ(font_file_reference_key_size % sizeof(base::char16), 0u);
-  size_t file_name_size =
-      static_cast<size_t>(font_file_reference_key_size) / sizeof(base::char16);
-
-  if (file_name_size == 0 || file_name[file_name_size - 1] != L'\0') {
+  if (font_file_reference_key_size != sizeof(HANDLE)) {
     return E_FAIL;
   }
 
   TRACE_EVENT0("dwrite", "FontFileEnumerator::CreateStreamFromKey");
 
-  mswr::ComPtr<IDWriteFontFileStream> stream;
-  if (!SUCCEEDED(mswr::MakeAndInitialize<FontFileStream>(&stream, file_name))) {
+  HANDLE file_handle =
+      *reinterpret_cast<const HANDLE*>(font_file_reference_key);
+
+  if (file_handle == NULL || file_handle == INVALID_HANDLE_VALUE) {
     DCHECK(false);
     return E_FAIL;
   }
+
+  mswr::ComPtr<FontFileStream> stream;
+  if (!SUCCEEDED(
+          mswr::MakeAndInitialize<FontFileStream>(&stream, file_handle))) {
+    DCHECK(false);
+    return E_FAIL;
+  }
   *font_file_stream = stream.Detach();
   return S_OK;
 }
 
 HRESULT DWriteFontCollectionProxy::RuntimeClassInitialize(
     IDWriteFactory* factory,
     IPC::Sender* sender_override) {
   DCHECK(factory);
 
   factory_ = factory;
@@ skipping 257 matching lines @@
 
   return SUCCEEDED(hr);
 }
 
 FontFileEnumerator::FontFileEnumerator() = default;
 
 FontFileEnumerator::~FontFileEnumerator() = default;
 
 HRESULT FontFileEnumerator::GetCurrentFontFile(IDWriteFontFile** file) {
   DCHECK(file);
-  if (current_file_ >= file_names_.size()) {
+  if (current_file_ >= files_.size()) {
     return E_FAIL;
   }
 
   TRACE_EVENT0("dwrite", "FontFileEnumerator::GetCurrentFontFile");
+
   // CreateCustomFontFileReference ends up calling
   // DWriteFontCollectionProxy::CreateStreamFromKey.
   HRESULT hr = factory_->CreateCustomFontFileReference(
-      reinterpret_cast<const void*>(file_names_[current_file_].c_str()),
-      (file_names_[current_file_].length() + 1) * sizeof(base::char16),
-      loader_.Get() /*IDWriteFontFileLoader*/, file);
+      reinterpret_cast<const void*>(&files_[current_file_]),
+      sizeof(files_[current_file_]), loader_.Get() /*IDWriteFontFileLoader*/,
+      file);
   DCHECK(SUCCEEDED(hr));
   return hr;
 }
 
 HRESULT FontFileEnumerator::MoveNext(BOOL* has_current_file) {
   DCHECK(has_current_file);
 
   TRACE_EVENT0("dwrite", "FontFileEnumerator::MoveNext");
-  if (next_file_ >= file_names_.size()) {
+  if (next_file_ >= files_.size()) {
     *has_current_file = FALSE;
     current_file_ = UINT_MAX;
     return S_OK;
   }
 
   current_file_ = next_file_;
   ++next_file_;
   *has_current_file = TRUE;
   return S_OK;
 }
 
 HRESULT FontFileEnumerator::RuntimeClassInitialize(
     IDWriteFactory* factory,
     IDWriteFontFileLoader* loader,
-    std::vector<base::string16>* file_names) {
+    std::vector<HANDLE>* files) {
   factory_ = factory;
   loader_ = loader;
-  file_names_.swap(*file_names);
-  file_streams_.resize(file_names_.size());
+  files_.swap(*files);
   return S_OK;
 }
 
 FontFileStream::FontFileStream() = default;
 
 FontFileStream::~FontFileStream() = default;
 
 HRESULT FontFileStream::GetFileSize(UINT64* file_size) {
   *file_size = data_.length();
   return S_OK;
@@ skipping 10 matching lines @@
                                          void** fragment_context) {
   if (fragment_offset + fragment_size < fragment_offset)
     return E_FAIL;
   if (fragment_offset + fragment_size > data_.length())
     return E_FAIL;
   *fragment_start = data_.data() + fragment_offset;
   *fragment_context = nullptr;
   return S_OK;
 }
 
-HRESULT FontFileStream::RuntimeClassInitialize(
-    const base::string16& file_name) {
-  data_.Initialize(base::FilePath(file_name));
+HRESULT FontFileStream::RuntimeClassInitialize(HANDLE handle) {
+  // Duplicate the original handle so we can reopen the file after the memory
+  // mapped section closes it.
+  HANDLE duplicate_handle;
+  if (!DuplicateHandle(GetCurrentProcess(), handle, GetCurrentProcess(),
+                       &duplicate_handle, 0 /* dwDesiredAccess */,
+                       false /* bInheritHandle */, DUPLICATE_SAME_ACCESS)) {
+    LogFontProxyError(DUPLICATE_HANDLE_FAILED);
+    return E_FAIL;
+  }
+
+  data_.Initialize(base::File(duplicate_handle));
   if (!data_.IsValid()) {
     LogFontProxyError(MAPPED_FILE_FAILED);
     return E_FAIL;
   }
   return S_OK;
 }
 
 }  // namespace content