commit a7edf06ea615af0ac76378143d07cf495076384f
author Tom Sepez <tsepez@chromium.org> Wed Sep 09 22:48:17 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Sep 09 22:48:17 2020 +0000
tree a2e44f726a4ee3f9431294da0ce56250727817b7
parent a7f5b4964458653cf1e6b7542b0a6003517175fa

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>

xfa/fxfa/cxfa_ffapp.cpp

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);

xfa/fxfa/cxfa_ffapp.h

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;

xfa/fxfa/cxfa_fontmgr.cpp

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) {

xfa/fxfa/cxfa_fontmgr.h

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;
 };