Saner memory managment in cttfontdesc, part 1.

A subsequent patch will tackle the ad-hoc ref counting, but we
can tidy this before going down that hole.

Decouple CTTFontDesc creation from face setting.
Remove union and treat single-entry case as vector's first element.
Pass unique_ptr to prove memory ownership.

Change-Id: Ic427798da04f3afbb65a56ee10045b9f22457a73
Reviewed-on: https://pdfium-review.googlesource.com/31730
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

5 files changed, 78 insertions, 95 deletions

diff --git a/core/fxge/cfx_fontmapper.cpp b/core/fxge/cfx_fontmapper.cpp
index b3e88c00fe..c0b111a51e 100644
--- a/core/fxge/cfx_fontmapper.cpp
+++ b/core/fxge/cfx_fontmapper.cpp
@@ -751,22 +751,25 @@ FXFT_Face CFX_FontMapper::GetCachedTTCFace(void* hFont,
                                            const uint32_t tableTTCF,
                                            uint32_t ttc_size,
                                            uint32_t font_size) {
-  uint8_t buffer[1024];
-  m_pFontInfo->GetFontData(hFont, tableTTCF, buffer, FX_ArraySize(buffer));
-  uint32_t* pBuffer = reinterpret_cast<uint32_t*>(buffer);
   uint32_t checksum = 0;
-  for (int i = 0; i < 256; i++)
-    checksum += pBuffer[i];
-  uint8_t* pFontData;
-  FXFT_Face face = m_pFontMgr->GetCachedTTCFace(
-      ttc_size, checksum, ttc_size - font_size, &pFontData);
-  if (!face) {
-    pFontData = FX_Alloc(uint8_t, ttc_size);
-    m_pFontInfo->GetFontData(hFont, tableTTCF, pFontData, ttc_size);
-    face = m_pFontMgr->AddCachedTTCFace(ttc_size, checksum, pFontData, ttc_size,
-                                        ttc_size - font_size);
+  {
+    uint8_t buffer[1024];
+    m_pFontInfo->GetFontData(hFont, tableTTCF, buffer, sizeof(buffer));
+    uint32_t* pBuffer = reinterpret_cast<uint32_t*>(buffer);
+    for (int i = 0; i < 256; i++)
+      checksum += pBuffer[i];
   }
-  return face;
+  uint8_t* pIgnore = nullptr;
+  FXFT_Face face = m_pFontMgr->GetCachedTTCFace(ttc_size, checksum,
+                                                ttc_size - font_size, &pIgnore);
+  if (face)
+    return face;
+
+  std::unique_ptr<uint8_t, FxFreeDeleter> pFontData(
+      FX_Alloc(uint8_t, ttc_size));
+  m_pFontInfo->GetFontData(hFont, tableTTCF, pFontData.get(), ttc_size);
+  return m_pFontMgr->AddCachedTTCFace(ttc_size, checksum, std::move(pFontData),
+                                      ttc_size, ttc_size - font_size);
 }
 
 FXFT_Face CFX_FontMapper::GetCachedFace(void* hFont,
@@ -774,17 +777,18 @@ FXFT_Face CFX_FontMapper::GetCachedFace(void* hFont,
                                         int weight,
                                         bool bItalic,
                                         uint32_t font_size) {
-  uint8_t* pFontData;
+  uint8_t* pIgnore = nullptr;
   FXFT_Face face =
-      m_pFontMgr->GetCachedFace(SubstName, weight, bItalic, &pFontData);
-  if (!face) {
-    pFontData = FX_Alloc(uint8_t, font_size);
-    m_pFontInfo->GetFontData(hFont, 0, pFontData, font_size);
-    face =
-        m_pFontMgr->AddCachedFace(SubstName, weight, bItalic, pFontData,
-                                  font_size, m_pFontInfo->GetFaceIndex(hFont));
-  }
-  return face;
+      m_pFontMgr->GetCachedFace(SubstName, weight, bItalic, &pIgnore);
+  if (face)
+    return face;
+
+  std::unique_ptr<uint8_t, FxFreeDeleter> pFontData(
+      FX_Alloc(uint8_t, font_size));
+  m_pFontInfo->GetFontData(hFont, 0, pFontData.get(), font_size);
+  return m_pFontMgr->AddCachedFace(SubstName, weight, bItalic,
+                                   std::move(pFontData), font_size,
+                                   m_pFontInfo->GetFaceIndex(hFont));
 }
 
 int PDF_GetStandardFontName(ByteString* name) {
diff --git a/core/fxge/cfx_fontmgr.cpp b/core/fxge/cfx_fontmgr.cpp
index 158b5d0cdb..2ee46373d5 100644
--- a/core/fxge/cfx_fontmgr.cpp
+++ b/core/fxge/cfx_fontmgr.cpp
@@ -126,19 +126,21 @@ FXFT_Face CFX_FontMgr::GetCachedFace(const ByteString& face_name,
   CTTFontDesc* pFontDesc = it->second.get();
   *pFontData = pFontDesc->FontData();
   pFontDesc->AddRef();
-  return pFontDesc->SingleFace();
+  return pFontDesc->GetFace(0);
 }
 
-FXFT_Face CFX_FontMgr::AddCachedFace(const ByteString& face_name,
-                                     int weight,
-                                     bool bItalic,
-                                     uint8_t* pData,
-                                     uint32_t size,
-                                     int face_index) {
+FXFT_Face CFX_FontMgr::AddCachedFace(
+    const ByteString& face_name,
+    int weight,
+    bool bItalic,
+    std::unique_ptr<uint8_t, FxFreeDeleter> pData,
+    uint32_t size,
+    int face_index) {
   InitFTLibrary();
 
   FXFT_Face face = nullptr;
-  int ret = FXFT_New_Memory_Face(m_FTLibrary, pData, size, face_index, &face);
+  int ret =
+      FXFT_New_Memory_Face(m_FTLibrary, pData.get(), size, face_index, &face);
   if (ret)
     return nullptr;
 
@@ -146,10 +148,11 @@ FXFT_Face CFX_FontMgr::AddCachedFace(const ByteString& face_name,
   if (ret)
     return nullptr;
 
-  auto pFontDesc = pdfium::MakeUnique<CTTFontDesc>(pData, face);
+  auto pFontDesc = pdfium::MakeUnique<CTTFontDesc>(std::move(pData));
+  pFontDesc->SetFace(0, face);
   CTTFontDesc* pResult = pFontDesc.get();
   m_FaceMap[KeyNameFromFace(face_name, weight, bItalic)] = std::move(pFontDesc);
-  return pResult->SingleFace();
+  return pResult->GetFace(0);
 }
 
 FXFT_Face CFX_FontMgr::GetCachedTTCFace(int ttc_size,
@@ -163,22 +166,24 @@ FXFT_Face CFX_FontMgr::GetCachedTTCFace(int ttc_size,
   CTTFontDesc* pFontDesc = it->second.get();
   *pFontData = pFontDesc->FontData();
   int face_index = GetTTCIndex(pFontDesc->FontData(), ttc_size, font_offset);
-  if (!pFontDesc->TTCFace(face_index)) {
-    pFontDesc->SetTTCFace(
+  if (!pFontDesc->GetFace(face_index)) {
+    pFontDesc->SetFace(
         face_index, GetFixedFace(pFontDesc->FontData(), ttc_size, face_index));
   }
   pFontDesc->AddRef();
-  return pFontDesc->TTCFace(face_index);
+  return pFontDesc->GetFace(face_index);
 }
 
-FXFT_Face CFX_FontMgr::AddCachedTTCFace(int ttc_size,
-                                        uint32_t checksum,
-                                        uint8_t* pData,
-                                        uint32_t size,
-                                        int font_offset) {
-  int face_index = GetTTCIndex(pData, ttc_size, font_offset);
-  FXFT_Face face = GetFixedFace(pData, ttc_size, face_index);
-  auto pFontDesc = pdfium::MakeUnique<CTTFontDesc>(pData, face_index, face);
+FXFT_Face CFX_FontMgr::AddCachedTTCFace(
+    int ttc_size,
+    uint32_t checksum,
+    std::unique_ptr<uint8_t, FxFreeDeleter> pData,
+    uint32_t size,
+    int font_offset) {
+  int face_index = GetTTCIndex(pData.get(), ttc_size, font_offset);
+  FXFT_Face face = GetFixedFace(pData.get(), ttc_size, face_index);
+  auto pFontDesc = pdfium::MakeUnique<CTTFontDesc>(std::move(pData));
+  pFontDesc->SetFace(face_index, face);
   m_FaceMap[KeyNameFromSize(ttc_size, checksum)] = std::move(pFontDesc);
   return face;
 }
diff --git a/core/fxge/cfx_fontmgr.h b/core/fxge/cfx_fontmgr.h
index 6ca0970bdc..24c5ad3b6f 100644
--- a/core/fxge/cfx_fontmgr.h
+++ b/core/fxge/cfx_fontmgr.h
@@ -10,6 +10,7 @@
 #include <map>
 #include <memory>
 
+#include "core/fxcrt/fx_memory.h"
 #include "core/fxge/fx_font.h"
 
 class CFX_FontMapper;
@@ -31,7 +32,7 @@ class CFX_FontMgr {
   FXFT_Face AddCachedFace(const ByteString& face_name,
                           int weight,
                           bool bItalic,
-                          uint8_t* pData,
+                          std::unique_ptr<uint8_t, FxFreeDeleter> pData,
                           uint32_t size,
                           int face_index);
   FXFT_Face GetCachedTTCFace(int ttc_size,
@@ -40,7 +41,7 @@ class CFX_FontMgr {
                              uint8_t** pFontData);
   FXFT_Face AddCachedTTCFace(int ttc_size,
                              uint32_t checksum,
-                             uint8_t* pData,
+                             std::unique_ptr<uint8_t, FxFreeDeleter> pData,
                              uint32_t size,
                              int font_offset);
   FXFT_Face GetFileFace(const char* filename, int face_index);
diff --git a/core/fxge/cttfontdesc.cpp b/core/fxge/cttfontdesc.cpp
index 97a12732e7..3d0b27b159 100644
--- a/core/fxge/cttfontdesc.cpp
+++ b/core/fxge/cttfontdesc.cpp
@@ -6,35 +6,26 @@
 
 #include "core/fxge/cttfontdesc.h"
 
+#include <utility>
+
 #include "core/fxge/fx_freetype.h"
 #include "third_party/base/stl_util.h"
 
-CTTFontDesc::CTTFontDesc(uint8_t* pData, FXFT_Face face)
-    : m_bIsTTC(false), m_pFontData(pData), m_SingleFace(face) {}
-
-CTTFontDesc::CTTFontDesc(uint8_t* pData, size_t index, FXFT_Face face)
-    : m_bIsTTC(true), m_pFontData(pData) {
+CTTFontDesc::CTTFontDesc(std::unique_ptr<uint8_t, FxFreeDeleter> pData)
+    : m_pFontData(std::move(pData)) {
   for (size_t i = 0; i < FX_ArraySize(m_TTCFaces); i++)
     m_TTCFaces[i] = nullptr;
-  SetTTCFace(index, face);
 }
 
 CTTFontDesc::~CTTFontDesc() {
   ASSERT(m_RefCount == 0);
-  if (m_bIsTTC) {
-    for (size_t i = 0; i < FX_ArraySize(m_TTCFaces); i++) {
-      if (m_TTCFaces[i])
-        FXFT_Done_Face(m_TTCFaces[i]);
-    }
-  } else {
-    if (m_SingleFace)
-      FXFT_Done_Face(m_SingleFace);
+  for (size_t i = 0; i < FX_ArraySize(m_TTCFaces); i++) {
+    if (m_TTCFaces[i])
+      FXFT_Done_Face(m_TTCFaces[i]);
   }
-  FX_Free(m_pFontData);
 }
 
-void CTTFontDesc::SetTTCFace(size_t index, FXFT_Face face) {
-  ASSERT(m_bIsTTC);
+void CTTFontDesc::SetFace(size_t index, FXFT_Face face) {
   ASSERT(index < FX_ArraySize(m_TTCFaces));
   m_TTCFaces[index] = face;
 }
@@ -45,24 +36,14 @@ void CTTFontDesc::AddRef() {
 }
 
 CTTFontDesc::ReleaseStatus CTTFontDesc::ReleaseFace(FXFT_Face face) {
-  if (m_bIsTTC) {
-    if (!pdfium::ContainsValue(m_TTCFaces, face))
-      return kNotAppropriate;
-  } else {
-    if (m_SingleFace != face)
-      return kNotAppropriate;
-  }
+  if (!pdfium::ContainsValue(m_TTCFaces, face))
+    return kNotAppropriate;
+
   ASSERT(m_RefCount > 0);
   return --m_RefCount == 0 ? kReleased : kNotReleased;
 }
 
-FXFT_Face CTTFontDesc::SingleFace() const {
-  ASSERT(!m_bIsTTC);
-  return m_SingleFace;
-}
-
-FXFT_Face CTTFontDesc::TTCFace(size_t index) const {
-  ASSERT(m_bIsTTC);
+FXFT_Face CTTFontDesc::GetFace(size_t index) const {
   ASSERT(index < FX_ArraySize(m_TTCFaces));
   return m_TTCFaces[index];
 }
diff --git a/core/fxge/cttfontdesc.h b/core/fxge/cttfontdesc.h
index bc0ea1737f..e73b03f6ec 100644
--- a/core/fxge/cttfontdesc.h
+++ b/core/fxge/cttfontdesc.h
@@ -7,6 +7,9 @@
 #ifndef CORE_FXGE_CTTFONTDESC_H_
 #define CORE_FXGE_CTTFONTDESC_H_
 
+#include <memory>
+
+#include "core/fxcrt/fx_memory.h"
 #include "core/fxcrt/fx_system.h"
 #include "core/fxge/fx_font.h"
 
@@ -18,34 +21,23 @@ class CTTFontDesc {
     kNotReleased  // Object still alive.
   };
 
-  // Single face ctor.
-  CTTFontDesc(uint8_t* pData, FXFT_Face face);
-
-  // TTC face ctor.
-  CTTFontDesc(uint8_t* pData, size_t index, FXFT_Face face);
-
+  explicit CTTFontDesc(std::unique_ptr<uint8_t, FxFreeDeleter> pData);
   ~CTTFontDesc();
 
-  void SetTTCFace(size_t index, FXFT_Face face);
+  void SetFace(size_t index, FXFT_Face face);
 
   void AddRef();
 
   // May not decrement refcount, depending on the value of |face|.
   ReleaseStatus ReleaseFace(FXFT_Face face);
 
-  uint8_t* FontData() const { return m_pFontData; }
-
-  FXFT_Face SingleFace() const;
-  FXFT_Face TTCFace(size_t index) const;
+  uint8_t* FontData() const { return m_pFontData.get(); }
+  FXFT_Face GetFace(size_t index) const;
 
  private:
-  const bool m_bIsTTC;
   int m_RefCount = 1;
-  uint8_t* const m_pFontData;
-  union {
-    const FXFT_Face m_SingleFace;
-    FXFT_Face m_TTCFaces[16];
-  };
+  std::unique_ptr<uint8_t, FxFreeDeleter> const m_pFontData;
+  FXFT_Face m_TTCFaces[16];
 };
 
 #endif  // CORE_FXGE_CTTFONTDESC_H_