Replace CountedFaceCache with RetainPtr<CFX_FaceCache>
Removes some add-hoc refcounting in favor of smart pointers. The
combination of Retainable and Observable creates a cache entry that
remains valid so long as there is at least one retain ptr outstanding.
Change-Id: I14f3c0b3ec42d9b845b6fb3965b0d7e6f9fc1a28
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/54272
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxge/cfx_facecache.h b/core/fxge/cfx_facecache.h
index 224dd37..a99ddb3 100644
--- a/core/fxge/cfx_facecache.h
+++ b/core/fxge/cfx_facecache.h
@@ -12,6 +12,8 @@
#include <tuple>
#include "core/fxcrt/fx_string.h"
+#include "core/fxcrt/observable.h"
+#include "core/fxcrt/retain_ptr.h"
#include "core/fxge/fx_freetype.h"
#if defined _SKIA_SUPPORT_ || _SKIA_SUPPORT_PATHS_
@@ -24,10 +26,13 @@
class CFX_Matrix;
class CFX_PathData;
-class CFX_FaceCache {
+class CFX_FaceCache : public Retainable, public Observable<CFX_FaceCache> {
public:
- explicit CFX_FaceCache(FXFT_Face face);
- ~CFX_FaceCache();
+ template <typename T, typename... Args>
+ friend RetainPtr<T> pdfium::MakeRetain(Args&&... args);
+
+ ~CFX_FaceCache() override;
+
const CFX_GlyphBitmap* LoadGlyphBitmap(const CFX_Font* pFont,
uint32_t glyph_index,
bool bFontStyle,
@@ -44,6 +49,8 @@
#endif
private:
+ explicit CFX_FaceCache(FXFT_Face face);
+
using SizeGlyphCache = std::map<uint32_t, std::unique_ptr<CFX_GlyphBitmap>>;
// <glyph_index, width, weight, angle, vertical>
using PathMapKey = std::tuple<uint32_t, uint32_t, int, int, bool>;
diff --git a/core/fxge/cfx_font.cpp b/core/fxge/cfx_font.cpp
index 79b57f0..2f39a0b 100644
--- a/core/fxge/cfx_font.cpp
+++ b/core/fxge/cfx_font.cpp
@@ -581,18 +581,14 @@
return true;
}
-CFX_FaceCache* CFX_Font::GetFaceCache() const {
+RetainPtr<CFX_FaceCache> CFX_Font::GetOrCreateFaceCache() const {
if (!m_FaceCache)
m_FaceCache = CFX_GEModule::Get()->GetFontCache()->GetCachedFace(this);
- return m_FaceCache.Get();
+ return m_FaceCache;
}
void CFX_Font::ClearFaceCache() {
- if (!m_FaceCache)
- return;
-
m_FaceCache = nullptr;
- CFX_GEModule::Get()->GetFontCache()->ReleaseCachedFace(this);
}
void CFX_Font::AdjustMMParams(int glyph_index,
@@ -713,17 +709,18 @@
uint32_t dest_width,
int anti_alias,
int* pTextFlags) const {
- return GetFaceCache()->LoadGlyphBitmap(this, glyph_index, bFontStyle, matrix,
- dest_width, anti_alias, pTextFlags);
+ return GetOrCreateFaceCache()->LoadGlyphBitmap(this, glyph_index, bFontStyle,
+ matrix, dest_width, anti_alias,
+ pTextFlags);
}
const CFX_PathData* CFX_Font::LoadGlyphPath(uint32_t glyph_index,
uint32_t dest_width) const {
- return GetFaceCache()->LoadGlyphPath(this, glyph_index, dest_width);
+ return GetOrCreateFaceCache()->LoadGlyphPath(this, glyph_index, dest_width);
}
#if defined _SKIA_SUPPORT_ || _SKIA_SUPPORT_PATHS_
CFX_TypeFace* CFX_Font::GetDeviceCache() const {
- return GetFaceCache()->GetDeviceCache(this);
+ return GetOrCreateFaceCache()->GetDeviceCache(this);
}
#endif
diff --git a/core/fxge/cfx_font.h b/core/fxge/cfx_font.h
index 8fe39ea..ab3fe6d 100644
--- a/core/fxge/cfx_font.h
+++ b/core/fxge/cfx_font.h
@@ -126,7 +126,7 @@
#endif // PDF_ENABLE_XFA
private:
- CFX_FaceCache* GetFaceCache() const;
+ RetainPtr<CFX_FaceCache> GetOrCreateFaceCache() const;
void DeleteFace();
void ClearFaceCache();
#if defined(OS_MACOSX)
@@ -136,7 +136,7 @@
ByteString GetFamilyNameOrUntitled() const;
mutable UnownedPtr<FXFT_FaceRec> m_Face;
- mutable UnownedPtr<CFX_FaceCache> m_FaceCache;
+ mutable RetainPtr<CFX_FaceCache> m_FaceCache;
std::unique_ptr<CFX_SubstFont> m_pSubstFont;
std::unique_ptr<uint8_t, FxFreeDeleter> m_pGsubData;
std::vector<uint8_t> m_pFontDataAllocation;
diff --git a/core/fxge/cfx_fontcache.cpp b/core/fxge/cfx_fontcache.cpp
index 5ccac8c..b9300bc 100644
--- a/core/fxge/cfx_fontcache.cpp
+++ b/core/fxge/cfx_fontcache.cpp
@@ -10,40 +10,26 @@
#include <utility>
#include "core/fxge/cfx_facecache.h"
+#include "core/fxge/cfx_font.h"
#include "core/fxge/fx_font.h"
#include "core/fxge/fx_freetype.h"
#include "third_party/base/ptr_util.h"
-CFX_FontCache::CountedFaceCache::CountedFaceCache() {}
+CFX_FontCache::CFX_FontCache() = default;
-CFX_FontCache::CountedFaceCache::~CountedFaceCache() {}
+CFX_FontCache::~CFX_FontCache() = default;
-CFX_FontCache::CFX_FontCache() {}
-
-CFX_FontCache::~CFX_FontCache() {
- ASSERT(m_ExtFaceMap.empty());
- ASSERT(m_FTFaceMap.empty());
-}
-
-CFX_FaceCache* CFX_FontCache::GetCachedFace(const CFX_Font* pFont) {
+RetainPtr<CFX_FaceCache> CFX_FontCache::GetCachedFace(const CFX_Font* pFont) {
FXFT_Face face = pFont->GetFace();
const bool bExternal = !face;
- CFX_FTCacheMap& map = bExternal ? m_ExtFaceMap : m_FTFaceMap;
+ auto& map = bExternal ? m_ExtFaceMap : m_FTFaceMap;
auto it = map.find(face);
- if (it != map.end()) {
- CountedFaceCache* counted_face_cache = it->second.get();
- counted_face_cache->m_nCount++;
- return counted_face_cache->m_Obj.get();
- }
+ if (it != map.end() && it->second)
+ return pdfium::WrapRetain(it->second.Get());
- auto counted_face_cache = pdfium::MakeUnique<CountedFaceCache>();
- counted_face_cache->m_nCount = 2;
- auto new_cache =
- pdfium::MakeUnique<CFX_FaceCache>(bExternal ? nullptr : face);
- CFX_FaceCache* face_cache = new_cache.get();
- counted_face_cache->m_Obj = std::move(new_cache);
- map[face] = std::move(counted_face_cache);
- return face_cache;
+ auto new_cache = pdfium::MakeRetain<CFX_FaceCache>(face);
+ map[face].Reset(new_cache.Get());
+ return new_cache;
}
#ifdef _SKIA_SUPPORT_
@@ -51,20 +37,3 @@
return GetCachedFace(pFont)->GetDeviceCache(pFont);
}
#endif
-
-void CFX_FontCache::ReleaseCachedFace(const CFX_Font* pFont) {
- FXFT_Face face = pFont->GetFace();
- const bool bExternal = !face;
- CFX_FTCacheMap& map = bExternal ? m_ExtFaceMap : m_FTFaceMap;
-
- auto it = map.find(face);
- if (it == map.end())
- return;
-
- CountedFaceCache* counted_face_cache = it->second.get();
- if (counted_face_cache->m_nCount > 2) {
- counted_face_cache->m_nCount--;
- } else {
- map.erase(it);
- }
-}
diff --git a/core/fxge/cfx_fontcache.h b/core/fxge/cfx_fontcache.h
index 44a4576..516e779 100644
--- a/core/fxge/cfx_fontcache.h
+++ b/core/fxge/cfx_fontcache.h
@@ -11,32 +11,24 @@
#include <memory>
#include "core/fxcrt/fx_system.h"
-#include "core/fxge/cfx_font.h"
+#include "core/fxge/cfx_facecache.h"
#include "core/fxge/fx_freetype.h"
-class CFX_FaceCache;
+class CFX_Font;
class CFX_FontCache {
public:
CFX_FontCache();
~CFX_FontCache();
- CFX_FaceCache* GetCachedFace(const CFX_Font* pFont);
- void ReleaseCachedFace(const CFX_Font* pFont);
+
+ RetainPtr<CFX_FaceCache> GetCachedFace(const CFX_Font* pFont);
#ifdef _SKIA_SUPPORT_
CFX_TypeFace* GetDeviceCache(const CFX_Font* pFont);
#endif
private:
- struct CountedFaceCache {
- CountedFaceCache();
- ~CountedFaceCache();
- std::unique_ptr<CFX_FaceCache> m_Obj;
- uint32_t m_nCount;
- };
-
- using CFX_FTCacheMap = std::map<FXFT_Face, std::unique_ptr<CountedFaceCache>>;
- CFX_FTCacheMap m_FTFaceMap;
- CFX_FTCacheMap m_ExtFaceMap;
+ std::map<FXFT_Face, CFX_FaceCache::ObservedPtr> m_FTFaceMap;
+ std::map<FXFT_Face, CFX_FaceCache::ObservedPtr> m_ExtFaceMap;
};
#endif // CORE_FXGE_CFX_FONTCACHE_H_
diff --git a/core/fxge/win32/cfx_psrenderer.cpp b/core/fxge/win32/cfx_psrenderer.cpp
index f2050c3..d684600 100644
--- a/core/fxge/win32/cfx_psrenderer.cpp
+++ b/core/fxge/win32/cfx_psrenderer.cpp
@@ -664,11 +664,11 @@
<< pObject2Device->e << " " << pObject2Device->f << "]cm\n";
CFX_FontCache* pCache = CFX_GEModule::Get()->GetFontCache();
- CFX_FaceCache* pFaceCache = pCache->GetCachedFace(pFont);
+ RetainPtr<CFX_FaceCache> pFaceCache = pCache->GetCachedFace(pFont);
int last_fontnum = -1;
for (int i = 0; i < nChars; i++) {
int ps_fontnum, ps_glyphindex;
- FindPSFontGlyph(pFaceCache, pFont, pCharPos[i], &ps_fontnum,
+ FindPSFontGlyph(pFaceCache.Get(), pFont, pCharPos[i], &ps_fontnum,
&ps_glyphindex);
if (last_fontnum != ps_fontnum) {
buf << "/X" << ps_fontnum << " Ff " << font_size << " Fs Sf ";
@@ -680,7 +680,6 @@
}
buf << "Q\n";
WriteToStream(&buf);
- pCache->ReleaseCachedFace(pFont);
return true;
}
