chromium/src
0
Fork 0
Code Activity

Make PDF font proxy default and remove feature

Browse Source 
PDFs now get non-embedded fonts via a skia/mojo proxy
rather than using "direct" access via sandbox hooks &
sandbox IPCs.

This CL removes the WinPdfUseFontProxy feature and
associated plumbing content code for allowing access to the
local fonts dir.

Bug: 344643689
Change-Id: Idbb5a9ef2e6c24a4f1c08c6505437c9ca1696096
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5805910
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1351684}
This commit is contained in:
Alex Gough
2024-09-05 20:53:12 +00:00
committed by Chromium LUCI CQ
parent 3b1fe031e1
commit 1fb6027884
17 changed files with 2 additions and 96 deletions

6
chrome/browser/about_flags.cc

@ -7212,12 +7212,6 @@ const FeatureEntry kFeatureEntries[] = {
FEATURE_VALUE_TYPE(chrome_pdf::features::kPdfInk2)},
#endif // BUILDFLAG(ENABLE_PDF_INK2)
#if BUILDFLAG(IS_WIN)
{"pdf-win-use-font-proxy", flag_descriptions::kWinPdfUseFontProxyName,
flag_descriptions::kWinPdfUseFontProxyDescription, kOsWin,
FEATURE_VALUE_TYPE(chrome_pdf::features::kWinPdfUseFontProxy)},
#endif // BUILDFLAG(IS_WIN)
#endif // BUILDFLAG(ENABLE_PDF)
#if BUILDFLAG(ENABLE_PRINTING)

12
chrome/browser/chrome_content_browser_client.cc

@ -712,9 +712,6 @@
#include "components/pdf/browser/pdf_navigation_throttle.h"
#include "components/pdf/browser/pdf_url_loader_request_interceptor.h"
#include "components/pdf/common/constants.h"
#if BUILDFLAG(IS_WIN)
#include "pdf/pdf_features.h"
#endif // BUILDFLAG(IS_WIN)
#endif // BUILDFLAG(ENABLE_PDF)
@ -5232,15 +5229,6 @@ bool ChromeContentBrowserClient::IsRendererCodeIntegrityEnabled() {
local_state->GetBoolean(prefs::kRendererCodeIntegrityEnabled);
}
bool ChromeContentBrowserClient::IsPdfFontProxyEnabled() {
#if BUILDFLAG(ENABLE_PDF)
return base::FeatureList::IsEnabled(
chrome_pdf::features::kWinPdfUseFontProxy);
#else
return false;
#endif
}
// Note: Only use sparingly to add Chrome specific sandbox functionality here.
// Other code should reside in the content layer. Changes to this function
// should be reviewed by the security team.

1
chrome/browser/chrome_content_browser_client.h

@ -550,7 +550,6 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
std::wstring GetLPACCapabilityNameForNetworkService() override;
bool IsUtilityCetCompatible(const std::string& utility_sub_type) override;
bool IsRendererCodeIntegrityEnabled() override;
bool IsPdfFontProxyEnabled() override;
void SessionEnding(std::optional<DWORD> control_type) override;
bool ShouldEnableAudioProcessHighPriority() override;
bool ShouldUseSkiaFontManager(const GURL& site_url) override;

5
chrome/browser/flag-metadata.json

