From c29fc707b24b9528e41a242cfa298275708ffc76 Mon Sep 17 00:00:00 2001
From: weili <weili@chromium.org>
Date: Thu, 8 Sep 2016 11:47:29 -0700
Subject: Fix memory management errors for font loading and copying

A few issues are fixed:
--Change variable |m_bLogic| in CFX_Font to |m_bShallowCopy| to
reflect its meaning better;
--For a shallow copy of font, we must guarantee that the copied font
will not be deleted until the shallow copy is deleted. So need to
increase the src font's refcount when copying it;
--The stream |m_pOwnedStream| needs to have matched new/delete

These errors need to be fixed before we can properly delete all the
fonts to address the leaks.

BUG=pdfium:242

Review-Url: https://codereview.chromium.org/2322043002
---
 core/fxge/ge/cfx_font.cpp     |  8 ++++----
 core/fxge/include/fx_font.h   |  4 ++--
 xfa/fgas/font/fgas_gefont.cpp | 18 +++++++++++++-----
 xfa/fgas/font/fgas_gefont.h   |  3 ++-
 4 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/core/fxge/ge/cfx_font.cpp b/core/fxge/ge/cfx_font.cpp
index 4aefec8d43..3d2e6d6499 100644
--- a/core/fxge/ge/cfx_font.cpp
+++ b/core/fxge/ge/cfx_font.cpp
@@ -220,7 +220,7 @@ const uint8_t CFX_Font::s_WeightPow_SHIFTJIS[] = {
 CFX_Font::CFX_Font()
     :
 #ifdef PDF_ENABLE_XFA
-      m_bLogic(FALSE),
+      m_bShallowCopy(false),
       m_pOwnedStream(nullptr),
 #endif  // PDF_ENABLE_XFA
       m_Face(nullptr),
@@ -239,7 +239,7 @@ FX_BOOL CFX_Font::LoadClone(const CFX_Font* pFont) {
   if (!pFont)
     return FALSE;
 
-  m_bLogic = TRUE;
+  m_bShallowCopy = true;
   if (pFont->m_pSubstFont) {
     m_pSubstFont.reset(new CFX_SubstFont);
     m_pSubstFont->m_Charset = pFont->m_pSubstFont->m_Charset;
@@ -268,7 +268,7 @@ FX_BOOL CFX_Font::LoadClone(const CFX_Font* pFont) {
 
 CFX_Font::~CFX_Font() {
 #ifdef PDF_ENABLE_XFA
-  if (m_bLogic) {
+  if (m_bShallowCopy) {
     m_OtfFontData.DetachBuffer();
     return;
   }
@@ -285,7 +285,7 @@ CFX_Font::~CFX_Font() {
       CFX_GEModule::Get()->GetFontMgr()->ReleaseFace(m_Face);
   }
 #ifdef PDF_ENABLE_XFA
-  FX_Free(m_pOwnedStream);
+  delete m_pOwnedStream;
 #endif  // PDF_ENABLE_XFA
   FX_Free(m_pGsubData);
 #if _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_ && !defined _SKIA_SUPPORT_
diff --git a/core/fxge/include/fx_font.h b/core/fxge/include/fx_font.h
index cdde643770..a607f3e0d5 100644
--- a/core/fxge/include/fx_font.h
+++ b/core/fxge/include/fx_font.h
@@ -151,8 +151,8 @@ class CFX_Font {
 #ifdef PDF_ENABLE_XFA
  protected:
   CFX_BinaryBuf m_OtfFontData;
-  FX_BOOL m_bLogic;
-  void* m_pOwnedStream;
+  bool m_bShallowCopy;
+  FXFT_StreamRec* m_pOwnedStream;
 #endif  // PDF_ENABLE_XFA
 
  private:
diff --git a/xfa/fgas/font/fgas_gefont.cpp b/xfa/fgas/font/fgas_gefont.cpp
index 1507fa4840..d3be9dcb48 100644
--- a/xfa/fgas/font/fgas_gefont.cpp
+++ b/xfa/fgas/font/fgas_gefont.cpp
@@ -77,26 +77,29 @@ CFGAS_GEFont::CFGAS_GEFont(IFGAS_FontMgr* pFontMgr)
       m_dwLogFontStyle(0),
 #endif
       m_pFont(nullptr),
+      m_pSrcFont(nullptr),
       m_pFontMgr(pFontMgr),
       m_iRefCount(1),
       m_bExtFont(FALSE),
       m_pProvider(nullptr) {
 }
 
-CFGAS_GEFont::CFGAS_GEFont(const CFGAS_GEFont& src, uint32_t dwFontStyles)
+CFGAS_GEFont::CFGAS_GEFont(CFGAS_GEFont* src, uint32_t dwFontStyles)
     :
 #if _FXM_PLATFORM_ != _FXM_PLATFORM_WINDOWS_
       m_bUseLogFontStyle(FALSE),
       m_dwLogFontStyle(0),
 #endif
       m_pFont(nullptr),
-      m_pFontMgr(src.m_pFontMgr),
+      m_pSrcFont(src),
+      m_pFontMgr(src->m_pFontMgr),
       m_iRefCount(1),
       m_bExtFont(FALSE),
       m_pProvider(nullptr) {
-  ASSERT(src.m_pFont);
+  ASSERT(m_pSrcFont->m_pFont);
+  m_pSrcFont->Retain();
   m_pFont = new CFX_Font;
-  m_pFont->LoadClone(src.m_pFont);
+  m_pFont->LoadClone(m_pSrcFont->m_pFont);
   CFX_SubstFont* pSubst = m_pFont->GetSubstFont();
   if (!pSubst) {
     pSubst = new CFX_SubstFont;
@@ -119,6 +122,11 @@ CFGAS_GEFont::~CFGAS_GEFont() {
 
   if (!m_bExtFont)
     delete m_pFont;
+
+  // If it is a shallow copy of another source font,
+  // decrease the refcount of the source font.
+  if (m_pSrcFont)
+    m_pSrcFont->Release();
 }
 
 void CFGAS_GEFont::Release() {
@@ -239,7 +247,7 @@ FX_BOOL CFGAS_GEFont::InitFont() {
 CFGAS_GEFont* CFGAS_GEFont::Derive(uint32_t dwFontStyles, uint16_t wCodePage) {
   if (GetFontStyles() == dwFontStyles)
     return Retain();
-  return new CFGAS_GEFont(*this, dwFontStyles);
+  return new CFGAS_GEFont(this, dwFontStyles);
 }
 
 void CFGAS_GEFont::GetFamilyName(CFX_WideString& wsFamily) const {
diff --git a/xfa/fgas/font/fgas_gefont.h b/xfa/fgas/font/fgas_gefont.h
index cdb19338a2..d139d445f4 100644
--- a/xfa/fgas/font/fgas_gefont.h
+++ b/xfa/fgas/font/fgas_gefont.h
@@ -65,7 +65,7 @@ class CFGAS_GEFont {
 
  protected:
   explicit CFGAS_GEFont(IFGAS_FontMgr* pFontMgr);
-  CFGAS_GEFont(const CFGAS_GEFont& src, uint32_t dwFontStyles);
+  CFGAS_GEFont(CFGAS_GEFont* src, uint32_t dwFontStyles);
 
 #if _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_
   FX_BOOL LoadFontInternal(const FX_WCHAR* pszFontFamily,
@@ -94,6 +94,7 @@ class CFGAS_GEFont {
   uint32_t m_dwLogFontStyle;
 #endif
   CFX_Font* m_pFont;
+  CFGAS_GEFont* const m_pSrcFont;
   IFGAS_FontMgr* const m_pFontMgr;
   int32_t m_iRefCount;
   FX_BOOL m_bExtFont;
-- 
cgit v1.2.3