Make CXFA_FontMgr a garbage-collected class Bug: pdfium:1563 Change-Id: I54d66004667d45fd9cc2c8bdac6bb48ecbe192b0 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73430 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/xfa/fxfa/cxfa_ffapp.cpp b/xfa/fxfa/cxfa_ffapp.cpp index dbcdeec..70518e1 100644 --- a/xfa/fxfa/cxfa_ffapp.cpp +++ b/xfa/fxfa/cxfa_ffapp.cpp
@@ -18,9 +18,10 @@ #include "xfa/fxfa/cxfa_fwladapterwidgetmgr.h" #include "xfa/fxfa/cxfa_fwltheme.h" -CXFA_FFApp::CXFA_FFApp(IXFA_AppProvider* pProvider) - : m_pProvider(pProvider), m_pXFAFontMgr(std::make_unique<CXFA_FontMgr>()) { +CXFA_FFApp::CXFA_FFApp(IXFA_AppProvider* pProvider) : m_pProvider(pProvider) { // Ensure fully initialized before making objects based on |this|. + m_pXFAFontMgr = cppgc::MakeGarbageCollected<CXFA_FontMgr>( + GetHeap()->GetAllocationHandle()); m_pFWLApp = cppgc::MakeGarbageCollected<CFWL_App>( GetHeap()->GetAllocationHandle(), this); } @@ -28,6 +29,7 @@ CXFA_FFApp::~CXFA_FFApp() = default; void CXFA_FFApp::Trace(cppgc::Visitor* visitor) const { + visitor->Trace(m_pXFAFontMgr); visitor->Trace(m_pAdapterWidgetMgr); visitor->Trace(m_pFWLTheme); visitor->Trace(m_pFWLApp);
diff --git a/xfa/fxfa/cxfa_ffapp.h b/xfa/fxfa/cxfa_ffapp.h index d1f90e0..59a4adc 100644 --- a/xfa/fxfa/cxfa_ffapp.h +++ b/xfa/fxfa/cxfa_ffapp.h
@@ -39,21 +39,13 @@ CFWL_WidgetMgr* GetFWLWidgetMgr() const { return m_pFWLApp->GetWidgetMgr(); } IXFA_AppProvider* GetAppProvider() const { return m_pProvider.Get(); } CFWL_App* GetFWLApp() const { return m_pFWLApp; } - CXFA_FontMgr* GetXFAFontMgr() const { return m_pXFAFontMgr.get(); } + CXFA_FontMgr* GetXFAFontMgr() const { return m_pXFAFontMgr; } private: explicit CXFA_FFApp(IXFA_AppProvider* pProvider); UnownedPtr<IXFA_AppProvider> const m_pProvider; - - // The fonts stored in the font manager may have been created by the default - // font manager. The GEFont::LoadFont call takes the manager as a param and - // stores it internally. When you destroy the GEFont it tries to unregister - // from the font manager and if the default font manager was destroyed first - // you get a use-after-free. The m_pFWLTheme can try to cleanup a GEFont - // when it frees, so make sure it gets cleaned up first. That requires - // m_pFWLApp to be cleaned up as well. - std::unique_ptr<CXFA_FontMgr> m_pXFAFontMgr; + cppgc::Member<CXFA_FontMgr> m_pXFAFontMgr; cppgc::Member<CXFA_FWLAdapterWidgetMgr> m_pAdapterWidgetMgr; cppgc::Member<CXFA_FWLTheme> m_pFWLTheme; cppgc::Member<CFWL_App> m_pFWLApp;
diff --git a/xfa/fxfa/cxfa_fontmgr.cpp b/xfa/fxfa/cxfa_fontmgr.cpp index b20b778..aae2461 100644 --- a/xfa/fxfa/cxfa_fontmgr.cpp +++ b/xfa/fxfa/cxfa_fontmgr.cpp
@@ -24,6 +24,8 @@ CXFA_FontMgr::~CXFA_FontMgr() = default; +void CXFA_FontMgr::Trace(cppgc::Visitor* visitor) const {} + RetainPtr<CFGAS_GEFont> CXFA_FontMgr::GetFont(CXFA_FFDoc* hDoc, WideStringView wsFontFamily, uint32_t dwFontStyles) {
diff --git a/xfa/fxfa/cxfa_fontmgr.h b/xfa/fxfa/cxfa_fontmgr.h index 2ad45dc..a85c478 100644 --- a/xfa/fxfa/cxfa_fontmgr.h +++ b/xfa/fxfa/cxfa_fontmgr.h
@@ -10,20 +10,25 @@ #include <map> #include "core/fxcrt/fx_string.h" +#include "fxjs/gc/heap.h" +#include "v8/include/cppgc/garbage-collected.h" class CFGAS_GEFont; class CXFA_FFDoc; -class CXFA_FontMgr { +class CXFA_FontMgr final : public cppgc::GarbageCollected<CXFA_FontMgr> { public: - CXFA_FontMgr(); + CONSTRUCT_VIA_MAKE_GARBAGE_COLLECTED; ~CXFA_FontMgr(); + void Trace(cppgc::Visitor* visitor) const; RetainPtr<CFGAS_GEFont> GetFont(CXFA_FFDoc* hDoc, WideStringView wsFontFamily, uint32_t dwFontStyles); private: + CXFA_FontMgr(); + std::map<ByteString, RetainPtr<CFGAS_GEFont>> m_FontMap; };