From 9a0736803ae6749ec508e1c3ff5f04a483bbcb56 Mon Sep 17 00:00:00 2001 From: weili Date: Thu, 8 Sep 2016 15:21:23 -0700 Subject: Reland of Fix leaked internal font Reland of Fix leaked internal font (patchset #2 id:60001 of https://codereview.chromium.org/2297303004/ ) In CFGAS_FontMgrImp::LoadFont(), a new internal font is created which is never released. It needs to be correctly marked as internal font to be released. Fix this by adding a new method to take the ownership of the font and mark it as internal font properly. The previous revert was caused by memory management errors which were fixed at https://codereview.chromium.org/2322043002/ BUG=pdfium:242 Review-Url: https://codereview.chromium.org/2320213002 --- xfa/fgas/font/fgas_gefont.cpp | 41 +++++++++++++++++++++++++++++---------- xfa/fgas/font/fgas_gefont.h | 10 +++++++--- xfa/fgas/font/fgas_stdfontmgr.cpp | 4 ++-- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/xfa/fgas/font/fgas_gefont.cpp b/xfa/fgas/font/fgas_gefont.cpp index d3be9dcb48..e386a0f208 100644 --- a/xfa/fgas/font/fgas_gefont.cpp +++ b/xfa/fgas/font/fgas_gefont.cpp @@ -34,10 +34,21 @@ CFGAS_GEFont* CFGAS_GEFont::LoadFont(const FX_WCHAR* pszFontFamily, } // static -CFGAS_GEFont* CFGAS_GEFont::LoadFont(CFX_Font* pExtFont, +CFGAS_GEFont* CFGAS_GEFont::LoadFont(CFX_Font* pExternalFont, IFGAS_FontMgr* pFontMgr) { CFGAS_GEFont* pFont = new CFGAS_GEFont(pFontMgr); - if (!pFont->LoadFontInternal(pExtFont)) { + if (!pFont->LoadFontInternal(pExternalFont)) { + pFont->Release(); + return nullptr; + } + return pFont; +} + +// static +CFGAS_GEFont* CFGAS_GEFont::LoadFont(std::unique_ptr pInternalFont, + IFGAS_FontMgr* pFontMgr) { + CFGAS_GEFont* pFont = new CFGAS_GEFont(pFontMgr); + if (!pFont->LoadFontInternal(std::move(pInternalFont))) { pFont->Release(); return nullptr; } @@ -80,7 +91,7 @@ CFGAS_GEFont::CFGAS_GEFont(IFGAS_FontMgr* pFontMgr) m_pSrcFont(nullptr), m_pFontMgr(pFontMgr), m_iRefCount(1), - m_bExtFont(FALSE), + m_bExternalFont(false), m_pProvider(nullptr) { } @@ -94,7 +105,7 @@ CFGAS_GEFont::CFGAS_GEFont(CFGAS_GEFont* src, uint32_t dwFontStyles) m_pSrcFont(src), m_pFontMgr(src->m_pFontMgr), m_iRefCount(1), - m_bExtFont(FALSE), + m_bExternalFont(false), m_pProvider(nullptr) { ASSERT(m_pSrcFont->m_pFont); m_pSrcFont->Retain(); @@ -120,7 +131,7 @@ CFGAS_GEFont::~CFGAS_GEFont() { m_SubstFonts.RemoveAll(); m_FontMapper.clear(); - if (!m_bExtFont) + if (!m_bExternalFont) delete m_pFont; // If it is a shallow copy of another source font, @@ -217,12 +228,22 @@ FX_BOOL CFGAS_GEFont::LoadFontInternal(IFX_Stream* pFontStream, } #endif // _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ -FX_BOOL CFGAS_GEFont::LoadFontInternal(CFX_Font* pExtFont) { - if (m_pFont || !pExtFont) { +FX_BOOL CFGAS_GEFont::LoadFontInternal(CFX_Font* pExternalFont) { + if (m_pFont || !pExternalFont) return FALSE; - } - m_pFont = pExtFont; - m_bExtFont = TRUE; + + m_pFont = pExternalFont; + m_bExternalFont = true; + return InitFont(); +} + +FX_BOOL CFGAS_GEFont::LoadFontInternal( + std::unique_ptr pInternalFont) { + if (m_pFont || !pInternalFont) + return FALSE; + + m_pFont = pInternalFont.release(); + m_bExternalFont = false; return InitFont(); } diff --git a/xfa/fgas/font/fgas_gefont.h b/xfa/fgas/font/fgas_gefont.h index d139d445f4..824f931d76 100644 --- a/xfa/fgas/font/fgas_gefont.h +++ b/xfa/fgas/font/fgas_gefont.h @@ -24,7 +24,10 @@ class CFGAS_GEFont { uint32_t dwFontStyles, uint16_t wCodePage, IFGAS_FontMgr* pFontMgr); - static CFGAS_GEFont* LoadFont(CFX_Font* pExtFont, IFGAS_FontMgr* pFontMgr); + static CFGAS_GEFont* LoadFont(CFX_Font* pExternalFont, + IFGAS_FontMgr* pFontMgr); + static CFGAS_GEFont* LoadFont(std::unique_ptr pInternalFont, + IFGAS_FontMgr* pFontMgr); #if _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ static CFGAS_GEFont* LoadFont(const uint8_t* pBuffer, int32_t iLength, @@ -74,7 +77,8 @@ class CFGAS_GEFont { FX_BOOL LoadFontInternal(const uint8_t* pBuffer, int32_t length); FX_BOOL LoadFontInternal(IFX_Stream* pFontStream, FX_BOOL bSaveStream); #endif - FX_BOOL LoadFontInternal(CFX_Font* pExtFont); + FX_BOOL LoadFontInternal(CFX_Font* pExternalFont); + FX_BOOL LoadFontInternal(std::unique_ptr pInternalFont); FX_BOOL InitFont(); FX_BOOL GetCharBBoxInternal(FX_WCHAR wUnicode, CFX_Rect& bbox, @@ -97,7 +101,7 @@ class CFGAS_GEFont { CFGAS_GEFont* const m_pSrcFont; IFGAS_FontMgr* const m_pFontMgr; int32_t m_iRefCount; - FX_BOOL m_bExtFont; + bool m_bExternalFont; std::unique_ptr> m_pStream; std::unique_ptr> m_pFileRead; std::unique_ptr m_pFontEncoding; diff --git a/xfa/fgas/font/fgas_stdfontmgr.cpp b/xfa/fgas/font/fgas_stdfontmgr.cpp index a0e7e39a40..96081f5799 100644 --- a/xfa/fgas/font/fgas_stdfontmgr.cpp +++ b/xfa/fgas/font/fgas_stdfontmgr.cpp @@ -847,13 +847,13 @@ CFGAS_GEFont* CFGAS_FontMgrImp::LoadFont(const CFX_WideString& wsFaceName, if (!pFontStream) return nullptr; - CFX_Font* pInternalFont = new CFX_Font(); + std::unique_ptr pInternalFont(new CFX_Font()); if (!pInternalFont->LoadFile(pFontStream, iFaceIndex)) { pFontStream->Release(); return nullptr; } - CFGAS_GEFont* pFont = CFGAS_GEFont::LoadFont(pInternalFont, this); + CFGAS_GEFont* pFont = CFGAS_GEFont::LoadFont(std::move(pInternalFont), this); if (!pFont) { pFontStream->Release(); return nullptr; -- cgit v1.2.3