@ -6987,11 +6987,6 @@
"owners": [ "thestig@chromium.org", "//pdf/OWNERS" ],
"expiry_milestone": 133
},
{
"name": "pdf-win-use-font-proxy",
"owners": [ "ajgo@chromium.org", "//pdf/OWNERS" ],
"expiry_milestone": 130
},
{
"name": "pdf-xfa-forms",
"owners": [ "thestig@chromium.org", "//pdf/OWNERS" ],

6
chrome/browser/flag_descriptions.cc

@ -8220,12 +8220,6 @@ const char kPdfInk2Description[] =
"Enables the ability to annotate PDFs using a new ink library.";
#endif // BUILDFLAG(ENABLE_PDF_INK2)
#if BUILDFLAG(IS_WIN)
const char kWinPdfUseFontProxyName[] = "PDF Font Proxy";
const char kWinPdfUseFontProxyDescription[] =
"Use Skia/IPC to lookup fallback fonts on Windows.";
#endif // BUILDFLAG(IS_WIN)
const char kPdfOopifName[] = "OOPIF for PDF Viewer";
const char kPdfOopifDescription[] =
"Use an OOPIF for the PDF Viewer, instead of a GuestView.";

5
chrome/browser/flag_descriptions.h

@ -4784,11 +4784,6 @@ extern const char kPdfInk2Name[];
extern const char kPdfInk2Description[];
#endif // BUILDFLAG(ENABLE_PDF_INK2)
#if BUILDFLAG(IS_WIN)
extern const char kWinPdfUseFontProxyName[];
extern const char kWinPdfUseFontProxyDescription[];
#endif // BUILDFLAG(IS_WIN)
extern const char kPdfOopifName[];
extern const char kPdfOopifDescription[];

5
content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.cc

@ -90,11 +90,6 @@ RendererSandboxedProcessLauncherDelegateWin::
}
}
bool RendererSandboxedProcessLauncherDelegateWin::AllowWindowsFontsDir() {
return is_pdf_renderer_ &&
!GetContentClient()->browser()->IsPdfFontProxyEnabled();
}
std::string RendererSandboxedProcessLauncherDelegateWin::GetSandboxTag() {
if (is_pdf_renderer_) {
// All pdf renderers are jitless so only need one tag for these.

1
content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.h

@ -45,7 +45,6 @@ class CONTENT_EXPORT RendererSandboxedProcessLauncherDelegateWin
bool InitializeConfig(sandbox::TargetConfig* config) override;
void PostSpawnTarget(base::ProcessHandle process) override;
bool CetCompatible() override;
bool AllowWindowsFontsDir() override;
// SandboxedProcessLauncherDelegate:
bool ShouldUseUntrustedMojoInvitation() override;

1
content/browser/utility_sandbox_delegate.h

@ -43,7 +43,6 @@ class CONTENT_EXPORT UtilitySandboxedProcessLauncherDelegate
bool InitializeConfig(sandbox::TargetConfig* config) override;
bool ShouldUnsandboxedRunInJob() override;
bool CetCompatible() override;
bool AllowWindowsFontsDir() override;
bool PreSpawnTarget(sandbox::TargetPolicy* policy) override;
// Set preload libraries to transfer as part of the sandbox delegate data,
// which will used in utility_main to preload these libraries before lockdown.

9
content/browser/utility_sandbox_delegate_win.cc

@ -437,15 +437,6 @@ bool UtilitySandboxedProcessLauncherDelegate::CetCompatible() {
utility_sub_type);
}
bool UtilitySandboxedProcessLauncherDelegate::AllowWindowsFontsDir() {
// New utilities should use a font proxy rather than allowing direct access.
if (sandbox_type_ == sandbox::mojom::Sandbox::kPrintCompositor &&
!GetContentClient()->browser()->IsPdfFontProxyEnabled()) {
return true;
}
return false;
}
bool UtilitySandboxedProcessLauncherDelegate::PreSpawnTarget(
sandbox::TargetPolicy* policy) {
AddDelegateData(policy, preload_libraries_);

4
content/public/browser/content_browser_client.cc

@ -990,10 +990,6 @@ bool ContentBrowserClient::IsRendererCodeIntegrityEnabled() {
return false;
}
bool ContentBrowserClient::IsPdfFontProxyEnabled() {
return false;
}
bool ContentBrowserClient::ShouldEnableAudioProcessHighPriority() {
// TODO(crbug.com/40242320): Delete this method when the
// kAudioProcessHighPriorityEnabled enterprise policy is deprecated.

5
content/public/browser/content_browser_client.h

@ -1726,11 +1726,6 @@ class CONTENT_EXPORT ContentBrowserClient {
// This is called on the UI thread.
virtual bool IsRendererCodeIntegrityEnabled();
// Returns whether PDF fallback fonts are proxied to child processes. If false
// they may require direct file system access. This is a short-term API and
// will be removed once crbug.com/344643689 is launched.
virtual bool IsPdfFontProxyEnabled();
// Performs a fast and orderly shutdown of the browser. If present,
// `control_type` is a CTRL_* value from a Windows console control handler;
// see https://learn.microsoft.com/en-us/windows/console/handlerroutine.

9
pdf/pdf_features.cc

@ -48,15 +48,6 @@ BASE_FEATURE(kPdfXfaSupport,
BASE_FEATURE(kPdfInk2, "PdfInk2", base::FEATURE_DISABLED_BY_DEFAULT);
#endif
#if BUILDFLAG(IS_WIN)
// On Windows - if enabled uses Skia to load system fonts, otherwise uses direct
// file system access that must be brokered by the sandbox.
// TODO(crbug.com/344643689) remove after M129.
BASE_FEATURE(kWinPdfUseFontProxy,
"WinPdfUseFontProxy",
base::FEATURE_ENABLED_BY_DEFAULT);
#endif
void SetIsOopifPdfPolicyEnabled(bool is_oopif_pdf_policy_enabled) {
g_is_oopif_pdf_policy_enabled = is_oopif_pdf_policy_enabled;
}

4
pdf/pdf_features.h

@ -28,10 +28,6 @@ BASE_DECLARE_FEATURE(kPdfXfaSupport);
BASE_DECLARE_FEATURE(kPdfInk2);
#endif
#if BUILDFLAG(IS_WIN)
BASE_DECLARE_FEATURE(kWinPdfUseFontProxy);
#endif
// Sets whether the OOPIF PDF policy enables the OOPIF PDF viewer. Otherwise,
// GuestView PDF viewer will be used. The policy is enabled by default.
void SetIsOopifPdfPolicyEnabled(bool is_oopif_pdf_policy_enabled);

3
pdf/pdfium/pdfium_engine.cc

@ -529,8 +529,7 @@ void InitializeSDK(bool enable_v8,
#endif
#if BUILDFLAG(IS_WIN)
if (font_mapping_mode == FontMappingMode::kBlink &&
base::FeatureList::IsEnabled(features::kWinPdfUseFontProxy)) {
if (font_mapping_mode == FontMappingMode::kBlink) {
g_font_mapping_mode = font_mapping_mode;
InitializeWindowsFontMapper();
}

7
pdf/pdfium/pdfium_font_win_unittest.cc

@ -8,8 +8,6 @@
#include "base/containers/heap_array.h"
#include "base/memory/raw_ptr.h"
#include "base/test/scoped_feature_list.h"
#include "pdf/pdf_features.h"
#include "pdf/pdfium/pdfium_engine.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/pdfium/public/fpdf_sysfontinfo.h"
@ -26,9 +24,7 @@ class PDFiumFontWinTest : public testing::Test {
// Secretly this is a skia typeface id.
using FontId = void*;
PDFiumFontWinTest() {
scoped_feature_list_.InitWithFeatures({features::kWinPdfUseFontProxy}, {});
}
PDFiumFontWinTest() = default;
PDFiumFontWinTest(const PDFiumFontWinTest&) = delete;
PDFiumFontWinTest& operator=(const PDFiumFontWinTest&) = delete;
~PDFiumFontWinTest() override = default;
@ -68,7 +64,6 @@ class PDFiumFontWinTest : public testing::Test {
private:
raw_ptr<FPDF_SYSFONTINFO> mapper_;
base::test::ScopedFeatureList scoped_feature_list_;
};
} // namespace

15
testing/variations/fieldtrial_testing_config.json

@ -26628,21 +26628,6 @@
]
}
],
"WinPdfUseFontProxy": [
{
"platforms": [
"windows"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"WinPdfUseFontProxy"
]
}
]
}
],
"WinSboxACProfileWithoutFirewall": [
{
"platforms": [