[XFA] Force destruction order of font managers.
The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.
The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.
BUG=chromium:637546
Review-Url: https://codereview.chromium.org/2259823004
diff --git a/xfa/fgas/font/fgas_stdfontmgr.cpp b/xfa/fgas/font/fgas_stdfontmgr.cpp
index ab7852e..7e5cfbb 100644
--- a/xfa/fgas/font/fgas_stdfontmgr.cpp
+++ b/xfa/fgas/font/fgas_stdfontmgr.cpp
@@ -1156,7 +1156,6 @@
}
void CFGAS_FontMgrImp::RegisterFace(FXFT_Face pFace,
- CFX_FontDescriptors& Fonts,
const CFX_WideString* pFaceName) {
if ((pFace->face_flags & FT_FACE_FLAG_SCALABLE) == 0)
return;
@@ -1188,7 +1187,7 @@
: CFX_WideString::FromLocal(FXFT_Get_Postscript_Name(pFace));
pFont->m_nFaceIndex = pFace->face_index;
- Fonts.Add(pFont.release());
+ m_InstalledFonts.Add(pFont.release());
}
void CFGAS_FontMgrImp::RegisterFaces(IFX_FileRead* pFontStream,
@@ -1202,7 +1201,7 @@
// All faces keep number of faces. It can be retrieved from any one face.
if (num_faces == 0)
num_faces = pFace->num_faces;
- RegisterFace(pFace, m_InstalledFonts, pFaceName);
+ RegisterFace(pFace, pFaceName);
if (FXFT_Get_Face_External_Stream(pFace))
FXFT_Clear_Face_External_Stream(pFace);
FXFT_Done_Face(pFace);
diff --git a/xfa/fgas/font/fgas_stdfontmgr.h b/xfa/fgas/font/fgas_stdfontmgr.h
index 65f260c..0506876 100644
--- a/xfa/fgas/font/fgas_stdfontmgr.h
+++ b/xfa/fgas/font/fgas_stdfontmgr.h
@@ -184,7 +184,6 @@
protected:
void RegisterFace(FXFT_Face pFace,
- CFX_FontDescriptors& Fonts,
const CFX_WideString* pFaceName);
void RegisterFaces(IFX_FileRead* pFontStream,
const CFX_WideString* pFaceName);
@@ -192,7 +191,6 @@
std::vector<uint16_t> GetCharsets(FXFT_Face pFace) const;
void GetUSBCSB(FXFT_Face pFace, uint32_t* USB, uint32_t* CSB);
uint32_t GetFlags(FXFT_Face pFace);
- CFX_FontDescriptors m_InstalledFonts;
FX_BOOL VerifyUnicode(CFX_FontDescriptor* pDesc, FX_WCHAR wcUnicode);
FX_BOOL VerifyUnicode(CFGAS_GEFont* pFont, FX_WCHAR wcUnicode);
int32_t IsPartName(const CFX_WideString& Name1, const CFX_WideString& Name2);
@@ -212,6 +210,7 @@
uint32_t index);
IFX_FileRead* CreateFontStream(const CFX_ByteString& bsFaceName);
+ CFX_FontDescriptors m_InstalledFonts;
CFX_MapPtrTemplate<uint32_t, CFX_FontDescriptorInfos*> m_Hash2CandidateList;
CFX_MapPtrTemplate<uint32_t, CFX_ArrayTemplate<CFGAS_GEFont*>*> m_Hash2Fonts;
CFX_MapPtrTemplate<CFGAS_GEFont*, IFX_FileRead*> m_IFXFont2FileRead;
diff --git a/xfa/fxfa/app/xfa_fontmgr.cpp b/xfa/fxfa/app/xfa_fontmgr.cpp
index 5bb4ccb..d191ce3 100644
--- a/xfa/fxfa/app/xfa_fontmgr.cpp
+++ b/xfa/fxfa/app/xfa_fontmgr.cpp
@@ -1775,18 +1775,17 @@
CFX_WideString wsReplace =
CFX_WideString(pReplace, pNameText - pReplace);
pFont = pFDEFontMgr->LoadFont(wsReplace.c_str(), dwStyle, wCodePage);
- if (pFont) {
+ if (pFont)
break;
- }
+
iLength--;
pNameText++;
pReplace = pNameText;
}
}
}
- if (pFont) {
+ if (pFont)
m_CacheFonts.Add(pFont);
- }
return pFont;
}
@@ -1804,9 +1803,8 @@
}
ASSERT(pFont);
- if (pFont) {
+ if (pFont)
m_CacheFonts.Add(pFont);
- }
return pFont;
}
struct XFA_PDFFONTNAME {
diff --git a/xfa/fxfa/include/xfa_ffapp.h b/xfa/fxfa/include/xfa_ffapp.h
index bc0d6df..90bfcc0 100644
--- a/xfa/fxfa/include/xfa_ffapp.h
+++ b/xfa/fxfa/include/xfa_ffapp.h
@@ -63,13 +63,26 @@
protected:
std::unique_ptr<CXFA_FFDocHandler> m_pDocHandler;
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
+ // get 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.
+ //
+ // TODO(dsinclair): The GEFont should have the FontMgr as the pointer instead
+ // of the DEFFontMgr so this goes away. Bug 561.
+ std::unique_ptr<IFGAS_FontMgr> m_pFDEFontMgr;
std::unique_ptr<CXFA_FontMgr> m_pFontMgr;
+
#if _FXM_PLATFORM_ != _FXM_PLATFORM_WINDOWS_
std::unique_ptr<CFX_FontSourceEnum_File> m_pFontSource;
#endif
std::unique_ptr<CXFA_FWLAdapterWidgetMgr> m_pAdapterWidgetMgr;
CFWL_WidgetMgrDelegate* m_pWidgetMgrDelegate; // not owned.
- std::unique_ptr<IFGAS_FontMgr> m_pFDEFontMgr;
+
// |m_pFWLApp| has to be released first, then |m_pFWLTheme| since the former
// may refers to theme manager and the latter refers to font manager.
std::unique_ptr<CXFA_FWLTheme> m_pFWLTheme;
