Refactor CFX_FontMgr and CFX_FontMapper to break circular dependency [pdfium : main]
0 views
Skip to first unread message
Tom Sepez (Gerrit)
Jun 24, 2026, 3:33:22 PM (11 hours ago) Jun 24
to Lei Zhang, pdfium-...@googlegroups.com
Attention needed from Lei Zhang
Tom Sepez added 4 comments![]( "Open in Gerrit")
File core/fxge/cfx_fontmapper.h
Line 20, Patchset 4:
#include "core/fxcrt/cfx_read_only_container_stream.h"Tom Sepez . resolved
Since `FixedSizeDataVector` is used by value in `AddFontCacheEntry` and `AddTTCFontCacheEntry`, and we no longer include `cfx_fontmgr.h` which previously provided it transitively, we should explicitly include `"core/fxcrt/fixed_size_data_vector.h"` here.
File core/fxge/cfx_fontmapper.cpp
Line 25, Patchset 4:
#include "core/fxge/cfx_gemodule.h"Tom Sepez . resolved
Since `CFX_GEModule::Get()->GetFontMgr()` was removed, you can likely remove this include as well.
File core/fxge/cfx_fontmgr.h
Line 16, Patchset 4:
#include <tuple>Tom Sepez . resolved
With `FontCacheEntry` and its maps moved, you can remove several unused includes from this file: `<array>`, `<tuple>`, `"core/fxcrt/bytestring.h"`, `"core/fxcrt/cfx_read_only_container_stream.h"`, and `"core/fxcrt/fixed_size_data_vector.h"`.
File core/fxge/cfx_fontmgr.cpp
Line 9, Patchset 4:
#include <array>Tom Sepez . resolved
With `kFoxitFonts` and `FontCacheEntry` moved, several includes appear to be unused now: `<array>`, `<iterator>`, `<utility>`, `"core/fxcrt/check_op.h"`, `"core/fxcrt/fixed_size_data_vector.h"`, and `"core/fxge/cfx_standardfont.h"`.
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. |
Lei Zhang (Gerrit)
Jun 24, 2026, 4:19:53 PM (10 hours ago) Jun 24
to Tom Sepez, Lei Zhang, pdfium-...@googlegroups.com
Attention needed from Tom Sepez
Lei Zhang voted and added 1 comment![]( "Open in Gerrit")
Votes added by Lei Zhang
| Code-Review | +1 |
1 comment
File core/fxge/cfx_fontmapper_unittest.cpp
Line 85, Patchset 6 (Parent):
Lei Zhang . resolved
Uh, sure, I guess cfx_fontmapper_unittest.cpp can get some participation points.
Related details
Attention is currently required from:
- Tom Sepez
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Jun 24, 2026, 5:13:27 PM (9 hours ago) Jun 24
to Tom Sepez, Lei Zhang, pdfium-...@googlegroups.com
Attention needed from Tom Sepez
Lei Zhang added 1 comment![]( "Open in Gerrit")
File core/fxge/cfx_fontmgr.h
Line 59, Patchset 6 (Latest):
// Must come before |builtin_mapper_| and |face_map_|.Lei Zhang . unresolved
This line should be updated in this CL and not the next CL.
Related details
Attention is currently required from:
- Tom Sepez
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Jun 24, 2026, 5:17:16 PM (9 hours ago) Jun 24
to Tom Sepez, Lei Zhang, pdfium-...@googlegroups.com
Attention needed from Tom Sepez
Lei Zhang added 1 comment![]( "Open in Gerrit")
File core/fxge/cfx_fontmgr.h
Line 59, Patchset 6 (Latest):
// Must come before |builtin_mapper_| and |face_map_|.Lei Zhang . unresolved
This line should be updated in this CL and not the next CL.
Lei Zhang
Might as well switch to backticks.
Related details
Attention is currently required from:
- Tom Sepez
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Jun 24, 2026, 5:20:28 PM (9 hours ago) Jun 24
to Tom Sepez, Lei Zhang, pdfium-...@googlegroups.com
Attention needed from Tom Sepez
Lei Zhang added 1 comment![]( "Open in Gerrit")
File core/fxge/cfx_fontmapper.cpp
Line 32, Patchset 6 (Latest):
constexpr std::array<pdfium::span<const uint8_t>,Lei Zhang . unresolved
Can this move straight into cfx_standardfont.cpp, instead of moving it here and immediately moving it again?
Related details
Attention is currently required from:
- Tom Sepez
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Sepez (Gerrit)
Jun 24, 2026, 6:08:53 PM (8 hours ago) Jun 24
to Lei Zhang, pdfium-...@googlegroups.com
Tom Sepez added 1 comment![]( "Open in Gerrit")
File core/fxge/cfx_fontmapper.cpp
Line 32, Patchset 6 (Latest):
constexpr std::array<pdfium::span<const uint8_t>,Lei Zhang . resolved
Can this move straight into cfx_standardfont.cpp, instead of moving it here and immediately moving it again?
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Sepez (Gerrit)
Jun 24, 2026, 6:22:48 PM (8 hours ago) Jun 24
to Lei Zhang, pdfium-...@googlegroups.com
Tom Sepez added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File core/fxge/cfx_fontmgr.h
Lei ZhangThis line should be updated in this CL and not the next CL.
Might as well switch to backticks.
Tom Sepez
Done
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Jun 24, 2026, 6:31:10 PM (8 hours ago) Jun 24
to Tom Sepez, Lei Zhang, pdfium-...@googlegroups.com
Attention needed from Tom Sepez
Lei Zhang voted and added 1 comment![]( "Open in Gerrit")
Votes added by Lei Zhang
| Code-Review | +1 |
Attention is currently required from:
- Tom Sepez
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Sepez (Gerrit)
Jun 24, 2026, 6:38:38 PM (8 hours ago) Jun 24
to Lei Zhang, pdfium-...@googlegroups.com
Tom Sepez added 1 comment![]( "Open in Gerrit")
File core/fxge/cfx_fontmapper.cpp
Lei Zhang . resolved
Was hard to see in the grandparent CL. OK.
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Sepez (Gerrit)
Jun 24, 2026, 7:20:00 PM (7 hours ago) Jun 24
to pdfium...@luci-project-accounts.iam.gserviceaccount.com, Lei Zhang, pdfium-...@googlegroups.com
Tom Sepez voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
Jun 24, 2026, 7:42:59 PM (7 hours ago) Jun 24
to Tom Sepez, Lei Zhang, pdfium-...@googlegroups.com
pdfium...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Unreviewed changes
9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
Refactor CFX_FontMgr and CFX_FontMapper to break circular dependency
Move font cache maps (face_map_, ttc_face_map_) and FontCacheEntry
from CFX_FontMgr to CFX_FontMapper, as they are exclusively used by
the mapper.
TAG=agy
CONV=a7eb1d9a-c49f-4737-82a1-59c7ae6d9d52
Change-Id: Ib93cb3d2795d8676930d97ef684879cf9db03fd9
Commit-Queue: Tom Sepez <tse...@chromium.org>
Reviewed-by: Lei Zhang <the...@chromium.org>
Files:
- M core/fxge/cfx_fontmapper.cpp
- M core/fxge/cfx_fontmapper.h
- M core/fxge/cfx_fontmapper_unittest.cpp
- M core/fxge/cfx_fontmgr.cpp
- M core/fxge/cfx_fontmgr.h
Change size: M
Delta: 5 files changed, 106 insertions(+), 121 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Lei Zhang
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages