Introduce CFX_GEModule::PlatformIface::CreateDefaultSystemFontInfo(). Replaces SystemFontInfoIface::CreateDefault(), which could not be implemented on android and was stubbed. The functional change are - FPDF_GetDefaultSystemFontInfo() on android should now work. - FPDF_GetDefaultSystemFontInfo() takes user-paths into consideration. The lack of user-paths meant that the object returned from this function would reference a different set of fonts that the one we actually installed by default at creation time. - Centralize setting default in CFX_GEModule itself. - Update a stale comment while we're at it. Change-Id: I621cc2e9e1ddcb12c0b843414bbefae3a0302bbe Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/73033 Commit-Queue: Tom Sepez <tsepez@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxge/android/cfx_androidfontinfo.cpp b/core/fxge/android/cfx_androidfontinfo.cpp index f70f275..dc25f1e 100644 --- a/core/fxge/android/cfx_androidfontinfo.cpp +++ b/core/fxge/android/cfx_androidfontinfo.cpp
@@ -80,8 +80,3 @@ } void CFX_AndroidFontInfo::DeleteFont(void* hFont) {} - -std::unique_ptr<SystemFontInfoIface> SystemFontInfoIface::CreateDefault( - const char** pUnused) { - return nullptr; -}
diff --git a/core/fxge/android/fx_android_impl.cpp b/core/fxge/android/fx_android_impl.cpp index f93223b..6b2000c 100644 --- a/core/fxge/android/fx_android_impl.cpp +++ b/core/fxge/android/fx_android_impl.cpp
@@ -20,15 +20,16 @@ m_pDeviceModule->Destroy(); } - void Init() override { - m_pDeviceModule = CFPF_GetSkiaDeviceModule(); + void Init() override { m_pDeviceModule = CFPF_GetSkiaDeviceModule(); } + + std::unique_ptr<SystemFontInfoIface> CreateDefaultSystemFontInfo() override { CFPF_SkiaFontMgr* pFontMgr = m_pDeviceModule->GetFontMgr(); if (!pFontMgr) - return; + return nullptr; auto pFontInfo = std::make_unique<CFX_AndroidFontInfo>(); pFontInfo->Init(pFontMgr); - CFX_GEModule::Get()->GetFontMgr()->SetSystemFontInfo(std::move(pFontInfo)); + return pFontInfo; } private:
diff --git a/core/fxge/apple/fx_mac_impl.cpp b/core/fxge/apple/fx_mac_impl.cpp index fc8d1bd..76a932b 100644 --- a/core/fxge/apple/fx_mac_impl.cpp +++ b/core/fxge/apple/fx_mac_impl.cpp
@@ -130,10 +130,16 @@ } } // namespace -std::unique_ptr<SystemFontInfoIface> SystemFontInfoIface::CreateDefault( - const char** pUserPaths) { +CApplePlatform::CApplePlatform() = default; + +CApplePlatform::~CApplePlatform() = default; + +void CApplePlatform::Init() {} + +std::unique_ptr<SystemFontInfoIface> +CApplePlatform::CreateDefaultSystemFontInfo() { auto pInfo = std::make_unique<CFX_MacFontInfo>(); - if (!pInfo->ParseFontCfg(pUserPaths)) { + if (!pInfo->ParseFontCfg(CFX_GEModule::Get()->GetUserFontPaths())) { pInfo->AddPath("~/Library/Fonts"); pInfo->AddPath("/Library/Fonts"); pInfo->AddPath("/System/Library/Fonts"); @@ -141,16 +147,6 @@ return std::move(pInfo); } -CApplePlatform::CApplePlatform() = default; - -CApplePlatform::~CApplePlatform() = default; - -void CApplePlatform::Init() { - CFX_GEModule* pModule = CFX_GEModule::Get(); - pModule->GetFontMgr()->SetSystemFontInfo( - SystemFontInfoIface::CreateDefault(pModule->GetUserFontPaths())); -} - // static std::unique_ptr<CFX_GEModule::PlatformIface> CFX_GEModule::PlatformIface::Create() {
diff --git a/core/fxge/apple/fx_mac_impl.h b/core/fxge/apple/fx_mac_impl.h index 23d0e63..d207639 100644 --- a/core/fxge/apple/fx_mac_impl.h +++ b/core/fxge/apple/fx_mac_impl.h
@@ -17,6 +17,7 @@ // CFX_GEModule::PlatformIface: void Init() override; + std::unique_ptr<SystemFontInfoIface> CreateDefaultSystemFontInfo() override; CQuartz2D m_quartz2d; };
diff --git a/core/fxge/cfx_gemodule.cpp b/core/fxge/cfx_gemodule.cpp index 9b6769d..46cfbee 100644 --- a/core/fxge/cfx_gemodule.cpp +++ b/core/fxge/cfx_gemodule.cpp
@@ -29,6 +29,8 @@ ASSERT(!g_pGEModule); g_pGEModule = new CFX_GEModule(pUserFontPaths); g_pGEModule->m_pPlatform->Init(); + g_pGEModule->GetFontMgr()->SetSystemFontInfo( + g_pGEModule->m_pPlatform->CreateDefaultSystemFontInfo()); } // static
diff --git a/core/fxge/cfx_gemodule.h b/core/fxge/cfx_gemodule.h index 47e2667..0303b57 100644 --- a/core/fxge/cfx_gemodule.h +++ b/core/fxge/cfx_gemodule.h
@@ -11,6 +11,7 @@ class CFX_FontCache; class CFX_FontMgr; +class SystemFontInfoIface; class CFX_GEModule { public: @@ -20,6 +21,8 @@ virtual ~PlatformIface() {} virtual void Init() = 0; + virtual std::unique_ptr<SystemFontInfoIface> + CreateDefaultSystemFontInfo() = 0; }; static void Create(const char** pUserFontPaths);
diff --git a/core/fxge/fx_font_unittest.cpp b/core/fxge/fx_font_unittest.cpp index e09db61..5045fbe 100644 --- a/core/fxge/fx_font_unittest.cpp +++ b/core/fxge/fx_font_unittest.cpp
@@ -6,6 +6,7 @@ #include "core/fxge/cfx_folderfontinfo.h" #include "core/fxge/cfx_fontmapper.h" +#include "core/fxge/cfx_gemodule.h" #include "core/fxge/fx_font.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/utils/path_service.h" @@ -45,7 +46,8 @@ folder_font_info.AddPath( (test_data_dir + PATH_SEPARATOR + "font_tests").c_str()); - font_mapper.SetSystemFontInfo(SystemFontInfoIface::CreateDefault(nullptr)); + font_mapper.SetSystemFontInfo( + CFX_GEModule::Get()->GetPlatform()->CreateDefaultSystemFontInfo()); ASSERT_TRUE(folder_font_info.EnumFontList(&font_mapper)); }
diff --git a/core/fxge/fx_ge_linux.cpp b/core/fxge/fx_ge_linux.cpp index 72cb263..d568813 100644 --- a/core/fxge/fx_ge_linux.cpp +++ b/core/fxge/fx_ge_linux.cpp
@@ -155,27 +155,22 @@ } // namespace -std::unique_ptr<SystemFontInfoIface> SystemFontInfoIface::CreateDefault( - const char** pUserPaths) { - auto pInfo = std::make_unique<CFX_LinuxFontInfo>(); - if (!pInfo->ParseFontCfg(pUserPaths)) { - pInfo->AddPath("/usr/share/fonts"); - pInfo->AddPath("/usr/share/X11/fonts/Type1"); - pInfo->AddPath("/usr/share/X11/fonts/TTF"); - pInfo->AddPath("/usr/local/share/fonts"); - } - return std::move(pInfo); -} - class CLinuxPlatform : public CFX_GEModule::PlatformIface { public: CLinuxPlatform() = default; ~CLinuxPlatform() override = default; - void Init() override { - CFX_GEModule* pModule = CFX_GEModule::Get(); - pModule->GetFontMgr()->SetSystemFontInfo( - SystemFontInfoIface::CreateDefault(pModule->GetUserFontPaths())); + void Init() override {} + + std::unique_ptr<SystemFontInfoIface> CreateDefaultSystemFontInfo() override { + auto pInfo = std::make_unique<CFX_LinuxFontInfo>(); + if (!pInfo->ParseFontCfg(CFX_GEModule::Get()->GetUserFontPaths())) { + pInfo->AddPath("/usr/share/fonts"); + pInfo->AddPath("/usr/share/X11/fonts/Type1"); + pInfo->AddPath("/usr/share/X11/fonts/TTF"); + pInfo->AddPath("/usr/local/share/fonts"); + } + return pInfo; } };
diff --git a/core/fxge/systemfontinfo_iface.h b/core/fxge/systemfontinfo_iface.h index 61704be..08a8c94 100644 --- a/core/fxge/systemfontinfo_iface.h +++ b/core/fxge/systemfontinfo_iface.h
@@ -17,9 +17,6 @@ class SystemFontInfoIface { public: - static std::unique_ptr<SystemFontInfoIface> CreateDefault( - const char** pUserPaths); - virtual ~SystemFontInfoIface() = default; virtual bool EnumFontList(CFX_FontMapper* pMapper) = 0;
diff --git a/core/fxge/win32/fx_win32_device.cpp b/core/fxge/win32/fx_win32_device.cpp index 1f405bd..8a8fee1 100644 --- a/core/fxge/win32/fx_win32_device.cpp +++ b/core/fxge/win32/fx_win32_device.cpp
@@ -677,8 +677,21 @@ WindowsPrintMode g_pdfium_print_mode = WindowsPrintMode::kModeEmf; -std::unique_ptr<SystemFontInfoIface> SystemFontInfoIface::CreateDefault( - const char** pUnused) { +CWin32Platform::CWin32Platform() = default; + +CWin32Platform::~CWin32Platform() = default; + +void CWin32Platform::Init() { + OSVERSIONINFO ver; + ver.dwOSVersionInfoSize = sizeof(ver); + GetVersionEx(&ver); + m_bHalfTone = ver.dwMajorVersion >= 5; + if (pdfium::base::win::IsUser32AndGdi32Available()) + m_GdiplusExt.Load(); +} + +std::unique_ptr<SystemFontInfoIface> +CWin32Platform::CreateDefaultSystemFontInfo() { if (pdfium::base::win::IsUser32AndGdi32Available()) return std::unique_ptr<SystemFontInfoIface>(new CFX_Win32FontInfo); @@ -696,21 +709,6 @@ return std::unique_ptr<SystemFontInfoIface>(pInfoFallback); } -CWin32Platform::CWin32Platform() = default; - -CWin32Platform::~CWin32Platform() = default; - -void CWin32Platform::Init() { - OSVERSIONINFO ver; - ver.dwOSVersionInfoSize = sizeof(ver); - GetVersionEx(&ver); - m_bHalfTone = ver.dwMajorVersion >= 5; - if (pdfium::base::win::IsUser32AndGdi32Available()) - m_GdiplusExt.Load(); - CFX_GEModule::Get()->GetFontMgr()->SetSystemFontInfo( - SystemFontInfoIface::CreateDefault(nullptr)); -} - // static std::unique_ptr<CFX_GEModule::PlatformIface> CFX_GEModule::PlatformIface::Create() {
diff --git a/core/fxge/win32/win32_int.h b/core/fxge/win32/win32_int.h index d28d6e2..a2bffea 100644 --- a/core/fxge/win32/win32_int.h +++ b/core/fxge/win32/win32_int.h
@@ -68,6 +68,7 @@ // CFX_GEModule::PlatformIface: void Init() override; + std::unique_ptr<SystemFontInfoIface> CreateDefaultSystemFontInfo() override; bool m_bHalfTone = false; CGdiplusExt m_GdiplusExt;
diff --git a/fpdfsdk/fpdf_sysfontinfo.cpp b/fpdfsdk/fpdf_sysfontinfo.cpp index 0b5c8e6..74dc3dd 100644 --- a/fpdfsdk/fpdf_sysfontinfo.cpp +++ b/fpdfsdk/fpdf_sysfontinfo.cpp
@@ -211,7 +211,7 @@ FPDF_EXPORT FPDF_SYSFONTINFO* FPDF_CALLCONV FPDF_GetDefaultSystemFontInfo() { std::unique_ptr<SystemFontInfoIface> pFontInfo = - SystemFontInfoIface::CreateDefault(nullptr); + CFX_GEModule::Get()->GetPlatform()->CreateDefaultSystemFontInfo(); if (!pFontInfo) return nullptr;
diff --git a/testing/xfa_test_environment.cpp b/testing/xfa_test_environment.cpp index 797341a..0fdab8d 100644 --- a/testing/xfa_test_environment.cpp +++ b/testing/xfa_test_environment.cpp
@@ -30,10 +30,10 @@ } void XFATestEnvironment::SetUp() { - // TODO(dsinclair): This font loading is slow. We should make a test font - // loader which loads up a single font we use in all tests. + // This font loading is slow, but we do it only once per binary + // execution, not once per test. CFX_GEModule::Get()->GetFontMgr()->SetSystemFontInfo( - SystemFontInfoIface::CreateDefault(nullptr)); + CFX_GEModule::Get()->GetPlatform()->CreateDefaultSystemFontInfo()); auto font_mgr = std::make_unique<CFGAS_FontMgr>(); if (font_mgr->EnumFonts())