From c48089977dc4d2a63d37e6668382c10b42e22a72 Mon Sep 17 00:00:00 2001 From: dsinclair Date: Mon, 19 Sep 2016 08:37:10 -0700 Subject: 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 --- core/fpdfapi/fpdf_page/fpdf_page_doc.cpp | 21 +++++---------- core/fpdfapi/fpdf_parser/cpdf_document.cpp | 1 + core/fpdfapi/fpdf_render/fpdf_render.cpp | 12 ++++++++- core/fpdfapi/fpdf_render/fpdf_render_text.cpp | 38 +++++++++++++++++++++------ core/fpdfapi/fpdf_render/render_int.h | 3 +++ 5 files changed, 52 insertions(+), 23 deletions(-) (limited to 'core/fpdfapi') diff --git a/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp b/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp index f83d6fa613..9e586e326f 100644 --- a/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp +++ b/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp @@ -214,10 +214,9 @@ void CPDF_DocPageData::ReleaseFont(const CPDF_Dictionary* pFontDict) { return; pFontData->RemoveRef(); - if (pFontData->use_count() > 1) + if (pFontData->use_count() != 0) return; - // We have font data only in m_FontMap cache. Clean it. pFontData->clear(); } @@ -331,10 +330,9 @@ void CPDF_DocPageData::ReleaseColorSpace(const CPDF_Object* pColorSpace) { return; pCountedColorSpace->RemoveRef(); - if (pCountedColorSpace->use_count() > 1) + if (pCountedColorSpace->use_count() != 0) return; - // We have item only in m_ColorSpaceMap cache. Clean it. pCountedColorSpace->get()->ReleaseCS(); pCountedColorSpace->reset(nullptr); } @@ -393,10 +391,9 @@ void CPDF_DocPageData::ReleasePattern(const CPDF_Object* pPatternObj) { return; pPattern->RemoveRef(); - if (pPattern->use_count() > 1) + if (pPattern->use_count() != 0) return; - // We have item only in m_PatternMap cache. Clean it. pPattern->clear(); } @@ -432,10 +429,9 @@ void CPDF_DocPageData::ReleaseImage(const CPDF_Object* pImageStream) { return; pCountedImage->RemoveRef(); - if (pCountedImage->use_count() > 1) + if (pCountedImage->use_count() != 0) return; - // We have item only in m_ImageMap cache. Clean it. delete pCountedImage->get(); delete pCountedImage; m_ImageMap.erase(it); @@ -458,8 +454,7 @@ CPDF_IccProfile* CPDF_DocPageData::GetIccProfile( auto hash_it = m_HashProfileMap.find(bsDigest); if (hash_it != m_HashProfileMap.end()) { auto it_copied_stream = m_IccProfileMap.find(hash_it->second); - if (it_copied_stream != m_IccProfileMap.end()) - return it_copied_stream->second->AddRef(); + return it_copied_stream->second->AddRef(); } CPDF_IccProfile* pProfile = new CPDF_IccProfile(stream.GetData(), stream.GetSize()); @@ -478,8 +473,7 @@ void CPDF_DocPageData::ReleaseIccProfile(const CPDF_IccProfile* pIccProfile) { continue; profile->RemoveRef(); - if (profile->use_count() == 1) { - // We have item only in m_IccProfileMap cache. Clean it. + if (profile->use_count() == 0) { delete profile->get(); delete profile; m_IccProfileMap.erase(it); @@ -524,10 +518,9 @@ void CPDF_DocPageData::ReleaseFontFileStreamAcc( return; pCountedStream->RemoveRef(); - if (pCountedStream->use_count() > 1) + if (pCountedStream->use_count() != 0) return; - // We have item only in m_FontFileMap cache. Clean it. delete pCountedStream->get(); delete pCountedStream; m_FontFileMap.erase(it); diff --git a/core/fpdfapi/fpdf_parser/cpdf_document.cpp b/core/fpdfapi/fpdf_parser/cpdf_document.cpp index d3909ceaec..b2a3cd4ef2 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_document.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_document.cpp @@ -21,6 +21,7 @@ #include "core/fpdfapi/fpdf_render/render_int.h" #include "core/fpdfapi/include/cpdf_modulemgr.h" #include "core/fxcodec/include/JBig2_DocumentContext.h" +#include "core/fxge/include/cfx_fontcache.h" #include "core/fxge/include/cfx_unicodeencoding.h" #include "core/fxge/include/fx_font.h" #include "third_party/base/stl_util.h" diff --git a/core/fpdfapi/fpdf_render/fpdf_render.cpp b/core/fpdfapi/fpdf_render/fpdf_render.cpp index 8043f932bb..59c8397f54 100644 --- a/core/fpdfapi/fpdf_render/fpdf_render.cpp +++ b/core/fpdfapi/fpdf_render/fpdf_render.cpp @@ -30,13 +30,14 @@ #include "core/fpdfapi/fpdf_render/include/cpdf_textrenderer.h" #include "core/fpdfapi/include/cpdf_modulemgr.h" #include "core/fpdfdoc/include/cpdf_occontext.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" #include "core/fxge/include/cfx_renderdevice.h" CPDF_DocRenderData::CPDF_DocRenderData(CPDF_Document* pPDFDoc) - : m_pPDFDoc(pPDFDoc) {} + : m_pPDFDoc(pPDFDoc), m_pFontCache(new CFX_FontCache) {} CPDF_DocRenderData::~CPDF_DocRenderData() { Clear(TRUE); @@ -62,6 +63,15 @@ void CPDF_DocRenderData::Clear(FX_BOOL bRelease) { m_TransferFuncMap.erase(curr_it); } } + + if (m_pFontCache) { + if (bRelease) { + delete m_pFontCache; + m_pFontCache = nullptr; + } else { + m_pFontCache->FreeCache(FALSE); + } + } } CPDF_Type3Cache* CPDF_DocRenderData::GetCachedType3(CPDF_Type3Font* pFont) { diff --git a/core/fpdfapi/fpdf_render/fpdf_render_text.cpp b/core/fpdfapi/fpdf_render/fpdf_render_text.cpp index 991b57ab7f..265948f1d5 100644 --- a/core/fpdfapi/fpdf_render/fpdf_render_text.cpp +++ b/core/fpdfapi/fpdf_render/fpdf_render_text.cpp @@ -25,6 +25,7 @@ #include "core/fpdfapi/fpdf_render/include/cpdf_textrenderer.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_gemodule.h" #include "core/fxge/include/cfx_graphstatedata.h" @@ -434,6 +435,9 @@ FX_BOOL CPDF_TextRenderer::DrawTextPath(CFX_RenderDevice* pDevice, FX_ARGB stroke_argb, CFX_PathData* pClippingPath, int nFlag) { + CFX_FontCache* pCache = + pFont->m_pDocument ? pFont->m_pDocument->GetRenderData()->GetFontCache() + : nullptr; CPDF_CharPosList CharPosList; CharPosList.Load(nChars, pCharCodes, pCharPos, pFont, font_size); if (CharPosList.m_nChars == 0) @@ -448,10 +452,10 @@ FX_BOOL CPDF_TextRenderer::DrawTextPath(CFX_RenderDevice* pDevice, auto* font = fontPosition == -1 ? &pFont->m_Font : pFont->m_FontFallbacks[fontPosition].get(); - if (!pDevice->DrawTextPath(i - startIndex, - CharPosList.m_pCharPos + startIndex, font, - font_size, pText2User, pUser2Device, pGraphState, - fill_argb, stroke_argb, pClippingPath, nFlag)) { + if (!pDevice->DrawTextPath( + i - startIndex, CharPosList.m_pCharPos + startIndex, font, pCache, + font_size, pText2User, pUser2Device, pGraphState, fill_argb, + stroke_argb, pClippingPath, nFlag)) { bDraw = false; } fontPosition = curFontPosition; @@ -460,7 +464,7 @@ FX_BOOL CPDF_TextRenderer::DrawTextPath(CFX_RenderDevice* pDevice, auto* font = fontPosition == -1 ? &pFont->m_Font : pFont->m_FontFallbacks[fontPosition].get(); if (!pDevice->DrawTextPath(CharPosList.m_nChars - startIndex, - CharPosList.m_pCharPos + startIndex, font, + CharPosList.m_pCharPos + startIndex, font, pCache, font_size, pText2User, pUser2Device, pGraphState, fill_argb, stroke_argb, pClippingPath, nFlag)) { bDraw = false; @@ -536,6 +540,9 @@ FX_BOOL CPDF_TextRenderer::DrawNormalText(CFX_RenderDevice* pDevice, const CFX_Matrix* pText2Device, FX_ARGB fill_argb, const CPDF_RenderOptions* pOptions) { + CFX_FontCache* pCache = + pFont->m_pDocument ? pFont->m_pDocument->GetRenderData()->GetFontCache() + : nullptr; CPDF_CharPosList CharPosList; CharPosList.Load(nChars, pCharCodes, pCharPos, pFont, font_size); if (CharPosList.m_nChars == 0) @@ -578,7 +585,7 @@ FX_BOOL CPDF_TextRenderer::DrawNormalText(CFX_RenderDevice* pDevice, ? &pFont->m_Font : pFont->m_FontFallbacks[fontPosition].get(); if (!pDevice->DrawNormalText( - i - startIndex, CharPosList.m_pCharPos + startIndex, font, + i - startIndex, CharPosList.m_pCharPos + startIndex, font, pCache, font_size, pText2Device, fill_argb, FXGE_flags)) { bDraw = false; } @@ -589,7 +596,7 @@ FX_BOOL CPDF_TextRenderer::DrawNormalText(CFX_RenderDevice* pDevice, : pFont->m_FontFallbacks[fontPosition].get(); if (!pDevice->DrawNormalText(CharPosList.m_nChars - startIndex, CharPosList.m_pCharPos + startIndex, font, - font_size, pText2Device, fill_argb, + pCache, font_size, pText2Device, fill_argb, FXGE_flags)) { bDraw = false; } @@ -620,9 +627,23 @@ void CPDF_RenderStatus::DrawTextPathWithPattern(const CPDF_TextObject* textobj, RenderSingleObject(&path, pObj2Device); return; } + CFX_FontCache* pCache; + if (pFont->m_pDocument) { + pCache = pFont->m_pDocument->GetRenderData()->GetFontCache(); + } else { + pCache = CFX_GEModule::Get()->GetFontCache(); + } CPDF_CharPosList CharPosList; CharPosList.Load(textobj->m_nChars, textobj->m_pCharCodes, textobj->m_pCharPos, pFont, font_size); + std::vector faceCaches; + std::vector autoFontCaches; + faceCaches.push_back(pCache->GetCachedFace(&pFont->m_Font)); + autoFontCaches.push_back(CFX_AutoFontCache(pCache, &pFont->m_Font)); + for (const auto& font : pFont->m_FontFallbacks) { + faceCaches.push_back(pCache->GetCachedFace(font.get())); + autoFontCaches.push_back(CFX_AutoFontCache(pCache, font.get())); + } for (uint32_t i = 0; i < CharPosList.m_nChars; i++) { FXTEXT_CHARPOS& charpos = CharPosList.m_pCharPos[i]; auto font = @@ -630,7 +651,8 @@ void CPDF_RenderStatus::DrawTextPathWithPattern(const CPDF_TextObject* textobj, ? &pFont->m_Font : pFont->m_FontFallbacks[charpos.m_FallbackFontPosition].get(); const CFX_PathData* pPath = - font->LoadGlyphPath(charpos.m_GlyphIndex, charpos.m_FontCharWidth); + faceCaches[charpos.m_FallbackFontPosition + 1]->LoadGlyphPath( + font, charpos.m_GlyphIndex, charpos.m_FontCharWidth); if (!pPath) { continue; } diff --git a/core/fpdfapi/fpdf_render/render_int.h b/core/fpdfapi/fpdf_render/render_int.h index afd9c83b44..672e5923df 100644 --- a/core/fpdfapi/fpdf_render/render_int.h +++ b/core/fpdfapi/fpdf_render/render_int.h @@ -21,6 +21,7 @@ class CCodec_Jbig2Context; class CCodec_ScanlineDecoder; +class CFX_FontCache; class CFX_GlyphBitmap; class CFX_ImageTransformer; class CFX_PathData; @@ -70,6 +71,7 @@ class CPDF_DocRenderData { ~CPDF_DocRenderData(); CPDF_Type3Cache* GetCachedType3(CPDF_Type3Font* pFont); CPDF_TransferFunc* GetTransferFunc(CPDF_Object* pObj); + CFX_FontCache* GetFontCache() { return m_pFontCache; } void Clear(FX_BOOL bRelease = FALSE); void ReleaseCachedType3(CPDF_Type3Font* pFont); void ReleaseTransferFunc(CPDF_Object* pObj); @@ -81,6 +83,7 @@ class CPDF_DocRenderData { std::map*>; CPDF_Document* m_pPDFDoc; + CFX_FontCache* m_pFontCache; CPDF_Type3CacheMap m_Type3FaceMap; CPDF_TransferFuncMap m_TransferFuncMap; }; -- cgit v1.2.3