commit 9a0736803ae6749ec508e1c3ff5f04a483bbcb56
author weili <weili@chromium.org> Thu Sep 08 15:21:23 2016 -0700
committer Commit bot <commit-bot@chromium.org> Thu Sep 08 15:21:23 2016 -0700
tree 7ab220c65348600539d3ec4abf8c0f6e8c1a1dac
parent 0ca160fbf31a7408df282823f7a3d4cbb5f386a1

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

diff --git a/xfa/fgas/font/fgas_gefont.cpp b/xfa/fgas/font/fgas_gefont.cpp
index d3be9dc..e386a0f 100644
--- a/xfa/fgas/font/fgas_gefont.cpp
+++ b/xfa/fgas/font/fgas_gefont.cpp
@@ -34,10 +34,21 @@
 }
 
 // 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<CFX_Font> 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 @@
       m_pSrcFont(nullptr),
       m_pFontMgr(pFontMgr),
       m_iRefCount(1),
-      m_bExtFont(FALSE),
+      m_bExternalFont(false),
       m_pProvider(nullptr) {
 }
 
@@ -94,7 +105,7 @@
       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 @@
   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 @@
 }
 #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<CFX_Font> pInternalFont) {
+  if (m_pFont || !pInternalFont)
+    return FALSE;
+
+  m_pFont = pInternalFont.release();
+  m_bExternalFont = false;
   return InitFont();
 }
 

xfa/fgas/font/fgas_gefont.h

diff --git a/xfa/fgas/font/fgas_gefont.h b/xfa/fgas/font/fgas_gefont.h
index d139d44..824f931 100644
--- a/xfa/fgas/font/fgas_gefont.h
+++ b/xfa/fgas/font/fgas_gefont.h
@@ -24,7 +24,10 @@
                                 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<CFX_Font> 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 @@
   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<CFX_Font> pInternalFont);
   FX_BOOL InitFont();
   FX_BOOL GetCharBBoxInternal(FX_WCHAR wUnicode,
                               CFX_Rect& bbox,
@@ -97,7 +101,7 @@
   CFGAS_GEFont* const m_pSrcFont;
   IFGAS_FontMgr* const m_pFontMgr;
   int32_t m_iRefCount;
-  FX_BOOL m_bExtFont;
+  bool m_bExternalFont;
   std::unique_ptr<IFX_Stream, ReleaseDeleter<IFX_Stream>> m_pStream;
   std::unique_ptr<IFX_FileRead, ReleaseDeleter<IFX_FileRead>> m_pFileRead;
   std::unique_ptr<CFX_UnicodeEncoding> m_pFontEncoding;

xfa/fgas/font/fgas_stdfontmgr.cpp

diff --git a/xfa/fgas/font/fgas_stdfontmgr.cpp b/xfa/fgas/font/fgas_stdfontmgr.cpp
index a0e7e39..96081f5 100644
--- a/xfa/fgas/font/fgas_stdfontmgr.cpp
+++ b/xfa/fgas/font/fgas_stdfontmgr.cpp
@@ -847,13 +847,13 @@
   if (!pFontStream)
     return nullptr;
 
-  CFX_Font* pInternalFont = new CFX_Font();
+  std::unique_ptr<CFX_Font> 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;