Add FPDF_SYSFONTINFO version 2 for per-request font matching This change adds support for version 2 of the FPDF_SYSFONTINFO interface, which enables per-request font matching instead of upfront font enumeration. Bug: 449573621 Change-Id: I794d9d092219ce8ee7a94f977a9be69415c98d7d Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/136690 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Nicolás Peña <npm@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxge/cfx_fontmapper.cpp b/core/fxge/cfx_fontmapper.cpp index c35c6d4..cf4e160 100644 --- a/core/fxge/cfx_fontmapper.cpp +++ b/core/fxge/cfx_fontmapper.cpp
@@ -513,7 +513,9 @@ return; } - font_info_->EnumFontList(this); + if (!skip_font_enumeration_) { + font_info_->EnumFontList(this); + } list_loaded_ = true; } @@ -730,7 +732,13 @@ if (!is_cjk) { if (!CheckSupportThirdPartFont(family, &pitch_family)) { is_italic = italic_angle != 0; - weight = old_weight; + // When `skip_font_enumeration_` is true (version 2), CFX_FontMapper + // relies on `MapFont()` to handle font matching with the correct + // weight. Don't reset weight here as it would break bold/light font + // variants. + if (!skip_font_enumeration_) { + weight = old_weight; + } } if (IsNarrowFontName(subst_name)) { family = kNarrowFamily;
diff --git a/core/fxge/cfx_fontmapper.h b/core/fxge/cfx_fontmapper.h index 38ccf1d..61028d6 100644 --- a/core/fxge/cfx_fontmapper.h +++ b/core/fxge/cfx_fontmapper.h
@@ -63,6 +63,7 @@ void SetSystemFontInfo(std::unique_ptr<SystemFontInfoIface> font_info); std::unique_ptr<SystemFontInfoIface> TakeSystemFontInfo(); + void SetSkipFontEnumeration(bool skip) { skip_font_enumeration_ = skip; } void AddInstalledFont(const ByteString& name, FX_Charset charset); void LoadInstalledFonts(); @@ -125,6 +126,7 @@ }; bool list_loaded_ = false; + bool skip_font_enumeration_ = false; ByteString last_family_; std::vector<FaceData> face_array_; std::unique_ptr<SystemFontInfoIface> font_info_;
diff --git a/core/fxge/cfx_fontmapper_unittest.cpp b/core/fxge/cfx_fontmapper_unittest.cpp index b29cdea..2b58087 100644 --- a/core/fxge/cfx_fontmapper_unittest.cpp +++ b/core/fxge/cfx_fontmapper_unittest.cpp
@@ -267,3 +267,42 @@ &subst_font)); EXPECT_EQ("Noto Sans SC Regular", subst_font.family_); } + +TEST(CFXFontMapperTest, LoadInstalledFontsWithEnumeration) { + CFX_FontMapper font_mapper(nullptr); + auto system_font_info = std::make_unique<MockSystemFontInfo>(); + auto* mock_font_info = system_font_info.get(); + font_mapper.SetSystemFontInfo(std::move(system_font_info)); + + // Default behavior: EnumFontList should be called + EXPECT_CALL(*mock_font_info, EnumFontList(&font_mapper)).Times(1); + font_mapper.LoadInstalledFonts(); +} + +TEST(CFXFontMapperTest, LoadInstalledFontsSkipEnumeration) { + CFX_FontMapper font_mapper(nullptr); + auto system_font_info = std::make_unique<MockSystemFontInfo>(); + auto* mock_font_info = system_font_info.get(); + font_mapper.SetSystemFontInfo(std::move(system_font_info)); + + // Enable skip enumeration + font_mapper.SetSkipFontEnumeration(true); + + // EnumFontList should NOT be called when skip_enumeration is enabled + EXPECT_CALL(*mock_font_info, EnumFontList(_)).Times(0); + font_mapper.LoadInstalledFonts(); +} + +TEST(CFXFontMapperTest, LoadInstalledFontsCalledOnlyOnce) { + CFX_FontMapper font_mapper(nullptr); + auto system_font_info = std::make_unique<MockSystemFontInfo>(); + auto* mock_font_info = system_font_info.get(); + font_mapper.SetSystemFontInfo(std::move(system_font_info)); + + // EnumFontList should be called only once even if LoadInstalledFonts is + // called multiple times + EXPECT_CALL(*mock_font_info, EnumFontList(&font_mapper)).Times(1); + font_mapper.LoadInstalledFonts(); + font_mapper.LoadInstalledFonts(); + font_mapper.LoadInstalledFonts(); +}
diff --git a/fpdfsdk/fpdf_sysfontinfo.cpp b/fpdfsdk/fpdf_sysfontinfo.cpp index 51edf69..542608f 100644 --- a/fpdfsdk/fpdf_sysfontinfo.cpp +++ b/fpdfsdk/fpdf_sysfontinfo.cpp
@@ -161,13 +161,17 @@ return; } - if (font_infoExt->version != 1) { + if (font_infoExt->version != 1 && font_infoExt->version != 2) { return; } mapper->SetSystemFontInfo( std::make_unique<CFX_ExternalFontInfo>(font_infoExt)); + // Version 1: Enable font enumeration via `EnumFonts()`. + // Version 2: Skip font enumeration, use per-request matching via `MapFont()`. + mapper->SetSkipFontEnumeration(font_infoExt->version == 2); + #ifdef PDF_ENABLE_XFA CFGAS_GEModule::Get()->GetFontMgr()->EnumFonts(); #endif
diff --git a/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp b/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp index 10007d9..8d79455 100644 --- a/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp +++ b/fpdfsdk/fpdf_sysfontinfo_embeddertest.cpp
@@ -16,47 +16,138 @@ namespace { +struct TrackingFontInfo { + FPDF_SYSFONTINFO base; + FPDF_SYSFONTINFO* wrapped_font_info; + int* enum_fonts_call_count; + int* map_font_call_count; +}; + extern "C" { -void FakeRelease(FPDF_SYSFONTINFO* pThis) {} -void FakeEnumFonts(FPDF_SYSFONTINFO* pThis, void* pMapper) {} +void TrackingEnumFonts(FPDF_SYSFONTINFO* sys_font_info, void* mapper) { + auto* tracking_info = reinterpret_cast<TrackingFontInfo*>(sys_font_info); + (*tracking_info->enum_fonts_call_count)++; + if (tracking_info->wrapped_font_info && + tracking_info->wrapped_font_info->EnumFonts) { + tracking_info->wrapped_font_info->EnumFonts( + tracking_info->wrapped_font_info, mapper); + } +} -void* FakeMapFont(FPDF_SYSFONTINFO* pThis, +void* TrackingMapFont(FPDF_SYSFONTINFO* sys_font_info, + int weight, + FPDF_BOOL italic, + int charset, + int pitch_family, + const char* face, + FPDF_BOOL* exact) { + auto* tracking_info = reinterpret_cast<TrackingFontInfo*>(sys_font_info); + (*tracking_info->map_font_call_count)++; + if (tracking_info->wrapped_font_info && + tracking_info->wrapped_font_info->MapFont) { + return tracking_info->wrapped_font_info->MapFont( + tracking_info->wrapped_font_info, weight, italic, charset, pitch_family, + face, exact); + } + return nullptr; +} + +void* TrackingGetFont(FPDF_SYSFONTINFO* sys_font_info, const char* face) { + auto* tracking_info = reinterpret_cast<TrackingFontInfo*>(sys_font_info); + if (tracking_info->wrapped_font_info && + tracking_info->wrapped_font_info->GetFont) { + return tracking_info->wrapped_font_info->GetFont( + tracking_info->wrapped_font_info, face); + } + return nullptr; +} + +unsigned long TrackingGetFontData(FPDF_SYSFONTINFO* sys_font_info, + void* font, + unsigned int table, + unsigned char* buffer, + unsigned long buf_size) { + auto* tracking_info = reinterpret_cast<TrackingFontInfo*>(sys_font_info); + if (tracking_info->wrapped_font_info && + tracking_info->wrapped_font_info->GetFontData) { + return tracking_info->wrapped_font_info->GetFontData( + tracking_info->wrapped_font_info, font, table, buffer, buf_size); + } + return 0; +} + +unsigned long TrackingGetFaceName(FPDF_SYSFONTINFO* sys_font_info, + void* font, + char* buffer, + unsigned long buf_size) { + auto* tracking_info = reinterpret_cast<TrackingFontInfo*>(sys_font_info); + if (tracking_info->wrapped_font_info && + tracking_info->wrapped_font_info->GetFaceName) { + return tracking_info->wrapped_font_info->GetFaceName( + tracking_info->wrapped_font_info, font, buffer, buf_size); + } + return 0; +} + +int TrackingGetFontCharset(FPDF_SYSFONTINFO* sys_font_info, void* font) { + auto* tracking_info = reinterpret_cast<TrackingFontInfo*>(sys_font_info); + if (tracking_info->wrapped_font_info && + tracking_info->wrapped_font_info->GetFontCharset) { + return tracking_info->wrapped_font_info->GetFontCharset( + tracking_info->wrapped_font_info, font); + } + return 0; +} + +void TrackingDeleteFont(FPDF_SYSFONTINFO* sys_font_info, void* font) { + auto* tracking_info = reinterpret_cast<TrackingFontInfo*>(sys_font_info); + if (tracking_info->wrapped_font_info && + tracking_info->wrapped_font_info->DeleteFont) { + tracking_info->wrapped_font_info->DeleteFont( + tracking_info->wrapped_font_info, font); + } +} + +void FakeRelease(FPDF_SYSFONTINFO* sys_font_info) {} +void FakeEnumFonts(FPDF_SYSFONTINFO* sys_font_info, void* mapper) {} + +void* FakeMapFont(FPDF_SYSFONTINFO* sys_font_info, int weight, - FPDF_BOOL bItalic, + FPDF_BOOL italic, int charset, int pitch_family, const char* face, - FPDF_BOOL* bExact) { + FPDF_BOOL* exact) { // Any non-null return will do. - return pThis; + return sys_font_info; } -void* FakeGetFont(FPDF_SYSFONTINFO* pThis, const char* face) { +void* FakeGetFont(FPDF_SYSFONTINFO* sys_font_info, const char* face) { // Any non-null return will do. - return pThis; + return sys_font_info; } -unsigned long FakeGetFontData(FPDF_SYSFONTINFO* pThis, - void* hFont, +unsigned long FakeGetFontData(FPDF_SYSFONTINFO* sys_font_info, + void* font, unsigned int table, unsigned char* buffer, unsigned long buf_size) { return 0; } -unsigned long FakeGetFaceName(FPDF_SYSFONTINFO* pThis, - void* hFont, +unsigned long FakeGetFaceName(FPDF_SYSFONTINFO* sys_font_info, + void* font, char* buffer, unsigned long buf_size) { return 0; } -int FakeGetFontCharset(FPDF_SYSFONTINFO* pThis, void* hFont) { +int FakeGetFontCharset(FPDF_SYSFONTINFO* sys_font_info, void* font) { return 1; } -void FakeDeleteFont(FPDF_SYSFONTINFO* pThis, void* hFont) {} +void FakeDeleteFont(FPDF_SYSFONTINFO* sys_font_info, void* font) {} } // extern "C" @@ -121,8 +212,91 @@ FPDF_SYSFONTINFO* font_info_; }; +class FPDFVersion1Versus2ComparisonTest : public EmbedderTest { + public: + FPDFVersion1Versus2ComparisonTest() = default; + ~FPDFVersion1Versus2ComparisonTest() override = default; + + void SetupVersion(int version) { + EmbedderTestEnvironment::GetInstance()->TearDown(); + EmbedderTestEnvironment::GetInstance()->SetUp(); + + enum_fonts_call_count_ = 0; + map_font_call_count_ = 0; + + wrapped_font_info_ = FPDF_GetDefaultSystemFontInfo(); + ASSERT_TRUE(wrapped_font_info_); + + tracking_font_info_.base.version = version; + tracking_font_info_.base.Release = nullptr; + tracking_font_info_.base.EnumFonts = TrackingEnumFonts; + tracking_font_info_.base.MapFont = TrackingMapFont; + tracking_font_info_.base.GetFont = TrackingGetFont; + tracking_font_info_.base.GetFontData = TrackingGetFontData; + tracking_font_info_.base.GetFaceName = TrackingGetFaceName; + tracking_font_info_.base.GetFontCharset = TrackingGetFontCharset; + tracking_font_info_.base.DeleteFont = TrackingDeleteFont; + tracking_font_info_.wrapped_font_info = wrapped_font_info_; + tracking_font_info_.enum_fonts_call_count = &enum_fonts_call_count_; + tracking_font_info_.map_font_call_count = &map_font_call_count_; + + FPDF_SetSystemFontInfo(&tracking_font_info_.base); + } + + void TearDown() override { + if (wrapped_font_info_) { + FPDF_SetSystemFontInfo(nullptr); + if (wrapped_font_info_->Release) { + wrapped_font_info_->Release(wrapped_font_info_); + } + FPDF_FreeDefaultSystemFontInfo(wrapped_font_info_); + wrapped_font_info_ = nullptr; + } + EmbedderTest::TearDown(); + + // Bouncing the library is the only reliable way to fully undo the initial + // FPDF_SetSystemFontInfo() call at the moment. + EmbedderTestEnvironment::GetInstance()->TearDown(); + EmbedderTestEnvironment::GetInstance()->SetUp(); + } + + int enum_fonts_call_count_ = 0; + int map_font_call_count_ = 0; + FPDF_SYSFONTINFO* wrapped_font_info_ = nullptr; + TrackingFontInfo tracking_font_info_ = {}; +}; + } // namespace +TEST_F(FPDFVersion1Versus2ComparisonTest, Version1CallsEnumFonts) { + SetupVersion(1); + + ASSERT_TRUE(OpenDocument("hello_world.pdf")); + FPDF_PAGE page = LoadPage(0); + ASSERT_TRUE(page); + ScopedFPDFBitmap bitmap = RenderPage(page); + ASSERT_TRUE(bitmap); + UnloadPage(page); + + // Version 1 should call EnumFonts at least once. + EXPECT_GT(enum_fonts_call_count_, 0); +} + +TEST_F(FPDFVersion1Versus2ComparisonTest, Version2SkipsEnumFonts) { + SetupVersion(2); + + ASSERT_TRUE(OpenDocument("hello_world.pdf")); + FPDF_PAGE page = LoadPage(0); + ASSERT_TRUE(page); + ScopedFPDFBitmap bitmap = RenderPage(page); + ASSERT_TRUE(bitmap); + UnloadPage(page); + + // Version 2 should NOT call EnumFonts but should call MapFont. + EXPECT_EQ(enum_fonts_call_count_, 0); + EXPECT_GT(map_font_call_count_, 0); +} + TEST_F(FPDFUnavailableSysFontInfoEmbedderTest, Bug972518) { ASSERT_TRUE(OpenDocument("bug_972518.pdf")); ASSERT_EQ(1, FPDF_GetPageCount(document()));
diff --git a/public/fpdf_sysfontinfo.h b/public/fpdf_sysfontinfo.h index f3bc544..07bec0d 100644 --- a/public/fpdf_sysfontinfo.h +++ b/public/fpdf_sysfontinfo.h
@@ -46,14 +46,17 @@ // Interface: FPDF_SYSFONTINFO // Interface for getting system font information and font mapping typedef struct _FPDF_SYSFONTINFO { - // Version number of the interface. Currently must be 1. + // Version number of the interface. Currently must be 1 or 2. + // Version 1: Traditional behavior - calls EnumFonts during initialization. + // Version 2: Per-request behavior - skips EnumFonts, relies on MapFont. + // Experimental: Subject to change based on feedback. int version; // Method: Release // Give implementation a chance to release any data after the // interface is no longer used. // Interface Version: - // 1 + // 1 and 2 // Implementation Required: // No // Parameters: @@ -80,13 +83,15 @@ // Implementations should call FPDF_AddInstalledFont() function for // each font found. Only TrueType/OpenType and Type1 fonts are // accepted by PDFium. + // NOTE: This method will not be called when version is set to 2. + // Version 2 relies entirely on MapFont() for per-request matching. void (*EnumFonts)(struct _FPDF_SYSFONTINFO* pThis, void* pMapper); // Method: MapFont // Use the system font mapper to get a font handle from requested // parameters. // Interface Version: - // 1 + // 1 and 2 // Implementation Required: // Required if GetFont method is not implemented. // Parameters: @@ -122,7 +127,7 @@ // Method: GetFont // Get a handle to a particular font by its internal ID // Interface Version: - // 1 + // 1 and 2 // Implementation Required: // Required if MapFont method is not implemented. // Return Value: @@ -138,7 +143,7 @@ // Method: GetFontData // Get font data from a font // Interface Version: - // 1 + // 1 and 2 // Implementation Required: // Yes // Parameters: @@ -164,7 +169,7 @@ // Method: GetFaceName // Get face name from a font handle // Interface Version: - // 1 + // 1 and 2 // Implementation Required: // No // Parameters: @@ -184,7 +189,7 @@ // Method: GetFontCharset // Get character set information for a font handle // Interface Version: - // 1 + // 1 and 2 // Implementation Required: // No // Parameters: @@ -197,7 +202,7 @@ // Method: DeleteFont // Delete a font handle // Interface Version: - // 1 + // 1 and 2 // Implementation Required: // Yes // Parameters: