Fix for the font files being maped multiple times (Fontconfig).

content/common/font_config_ipc_linux.cc

Index: content/common/font_config_ipc_linux.cc
diff --git a/content/common/font_config_ipc_linux.cc b/content/common/font_config_ipc_linux.cc
index fcb06ebac0def482978e79011532e8b59804bebd..4631fb92cb9b79cbf00d2dbeddaa75f7d1db0e5a 100644
--- a/content/common/font_config_ipc_linux.cc
+++ b/content/common/font_config_ipc_linux.cc
@@ -14,6 +14,8 @@
 
 #include "base/debug/trace_event.h"
 #include "base/files/file_util.h"
+#include "base/files/memory_mapped_file.h"
+#include "base/memory/ref_counted.h"
 #include "base/pickle.h"
 #include "base/posix/unix_domain_socket_linux.h"
 #include "skia/ext/refptr.h"
@@ -23,14 +25,43 @@
 
 namespace content {
 
-// Return a stream from the file descriptor, or NULL on failure.
-SkStream* StreamFromFD(int fd) {
-  skia::RefPtr<SkData> data = skia::AdoptRef(SkData::NewFromFD(fd));
-  if (!data) {
-    return NULL;
+class FontConfigIPC::MappedFontFile
+    : public base::RefCountedThreadSafe<MappedFontFile> {
+ public:
+  explicit MappedFontFile(uint32_t font_id) : font_id_(font_id) {}
+
+  bool Initialize(int fd) {
+    return mapped_font_file_.Initialize(base::File(fd));
   }
-  return new SkMemoryStream(data.get());
-}
+
+  SkMemoryStream* CreateMemoryStream() {
+    DCHECK(mapped_font_file_.IsValid());
+    auto data = skia::AdoptRef(SkData::NewWithProc(
+        mapped_font_file_.data(), mapped_font_file_.length(),
+        &MappedFontFile::ReleaseProc, this));
+    if (!data)
+      return nullptr;
+    AddRef();
+    return new SkMemoryStream(data.get());
+  }
+
+  uint32_t font_id() const { return font_id_; }

[Daniel Erat, 2014/11/07 15:07:36]

move inlined method up, just after c'tor

[Krzysztof Olczyk, 2015/01/14 12:42:23]

Done

+
+ private:
+  friend class base::RefCountedThreadSafe<MappedFontFile>;
+
+  ~MappedFontFile() {

[Daniel Erat, 2014/11/07 15:07:36]

override

[Krzysztof Olczyk, 2015/01/14 12:42:23]

This destructor is not virtual.

+    auto font_config = static_cast<FontConfigIPC*>(FontConfigIPC::RefGlobal());
+    font_config->RemoveMappedFontFile(this);
+  }
+
+  static void ReleaseProc(const void* ptr, size_t length, void* context) {
+    static_cast<MappedFontFile*>(context)->Release();
+  }
+
+  uint32_t font_id_;
+  base::MemoryMappedFile mapped_font_file_;
+};
 
 void CloseFD(int fd) {
   int err = IGNORE_EINTR(close(fd));
@@ -96,6 +127,12 @@ bool FontConfigIPC::matchFamilyName(const char familyName[],
 
 SkStream* FontConfigIPC::openStream(const FontIdentity& identity) {
   TRACE_EVENT0("sandbox_ipc", "FontConfigIPC::openStream");
+
+  base::AutoLock lock(stream_opening_lock_);

[Daniel Erat, 2014/11/07 15:07:36]

i'm nervous about this lock being held across the duration of the method, and
across an IPC. if two requests come in on different threads around the same
time, is it okay if we actually load the font file twice? if so, it may be
better/faster to do:

  {
    base::AutoLock
    // check cache, return if present
  }
  // do IPC, initialize MappedFontFile
  {
    base::AutoLock
    // insert into |mapped_font_files_| and return
  }
  

[Krzysztof Olczyk, 2015/01/14 12:42:23]

Done

+  auto mapped_font_files_it = mapped_font_files_.find(identity.fID);
+  if (mapped_font_files_it != mapped_font_files_.end())
+    return mapped_font_files_it->second->CreateMemoryStream();
+
   Pickle request;
   request.WriteInt(METHOD_OPEN);
   request.WriteUInt32(identity.fID);
@@ -119,9 +156,17 @@ SkStream* FontConfigIPC::openStream(const FontIdentity& identity) {
     return NULL;
   }
 
-  SkStream* stream = StreamFromFD(result_fd);
-  CloseFD(result_fd);
-  return stream;
+  scoped_refptr<MappedFontFile> mapped_font_file =
+      new MappedFontFile(identity.fID);
+  if (!mapped_font_file->Initialize(result_fd))
+    return nullptr;
+  mapped_font_files_[mapped_font_file->font_id()] = mapped_font_file.get();
+  return mapped_font_file->CreateMemoryStream();
+}
+
+void FontConfigIPC::RemoveMappedFontFile(MappedFontFile* mapped_font_file) {
+  base::AutoLock lock(stream_opening_lock_);
+  mapped_font_files_.erase(mapped_font_file->font_id());
 }
 
 }  // namespace content