summaryrefslogtreecommitdiff
path: root/core/fxge/ge
diff options
context:
space:
mode:
authordsinclair <dsinclair@chromium.org>2016-09-19 08:37:10 -0700
committerCommit bot <commit-bot@chromium.org>2016-09-19 08:37:10 -0700
commitc48089977dc4d2a63d37e6668382c10b42e22a72 (patch)
tree931e8a2f17d997e55c9bead576f7f672196844c0 /core/fxge/ge
parentea3c3be83dae12ef682c68fc7cf906d790fd9f84 (diff)
downloadpdfium-c48089977dc4d2a63d37e6668382c10b42e22a72.tar.xz
Revert of Pdfium: Fix fonts leaking on ClosePage. (patchset #10 id:180001 of https://codereview.chromium.org/2158023002/ )
Reason for revert: Causes heap-use-after-free. See crbug.com/647612. Original issue's description: > Fix memory leaking on ClosePage. > CFX_FontCache refactoring: > after this CL: Only one global CFX_FontCache used. Any cached items from it, are released, when its are not used. > > BUG=79367,48791 > > The fonts was not cleared after unloading pages. > > Test pdf: > > http://www.nasa.gov/pdf/750614main_NASA_FY_2014_Budget_Estimates-508.pdf > > For this file, we have ~5 fonts per page, which equal ~1 Mb per page. > In this PDF we have 670 pages, as result after slow scrolling(reading) full document we have ~600 Mb fonts data in memory. > > memory usage of PDF Plugin: > before this CL: ~660 Mb > after this CL: ~100 Mb > > Committed: https://pdfium.googlesource.com/pdfium/+/cde5101eb15b24519e89fa500fe37038bc8e2201 TBR=tsepez@chromium.org,brucedawson@chromium.org,npm@chromium.org,art-snake@yandex-team.ru # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=79367,48791 Review-Url: https://codereview.chromium.org/2350763002
Diffstat (limited to 'core/fxge/ge')
-rw-r--r--core/fxge/ge/cfx_facecache.cpp12
-rw-r--r--core/fxge/ge/cfx_font.cpp64
-rw-r--r--core/fxge/ge/cfx_fontcache.cpp53
-rw-r--r--core/fxge/ge/cfx_renderdevice.cpp37
4 files changed, 72 insertions, 94 deletions
diff --git a/core/fxge/ge/cfx_facecache.cpp b/core/fxge/ge/cfx_facecache.cpp
index b78dd5b2ad..2b8405941e 100644
--- a/core/fxge/ge/cfx_facecache.cpp
+++ b/core/fxge/ge/cfx_facecache.cpp
@@ -89,7 +89,7 @@ CFX_FaceCache::~CFX_FaceCache() {
#endif
}
-CFX_GlyphBitmap* CFX_FaceCache::RenderGlyph(const CFX_Font* pFont,
+CFX_GlyphBitmap* CFX_FaceCache::RenderGlyph(CFX_Font* pFont,
uint32_t glyph_index,
FX_BOOL bFontStyle,
const CFX_Matrix* pMatrix,
@@ -223,7 +223,7 @@ CFX_GlyphBitmap* CFX_FaceCache::RenderGlyph(const CFX_Font* pFont,
return pGlyphBitmap;
}
-const CFX_PathData* CFX_FaceCache::LoadGlyphPath(const CFX_Font* pFont,
+const CFX_PathData* CFX_FaceCache::LoadGlyphPath(CFX_Font* pFont,
uint32_t glyph_index,
int dest_width) {
if (!m_Face || glyph_index == kInvalidGlyphIndex || dest_width < 0)
@@ -247,12 +247,12 @@ const CFX_PathData* CFX_FaceCache::LoadGlyphPath(const CFX_Font* pFont,
if (it != m_PathMap.end())
return it->second.get();
- CFX_PathData* pGlyphPath = pFont->LoadGlyphPathImpl(glyph_index, dest_width);
+ CFX_PathData* pGlyphPath = pFont->LoadGlyphPath(glyph_index, dest_width);
m_PathMap[key] = std::unique_ptr<CFX_PathData>(pGlyphPath);
return pGlyphPath;
}
-const CFX_GlyphBitmap* CFX_FaceCache::LoadGlyphBitmap(const CFX_Font* pFont,
+const CFX_GlyphBitmap* CFX_FaceCache::LoadGlyphBitmap(CFX_Font* pFont,
uint32_t glyph_index,
FX_BOOL bFontStyle,
const CFX_Matrix* pMatrix,
@@ -349,7 +349,7 @@ const CFX_GlyphBitmap* CFX_FaceCache::LoadGlyphBitmap(const CFX_Font* pFont,
}
#ifdef _SKIA_SUPPORT_
-CFX_TypeFace* CFX_FaceCache::GetDeviceCache(const CFX_Font* pFont) {
+CFX_TypeFace* CFX_FaceCache::GetDeviceCache(CFX_Font* pFont) {
if (!m_pTypeface) {
m_pTypeface =
SkTypeface::MakeFromStream(
@@ -365,7 +365,7 @@ void CFX_FaceCache::InitPlatform() {}
#endif
CFX_GlyphBitmap* CFX_FaceCache::LookUpGlyphBitmap(
- const CFX_Font* pFont,
+ CFX_Font* pFont,
const CFX_Matrix* pMatrix,
const CFX_ByteString& FaceGlyphsKey,
uint32_t glyph_index,
diff --git a/core/fxge/ge/cfx_font.cpp b/core/fxge/ge/cfx_font.cpp
index b184711211..feea8b0aa1 100644
--- a/core/fxge/ge/cfx_font.cpp
+++ b/core/fxge/ge/cfx_font.cpp
@@ -8,8 +8,6 @@
#include "core/fpdfapi/fpdf_font/include/cpdf_font.h"
#include "core/fxge/ge/fx_text_int.h"
-#include "core/fxge/include/cfx_facecache.h"
-#include "core/fxge/include/cfx_fontcache.h"
#include "core/fxge/include/cfx_fontmgr.h"
#include "core/fxge/include/cfx_gemodule.h"
#include "core/fxge/include/cfx_pathdata.h"
@@ -226,7 +224,6 @@ CFX_Font::CFX_Font()
m_pOwnedStream(nullptr),
#endif // PDF_ENABLE_XFA
m_Face(nullptr),
- m_FaceCache(nullptr),
m_pFontData(nullptr),
m_pGsubData(nullptr),
m_dwSize(0),
@@ -265,15 +262,8 @@ FX_BOOL CFX_Font::LoadClone(const CFX_Font* pFont) {
m_pPlatformFont = pFont->m_pPlatformFont;
#endif
m_pOwnedStream = pFont->m_pOwnedStream;
- m_FaceCache = pFont->GetFaceCache();
return TRUE;
}
-
-void CFX_Font::SetFace(FXFT_Face face) {
- ClearFaceCache();
- m_Face = face;
-}
-
#endif // PDF_ENABLE_XFA
CFX_Font::~CFX_Font() {
@@ -289,7 +279,10 @@ CFX_Font::~CFX_Font() {
FXFT_Clear_Face_External_Stream(m_Face);
}
#endif // PDF_ENABLE_XFA
- DeleteFace();
+ if (m_bEmbedded)
+ DeleteFace();
+ else
+ CFX_GEModule::Get()->GetFontMgr()->ReleaseFace(m_Face);
}
#ifdef PDF_ENABLE_XFA
delete m_pOwnedStream;
@@ -301,12 +294,7 @@ CFX_Font::~CFX_Font() {
}
void CFX_Font::DeleteFace() {
- ClearFaceCache();
- if (m_bEmbedded) {
- FXFT_Done_Face(m_Face);
- } else {
- CFX_GEModule::Get()->GetFontMgr()->ReleaseFace(m_Face);
- }
+ FXFT_Done_Face(m_Face);
m_Face = nullptr;
}
@@ -549,20 +537,6 @@ int CFX_Font::GetMaxAdvanceWidth() const {
FXFT_Get_Face_MaxAdvanceWidth(m_Face));
}
-CFX_FaceCache* CFX_Font::GetFaceCache() const {
- if (!m_FaceCache) {
- m_FaceCache = CFX_GEModule::Get()->GetFontCache()->GetCachedFace(this);
- }
- return m_FaceCache;
-}
-
-void CFX_Font::ClearFaceCache() {
- if (!m_FaceCache)
- return;
- CFX_GEModule::Get()->GetFontCache()->ReleaseCachedFace(this);
- m_FaceCache = nullptr;
-}
-
int CFX_Font::GetULPos() const {
if (!m_Face)
return 0;
@@ -579,9 +553,7 @@ int CFX_Font::GetULthickness() const {
FXFT_Get_Face_UnderLineThickness(m_Face));
}
-void CFX_Font::AdjustMMParams(int glyph_index,
- int dest_width,
- int weight) const {
+void CFX_Font::AdjustMMParams(int glyph_index, int dest_width, int weight) {
FXFT_MM_Var pMasters = nullptr;
FXFT_Get_MM_Var(m_Face, &pMasters);
if (!pMasters)
@@ -621,8 +593,7 @@ void CFX_Font::AdjustMMParams(int glyph_index,
FXFT_Set_MM_Design_Coordinates(m_Face, 2, coords);
}
-CFX_PathData* CFX_Font::LoadGlyphPathImpl(uint32_t glyph_index,
- int dest_width) const {
+CFX_PathData* CFX_Font::LoadGlyphPath(uint32_t glyph_index, int dest_width) {
if (!m_Face)
return nullptr;
FXFT_Set_Pixel_Sizes(m_Face, 0, 64);
@@ -691,24 +662,3 @@ CFX_PathData* CFX_Font::LoadGlyphPathImpl(uint32_t glyph_index,
pPath->GetPoints()[params.m_PointCount - 1].m_Flag |= FXPT_CLOSEFIGURE;
return pPath;
}
-
-const CFX_GlyphBitmap* CFX_Font::LoadGlyphBitmap(uint32_t glyph_index,
- FX_BOOL bFontStyle,
- const CFX_Matrix* pMatrix,
- int dest_width,
- int anti_alias,
- int& text_flags) const {
- return GetFaceCache()->LoadGlyphBitmap(this, glyph_index, bFontStyle, pMatrix,
- dest_width, anti_alias, text_flags);
-}
-
-const CFX_PathData* CFX_Font::LoadGlyphPath(uint32_t glyph_index,
- int dest_width) const {
- return GetFaceCache()->LoadGlyphPath(this, glyph_index, dest_width);
-}
-
-#ifdef _SKIA_SUPPORT_
-CFX_TypeFace* CFX_Font::GetDeviceCache() const {
- return GetFaceCache()->GetDeviceCache(this);
-}
-#endif
diff --git a/core/fxge/ge/cfx_fontcache.cpp b/core/fxge/ge/cfx_fontcache.cpp
index acae018c83..3ecd83c33f 100644
--- a/core/fxge/ge/cfx_fontcache.cpp
+++ b/core/fxge/ge/cfx_fontcache.cpp
@@ -10,43 +10,38 @@
#include "core/fxge/include/fx_font.h"
#include "core/fxge/include/fx_freetype.h"
-CFX_FontCache::CountedFaceCache::CountedFaceCache() {}
-
-CFX_FontCache::CountedFaceCache::~CountedFaceCache() {}
-
CFX_FontCache::CFX_FontCache() {}
CFX_FontCache::~CFX_FontCache() {
- ASSERT(m_ExtFaceMap.empty());
- ASSERT(m_FTFaceMap.empty());
+ FreeCache(TRUE);
}
-CFX_FaceCache* CFX_FontCache::GetCachedFace(const CFX_Font* pFont) {
+CFX_FaceCache* CFX_FontCache::GetCachedFace(CFX_Font* pFont) {
FXFT_Face face = pFont->GetFace();
const bool bExternal = !face;
CFX_FTCacheMap& map = bExternal ? m_ExtFaceMap : m_FTFaceMap;
auto it = map.find(face);
if (it != map.end()) {
- CountedFaceCache* counted_face_cache = it->second.get();
+ CFX_CountedFaceCache* counted_face_cache = it->second;
counted_face_cache->m_nCount++;
- return counted_face_cache->m_Obj.get();
+ return counted_face_cache->m_Obj;
}
- std::unique_ptr<CountedFaceCache> counted_face_cache(new CountedFaceCache);
- counted_face_cache->m_nCount = 2;
CFX_FaceCache* face_cache = new CFX_FaceCache(bExternal ? nullptr : face);
- counted_face_cache->m_Obj.reset(face_cache);
- map[face] = std::move(counted_face_cache);
+ CFX_CountedFaceCache* counted_face_cache = new CFX_CountedFaceCache;
+ counted_face_cache->m_nCount = 2;
+ counted_face_cache->m_Obj = face_cache;
+ map[face] = counted_face_cache;
return face_cache;
}
#ifdef _SKIA_SUPPORT_
-CFX_TypeFace* CFX_FontCache::GetDeviceCache(const CFX_Font* pFont) {
+CFX_TypeFace* CFX_FontCache::GetDeviceCache(CFX_Font* pFont) {
return GetCachedFace(pFont)->GetDeviceCache(pFont);
}
#endif
-void CFX_FontCache::ReleaseCachedFace(const CFX_Font* pFont) {
+void CFX_FontCache::ReleaseCachedFace(CFX_Font* pFont) {
FXFT_Face face = pFont->GetFace();
const bool bExternal = !face;
CFX_FTCacheMap& map = bExternal ? m_ExtFaceMap : m_FTFaceMap;
@@ -55,10 +50,30 @@ void CFX_FontCache::ReleaseCachedFace(const CFX_Font* pFont) {
if (it == map.end())
return;
- CountedFaceCache* counted_face_cache = it->second.get();
- if (counted_face_cache->m_nCount > 2) {
+ CFX_CountedFaceCache* counted_face_cache = it->second;
+ if (counted_face_cache->m_nCount > 1) {
counted_face_cache->m_nCount--;
- } else {
- map.erase(it);
+ }
+}
+
+void CFX_FontCache::FreeCache(FX_BOOL bRelease) {
+ for (auto it = m_FTFaceMap.begin(); it != m_FTFaceMap.end();) {
+ auto curr_it = it++;
+ CFX_CountedFaceCache* cache = curr_it->second;
+ if (bRelease || cache->m_nCount < 2) {
+ delete cache->m_Obj;
+ delete cache;
+ m_FTFaceMap.erase(curr_it);
+ }
+ }
+
+ for (auto it = m_ExtFaceMap.begin(); it != m_ExtFaceMap.end();) {
+ auto curr_it = it++;
+ CFX_CountedFaceCache* cache = curr_it->second;
+ if (bRelease || cache->m_nCount < 2) {
+ delete cache->m_Obj;
+ delete cache;
+ m_ExtFaceMap.erase(curr_it);
+ }
}
}
diff --git a/core/fxge/ge/cfx_renderdevice.cpp b/core/fxge/ge/cfx_renderdevice.cpp
index 11f0087f92..ad05ef743b 100644
--- a/core/fxge/ge/cfx_renderdevice.cpp
+++ b/core/fxge/ge/cfx_renderdevice.cpp
@@ -9,6 +9,7 @@
#include "core/fxcrt/include/fx_safe_types.h"
#include "core/fxge/include/cfx_autofontcache.h"
#include "core/fxge/include/cfx_facecache.h"
+#include "core/fxge/include/cfx_fontcache.h"
#include "core/fxge/include/cfx_fxgedevice.h"
#include "core/fxge/include/cfx_graphstatedata.h"
#include "core/fxge/include/cfx_pathdata.h"
@@ -823,6 +824,7 @@ bool CFX_RenderDevice::SetBitsWithMask(const CFX_DIBSource* pBitmap,
FX_BOOL CFX_RenderDevice::DrawNormalText(int nChars,
const FXTEXT_CHARPOS* pCharPos,
CFX_Font* pFont,
+ CFX_FontCache* pCache,
FX_FLOAT font_size,
const CFX_Matrix* pText2Device,
uint32_t fill_color,
@@ -831,8 +833,9 @@ FX_BOOL CFX_RenderDevice::DrawNormalText(int nChars,
if (m_DeviceClass != FXDC_DISPLAY) {
if (!(text_flags & FXTEXT_PRINTGRAPHICTEXT)) {
if (ShouldDrawDeviceText(pFont, text_flags) &&
- m_pDeviceDriver->DrawDeviceText(nChars, pCharPos, pFont, pText2Device,
- font_size, fill_color)) {
+ m_pDeviceDriver->DrawDeviceText(nChars, pCharPos, pFont, pCache,
+ pText2Device, font_size,
+ fill_color)) {
return TRUE;
}
}
@@ -840,8 +843,8 @@ FX_BOOL CFX_RenderDevice::DrawNormalText(int nChars,
return FALSE;
} else if (!(text_flags & FXTEXT_NO_NATIVETEXT)) {
if (ShouldDrawDeviceText(pFont, text_flags) &&
- m_pDeviceDriver->DrawDeviceText(nChars, pCharPos, pFont, pText2Device,
- font_size, fill_color)) {
+ m_pDeviceDriver->DrawDeviceText(nChars, pCharPos, pFont, pCache,
+ pText2Device, font_size, fill_color)) {
return TRUE;
}
}
@@ -859,8 +862,9 @@ FX_BOOL CFX_RenderDevice::DrawNormalText(int nChars,
(pFont->GetSubstFont()->m_SubstFlags & FXFONT_SUBST_GLYPHPATH)) {
int nPathFlags =
(text_flags & FXTEXT_NOSMOOTH) == 0 ? 0 : FXFILL_NOPATHSMOOTH;
- return DrawTextPath(nChars, pCharPos, pFont, font_size, pText2Device,
- nullptr, nullptr, fill_color, 0, nullptr, nPathFlags);
+ return DrawTextPath(nChars, pCharPos, pFont, pCache, font_size,
+ pText2Device, nullptr, nullptr, fill_color, 0,
+ nullptr, nPathFlags);
}
}
int anti_alias = FXFT_RENDER_MODE_MONO;
@@ -891,6 +895,10 @@ FX_BOOL CFX_RenderDevice::DrawNormalText(int nChars,
}
}
}
+ if (!pCache)
+ pCache = CFX_GEModule::Get()->GetFontCache();
+ CFX_FaceCache* pFaceCache = pCache->GetCachedFace(pFont);
+ CFX_AutoFontCache autoFontCache(pCache, pFont);
std::vector<FXTEXT_GLYPHPOS> glyphs(nChars);
CFX_Matrix matrixCTM = GetCTM();
FX_FLOAT scale_x = FXSYS_fabs(matrixCTM.a);
@@ -914,12 +922,12 @@ FX_BOOL CFX_RenderDevice::DrawNormalText(int nChars,
charpos.m_AdjustMatrix[0], charpos.m_AdjustMatrix[1],
charpos.m_AdjustMatrix[2], charpos.m_AdjustMatrix[3], 0, 0);
new_matrix.Concat(deviceCtm);
- glyph.m_pGlyph = pFont->LoadGlyphBitmap(
- charpos.m_GlyphIndex, charpos.m_bFontStyle, &new_matrix,
+ glyph.m_pGlyph = pFaceCache->LoadGlyphBitmap(
+ pFont, charpos.m_GlyphIndex, charpos.m_bFontStyle, &new_matrix,
charpos.m_FontCharWidth, anti_alias, nativetext_flags);
} else {
- glyph.m_pGlyph = pFont->LoadGlyphBitmap(
- charpos.m_GlyphIndex, charpos.m_bFontStyle, &deviceCtm,
+ glyph.m_pGlyph = pFaceCache->LoadGlyphBitmap(
+ pFont, charpos.m_GlyphIndex, charpos.m_bFontStyle, &deviceCtm,
charpos.m_FontCharWidth, anti_alias, nativetext_flags);
}
}
@@ -1040,6 +1048,7 @@ FX_BOOL CFX_RenderDevice::DrawNormalText(int nChars,
FX_BOOL CFX_RenderDevice::DrawTextPath(int nChars,
const FXTEXT_CHARPOS* pCharPos,
CFX_Font* pFont,
+ CFX_FontCache* pCache,
FX_FLOAT font_size,
const CFX_Matrix* pText2User,
const CFX_Matrix* pUser2Device,
@@ -1048,6 +1057,10 @@ FX_BOOL CFX_RenderDevice::DrawTextPath(int nChars,
FX_ARGB stroke_color,
CFX_PathData* pClippingPath,
int nFlag) {
+ if (!pCache)
+ pCache = CFX_GEModule::Get()->GetFontCache();
+ CFX_FaceCache* pFaceCache = pCache->GetCachedFace(pFont);
+ CFX_AutoFontCache autoFontCache(pCache, pFont);
for (int iChar = 0; iChar < nChars; iChar++) {
const FXTEXT_CHARPOS& charpos = pCharPos[iChar];
CFX_Matrix matrix;
@@ -1057,8 +1070,8 @@ FX_BOOL CFX_RenderDevice::DrawTextPath(int nChars,
}
matrix.Concat(font_size, 0, 0, font_size, charpos.m_OriginX,
charpos.m_OriginY);
- const CFX_PathData* pPath =
- pFont->LoadGlyphPath(charpos.m_GlyphIndex, charpos.m_FontCharWidth);
+ const CFX_PathData* pPath = pFaceCache->LoadGlyphPath(
+ pFont, charpos.m_GlyphIndex, charpos.m_FontCharWidth);
if (!pPath)
continue;
matrix.Concat(*pText2User);