Weakly cache CFX_FontMgr::FontDescs.
The intent is keep the FontDesc alive only so long as there at least
one CFX_Face that is still using it. This is similar to the
original manually counted implementation from FX than the current
implementation where we keep the FontDesc around for the lifetime
of the FontMgr.
- Avoid some circularity between CFX_Face and CFX_FontMgr::FontDesc
by only telling CFX_Face that it has a retainable.
Change-Id: If8b441b9b1c2aa81892cb4cb62762a44a322dd0c
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/56652
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxge/cfx_face.cpp b/core/fxge/cfx_face.cpp
index c99210c..6a41376 100644
--- a/core/fxge/cfx_face.cpp
+++ b/core/fxge/cfx_face.cpp
@@ -8,6 +8,7 @@
// static
RetainPtr<CFX_Face> CFX_Face::New(FT_Library library,
+ const RetainPtr<Retainable>& pDesc,
pdfium::span<const FT_Byte> data,
FT_Long face_index) {
FXFT_FaceRec* pRec = nullptr;
@@ -15,7 +16,7 @@
&pRec) != 0) {
return nullptr;
}
- return pdfium::WrapRetain(new CFX_Face(pRec));
+ return pdfium::WrapRetain(new CFX_Face(pRec, pDesc));
}
// static
@@ -26,10 +27,11 @@
if (FT_Open_Face(library, args, face_index, &pRec) != 0)
return nullptr;
- return pdfium::WrapRetain(new CFX_Face(pRec));
+ return pdfium::WrapRetain(new CFX_Face(pRec, nullptr));
}
-CFX_Face::CFX_Face(FXFT_FaceRec* rec) : m_pRec(rec) {
+CFX_Face::CFX_Face(FXFT_FaceRec* rec, const RetainPtr<Retainable>& pDesc)
+ : m_pRec(rec), m_pDesc(pDesc) {
ASSERT(m_pRec);
}
diff --git a/core/fxge/cfx_face.h b/core/fxge/cfx_face.h
index fd7abdf..fb8b8a4 100644
--- a/core/fxge/cfx_face.h
+++ b/core/fxge/cfx_face.h
@@ -13,6 +13,7 @@
class CFX_Face : public Retainable, public Observable {
public:
static RetainPtr<CFX_Face> New(FT_Library library,
+ const RetainPtr<Retainable>& pDesc,
pdfium::span<const FT_Byte> file_span,
FT_Long face_index);
@@ -25,9 +26,10 @@
FXFT_FaceRec* GetRec() { return m_pRec.get(); }
private:
- explicit CFX_Face(FXFT_FaceRec* pRec);
+ CFX_Face(FXFT_FaceRec* pRec, const RetainPtr<Retainable>& pDesc);
ScopedFXFTFaceRec const m_pRec;
+ RetainPtr<Retainable> const m_pDesc;
};
#endif // CORE_FXGE_CFX_FACE_H_
diff --git a/core/fxge/cfx_font.cpp b/core/fxge/cfx_font.cpp
index e656c82..3bf8c69 100644
--- a/core/fxge/cfx_font.cpp
+++ b/core/fxge/cfx_font.cpp
@@ -377,8 +377,8 @@
bool CFX_Font::LoadEmbedded(pdfium::span<const uint8_t> src_span) {
m_pFontDataAllocation =
std::vector<uint8_t>(src_span.begin(), src_span.end());
- m_Face =
- CFX_GEModule::Get()->GetFontMgr()->NewFixedFace(m_pFontDataAllocation, 0);
+ m_Face = CFX_GEModule::Get()->GetFontMgr()->NewFixedFace(
+ nullptr, m_pFontDataAllocation, 0);
m_bEmbedded = true;
m_FontData = m_pFontDataAllocation;
return !!m_Face;
diff --git a/core/fxge/cfx_fontmapper.cpp b/core/fxge/cfx_fontmapper.cpp
index 6d0322a..d803a3a 100644
--- a/core/fxge/cfx_fontmapper.cpp
+++ b/core/fxge/cfx_fontmapper.cpp
@@ -355,7 +355,8 @@
Optional<pdfium::span<const uint8_t>> font_data =
m_pFontMgr->GetBuiltinFont(iBaseFont);
if (font_data.has_value()) {
- m_FoxitFaces[iBaseFont] = m_pFontMgr->NewFixedFace(font_data.value(), 0);
+ m_FoxitFaces[iBaseFont] =
+ m_pFontMgr->NewFixedFace(nullptr, font_data.value(), 0);
return m_FoxitFaces[iBaseFont];
}
}
@@ -367,15 +368,15 @@
pSubstFont->m_Weight = pSubstFont->m_Weight * 4 / 5;
pSubstFont->m_Family = "Chrome Serif";
if (!m_MMFaces[1]) {
- m_MMFaces[1] =
- m_pFontMgr->NewFixedFace(m_pFontMgr->GetBuiltinFont(14).value(), 0);
+ m_MMFaces[1] = m_pFontMgr->NewFixedFace(
+ nullptr, m_pFontMgr->GetBuiltinFont(14).value(), 0);
}
return m_MMFaces[1];
}
pSubstFont->m_Family = "Chrome Sans";
if (!m_MMFaces[0]) {
- m_MMFaces[0] =
- m_pFontMgr->NewFixedFace(m_pFontMgr->GetBuiltinFont(15).value(), 0);
+ m_MMFaces[0] = m_pFontMgr->NewFixedFace(
+ nullptr, m_pFontMgr->GetBuiltinFont(15).value(), 0);
}
return m_MMFaces[0];
}
@@ -681,7 +682,7 @@
uint32_t ttc_size,
uint32_t font_size) {
uint32_t checksum = GetChecksumFromTT(hFont);
- CFX_FontMgr::FontDesc* pFontDesc =
+ RetainPtr<CFX_FontMgr::FontDesc> pFontDesc =
m_pFontMgr->GetCachedTTCFontDesc(ttc_size, checksum);
if (!pFontDesc) {
std::unique_ptr<uint8_t, FxFreeDeleter> pFontData(
@@ -698,8 +699,8 @@
if (pFace)
return pFace;
- pFace = m_pFontMgr->NewFixedFace(pFontDesc->FontData().first(ttc_size),
- face_index);
+ pFace = m_pFontMgr->NewFixedFace(
+ pFontDesc, pFontDesc->FontData().first(ttc_size), face_index);
if (!pFace)
return nullptr;
@@ -712,7 +713,7 @@
int weight,
bool bItalic,
uint32_t font_size) {
- CFX_FontMgr::FontDesc* pFontDesc =
+ RetainPtr<CFX_FontMgr::FontDesc> pFontDesc =
m_pFontMgr->GetCachedFontDesc(SubstName, weight, bItalic);
if (!pFontDesc) {
std::unique_ptr<uint8_t, FxFreeDeleter> pFontData(
@@ -725,7 +726,8 @@
if (pFace)
return pFace;
- pFace = m_pFontMgr->NewFixedFace(pFontDesc->FontData().first(font_size),
+ pFace = m_pFontMgr->NewFixedFace(pFontDesc,
+ pFontDesc->FontData().first(font_size),
m_pFontInfo->GetFaceIndex(hFont));
if (!pFace)
return nullptr;
diff --git a/core/fxge/cfx_fontmgr.cpp b/core/fxge/cfx_fontmgr.cpp
index f47f055..7a08d0f 100644
--- a/core/fxge/cfx_fontmgr.cpp
+++ b/core/fxge/cfx_fontmgr.cpp
@@ -108,46 +108,47 @@
italic_angle, CharsetCP, pSubstFont);
}
-CFX_FontMgr::FontDesc* CFX_FontMgr::GetCachedFontDesc(
+RetainPtr<CFX_FontMgr::FontDesc> CFX_FontMgr::GetCachedFontDesc(
const ByteString& face_name,
int weight,
bool bItalic) {
auto it = m_FaceMap.find(KeyNameFromFace(face_name, weight, bItalic));
- return it != m_FaceMap.end() ? it->second.get() : nullptr;
+ return it != m_FaceMap.end() ? pdfium::WrapRetain(it->second.Get()) : nullptr;
}
-CFX_FontMgr::FontDesc* CFX_FontMgr::AddCachedFontDesc(
+RetainPtr<CFX_FontMgr::FontDesc> CFX_FontMgr::AddCachedFontDesc(
const ByteString& face_name,
int weight,
bool bItalic,
std::unique_ptr<uint8_t, FxFreeDeleter> pData,
uint32_t size) {
- auto pFontDesc = pdfium::MakeUnique<FontDesc>(std::move(pData), size);
- auto* result = pFontDesc.get();
- m_FaceMap[KeyNameFromFace(face_name, weight, bItalic)] = std::move(pFontDesc);
- return result;
+ auto pFontDesc = pdfium::MakeRetain<FontDesc>(std::move(pData), size);
+ m_FaceMap[KeyNameFromFace(face_name, weight, bItalic)].Reset(pFontDesc.Get());
+ return pFontDesc;
}
-CFX_FontMgr::FontDesc* CFX_FontMgr::GetCachedTTCFontDesc(int ttc_size,
- uint32_t checksum) {
+RetainPtr<CFX_FontMgr::FontDesc> CFX_FontMgr::GetCachedTTCFontDesc(
+ int ttc_size,
+ uint32_t checksum) {
auto it = m_FaceMap.find(KeyNameFromSize(ttc_size, checksum));
- return it != m_FaceMap.end() ? it->second.get() : nullptr;
+ return it != m_FaceMap.end() ? pdfium::WrapRetain(it->second.Get()) : nullptr;
}
-CFX_FontMgr::FontDesc* CFX_FontMgr::AddCachedTTCFontDesc(
+RetainPtr<CFX_FontMgr::FontDesc> CFX_FontMgr::AddCachedTTCFontDesc(
int ttc_size,
uint32_t checksum,
std::unique_ptr<uint8_t, FxFreeDeleter> pData,
uint32_t size) {
- auto pNewDesc = pdfium::MakeUnique<FontDesc>(std::move(pData), size);
- auto* pResult = pNewDesc.get();
- m_FaceMap[KeyNameFromSize(ttc_size, checksum)] = std::move(pNewDesc);
- return pResult;
+ auto pNewDesc = pdfium::MakeRetain<FontDesc>(std::move(pData), size);
+ m_FaceMap[KeyNameFromSize(ttc_size, checksum)].Reset(pNewDesc.Get());
+ return pNewDesc;
}
-RetainPtr<CFX_Face> CFX_FontMgr::NewFixedFace(pdfium::span<const uint8_t> span,
+RetainPtr<CFX_Face> CFX_FontMgr::NewFixedFace(const RetainPtr<FontDesc>& pDesc,
+ pdfium::span<const uint8_t> span,
int face_index) {
- RetainPtr<CFX_Face> face = CFX_Face::New(m_FTLibrary.get(), span, face_index);
+ RetainPtr<CFX_Face> face =
+ CFX_Face::New(m_FTLibrary.get(), pDesc, span, face_index);
if (!face)
return nullptr;
diff --git a/core/fxge/cfx_fontmgr.h b/core/fxge/cfx_fontmgr.h
index 0db8fa3..ff3a4bd 100644
--- a/core/fxge/cfx_fontmgr.h
+++ b/core/fxge/cfx_fontmgr.h
@@ -13,7 +13,7 @@
#include "core/fxcrt/fx_memory.h"
#include "core/fxcrt/fx_string.h"
#include "core/fxcrt/observed_ptr.h"
-#include "core/fxge/cfx_face.h"
+#include "core/fxcrt/retain_ptr.h"
#include "core/fxge/fx_freetype.h"
#include "third_party/base/optional.h"
#include "third_party/base/span.h"
@@ -25,10 +25,12 @@
class CFX_FontMgr {
public:
- class FontDesc {
+ class FontDesc final : public Retainable, public Observable {
public:
- FontDesc(std::unique_ptr<uint8_t, FxFreeDeleter> pData, size_t size);
- ~FontDesc();
+ template <typename T, typename... Args>
+ friend RetainPtr<T> pdfium::MakeRetain(Args&&... args);
+
+ ~FontDesc() override;
pdfium::span<uint8_t> FontData() const {
return {m_pFontData.get(), m_Size};
@@ -37,6 +39,8 @@
CFX_Face* GetFace(size_t index) const;
private:
+ FontDesc(std::unique_ptr<uint8_t, FxFreeDeleter> pData, size_t size);
+
const size_t m_Size;
std::unique_ptr<uint8_t, FxFreeDeleter> const m_pFontData;
ObservedPtr<CFX_Face> m_TTCFaces[16];
@@ -47,22 +51,25 @@
CFX_FontMgr();
~CFX_FontMgr();
- FontDesc* GetCachedFontDesc(const ByteString& face_name,
- int weight,
- bool bItalic);
- FontDesc* AddCachedFontDesc(const ByteString& face_name,
- int weight,
- bool bItalic,
- std::unique_ptr<uint8_t, FxFreeDeleter> pData,
- uint32_t size);
+ RetainPtr<FontDesc> GetCachedFontDesc(const ByteString& face_name,
+ int weight,
+ bool bItalic);
+ RetainPtr<FontDesc> AddCachedFontDesc(
+ const ByteString& face_name,
+ int weight,
+ bool bItalic,
+ std::unique_ptr<uint8_t, FxFreeDeleter> pData,
+ uint32_t size);
- FontDesc* GetCachedTTCFontDesc(int ttc_size, uint32_t checksum);
- FontDesc* AddCachedTTCFontDesc(int ttc_size,
- uint32_t checksum,
- std::unique_ptr<uint8_t, FxFreeDeleter> pData,
- uint32_t size);
+ RetainPtr<FontDesc> GetCachedTTCFontDesc(int ttc_size, uint32_t checksum);
+ RetainPtr<FontDesc> AddCachedTTCFontDesc(
+ int ttc_size,
+ uint32_t checksum,
+ std::unique_ptr<uint8_t, FxFreeDeleter> pData,
+ uint32_t size);
- RetainPtr<CFX_Face> NewFixedFace(pdfium::span<const uint8_t> span,
+ RetainPtr<CFX_Face> NewFixedFace(const RetainPtr<FontDesc>& pDesc,
+ pdfium::span<const uint8_t> span,
int face_index);
RetainPtr<CFX_Face> FindSubstFont(const ByteString& face_name,
bool bTrueType,
@@ -87,7 +94,7 @@
// Must come before |m_pBuiltinMapper| and |m_FaceMap|.
ScopedFXFTLibraryRec const m_FTLibrary;
std::unique_ptr<CFX_FontMapper> m_pBuiltinMapper;
- std::map<ByteString, std::unique_ptr<FontDesc>> m_FaceMap;
+ std::map<ByteString, ObservedPtr<FontDesc>> m_FaceMap;
const bool m_FTLibrarySupportsHinting;
};
