Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1381)

Unified Diff: content/common/font_config_ipc_linux.cc

Issue 697383002: Fix for the font files being maped multiple times (Fontconfig). (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fixes after review. Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 c64aebf66a57fa3c432646d1906e5a15bf12b2ef..d4eaebfb7a17a78e5265f53e7974fb32dab2f3c8 100644
--- a/content/common/font_config_ipc_linux.cc
+++ b/content/common/font_config_ipc_linux.cc
@@ -14,8 +14,11 @@
#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 "base/threading/thread_restrictions.h"
#include "skia/ext/refptr.h"
#include "skia/ext/skia_utils_base.h"
#include "third_party/skia/include/core/SkData.h"
@@ -23,14 +26,45 @@
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) {}
+
+ uint32_t font_id() const { return font_id_; }
+
+ bool Initialize(int fd) {
+ base::ThreadRestrictions::ScopedAllowIO allow_mmap;
+ return mapped_font_file_.Initialize(base::File(fd));
+ }
+
+ 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());
+ }
+
+ private:
+ friend class base::RefCountedThreadSafe<MappedFontFile>;
+
+ ~MappedFontFile() {
Daniel Erat 2015/01/14 23:40:27 as i understand it, you should be using 'override'
Krzysztof Olczyk 2015/01/15 09:05:28 When it derives from base class which has virtual
Daniel Erat 2015/01/15 15:55:54 ah, thanks; i didn't realize that RefCountedThread
Krzysztof Olczyk 2015/01/15 17:44:11 It uses CRTP in order to match right destructor in
+ auto font_config = static_cast<FontConfigIPC*>(FontConfigIPC::RefGlobal());
+ font_config->RemoveMappedFontFile(this);
}
- return new SkMemoryStream(data.get());
-}
+
+ static void ReleaseProc(const void* ptr, size_t length, void* context) {
+ base::ThreadRestrictions::ScopedAllowIO allow_munmap;
+ 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 +130,14 @@ bool FontConfigIPC::matchFamilyName(const char familyName[],
SkStream* FontConfigIPC::openStream(const FontIdentity& identity) {
TRACE_EVENT0("sandbox_ipc", "FontConfigIPC::openStream");
+
+ {
+ base::AutoLock lock(stream_opening_lock_);
+ 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);
@@ -118,9 +160,23 @@ 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;
+
+ {
+ base::AutoLock lock(stream_opening_lock_);
+ auto mapped_font_files_it =
+ mapped_font_files_.insert(std::make_pair(mapped_font_file->font_id(),
+ mapped_font_file.get())).first;
+ return mapped_font_files_it->second->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
« content/common/font_config_ipc_linux.h ('K') | « content/common/font_config_ipc_linux.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698