Share manual font table parsing in CFX_FolderFontInfo [pdfium : main]

0 views

Skip to first unread message

Tom Sepez (Gerrit)

unread,

Jun 2, 2026, 6:41:41 PM (2 days ago) Jun 2

to Lei Zhang, pdfium...@luci-project-accounts.iam.gserviceaccount.com, pdfium-...@googlegroups.com

Attention needed from Lei Zhang

Tom Sepez added 5 comments

Patchset-level comments

File-level comment, Patchset 14:

Lei Zhang . resolved

How about prepending a CL to add FindFontTable() first? There's a lot of moving pieces in this CL and it's a bit hard to keep track of it all.

Tom Sepez

Done

Commit Message

Line 12, Patchset 13:on FreeType. Following this CL, we can re-shape the fxge code to operate

Lei Zhang . resolved

go/avoid-we

Tom Sepez

Done

File core/fxge/android/cfpf_skiafont.cpp

Line 37, Patchset 13: return pdfium::checked_cast<uint32_t>(table_data.size());

Lei Zhang . resolved

Flip the conditional on line 33 and combine this line with line 34?

Tom Sepez

Done

File core/fxge/cfx_folderfontinfo.cpp

Line 193, Patchset 13: uint32_t name_tag = CFX_FontMapper::MakeTag('n', 'a', 'm', 'e');

Lei Zhang . resolved

1) Why make this non-const?
2) Actually, this one can be `SystemFontInfoIface::kTableNAME`.

Tom Sepez

Done

File core/fxge/fx_font_unittest.cpp

Line 94, Patchset 13: EXPECT_EQ(10u, head_table.size());

Lei Zhang . resolved

ASSERT_EQ() here and line 101.

Tom Sepez

Done

Open in Gerrit

Related details

Attention is currently required from:

Lei Zhang

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages