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

content/common/font_config_ipc_linux.cc

 // Copyright (c) 2012 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/common/font_config_ipc_linux.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <sys/mman.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/uio.h>
 #include <unistd.h>
 
 #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"
 #include "skia/ext/skia_utils_base.h"
 #include "third_party/skia/include/core/SkData.h"
 #include "third_party/skia/include/core/SkStream.h"
 
 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));
   DCHECK(!err);
 }
 
 FontConfigIPC::FontConfigIPC(int fd)
     : fd_(fd) {
 }
 
@@ skipping 45 matching lines @@
   if (outFamilyName)
     *outFamilyName = reply_family;
   if (outStyle)
     *outStyle = static_cast<SkTypeface::Style>(reply_style);
 
   return true;
 }
 
 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);
 
   int result_fd = -1;
   uint8_t reply_buf[256];
   const ssize_t r = UnixDomainSocket::SendRecvMsg(fd_, reply_buf,
                                                   sizeof(reply_buf),
                                                   &result_fd, request);
 
   if (r == -1)
     return NULL;
 
   Pickle reply(reinterpret_cast<char*>(reply_buf), r);
   bool result;
   PickleIterator iter(reply);
   if (!reply.ReadBool(&iter, &result) ||
       !result) {
     if (result_fd)
       CloseFD(result_fd);
     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