Determine whether to use Skia renderer from browser process
Currently PDFium SDK determines whether to use Skia renderer in the renderer process, and it's determined by feature flag (it's a choice made by either the user or the finch experiment). An enterprise policy will be created to also control the decision on whether to use Skia renderer. Therefore the decision point needs to be moved into the browser process. This CL moves the feature flag check to the browser process before the PDF plugin is created, and sends a `use_skia` flag through the mime handler together with other PDF plugin attributes, so that the logic of checking enterprise policy can be added in the browser process in the future. Bug: 1440430 Change-Id: I8b74d493576f62be6231b872aec4018e7627a3b7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4481627 Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: Nigi <nigi@chromium.org> Reviewed-by: Matthew Denton <mpdenton@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/main@{#1141525}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
eb059009fe
commit
4e9dea8d3d
chrome/browser/pdf
components/pdf/browser
extensions
pdf
@ -31,6 +31,7 @@ source_set("pdf") {
|
|||||||
"//extensions/common:common_constants",
|
"//extensions/common:common_constants",
|
||||||
"//extensions/common/api",
|
"//extensions/common/api",
|
||||||
"//pdf:buildflags",
|
"//pdf:buildflags",
|
||||||
|
"//pdf:features",
|
||||||
"//printing/buildflags",
|
"//printing/buildflags",
|
||||||
"//ui/base",
|
"//ui/base",
|
||||||
"//ui/gfx:color_utils",
|
"//ui/gfx:color_utils",
|
||||||
|
@ -7,6 +7,7 @@
|
|||||||
#include <string>
|
#include <string>
|
||||||
#include <utility>
|
#include <utility>
|
||||||
|
|
||||||
|
#include "base/feature_list.h"
|
||||||
#include "base/memory/weak_ptr.h"
|
#include "base/memory/weak_ptr.h"
|
||||||
#include "base/no_destructor.h"
|
#include "base/no_destructor.h"
|
||||||
#include "base/numerics/safe_conversions.h"
|
#include "base/numerics/safe_conversions.h"
|
||||||
@ -16,6 +17,7 @@
|
|||||||
#include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
|
#include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
|
||||||
#include "extensions/common/api/mime_handler.mojom.h"
|
#include "extensions/common/api/mime_handler.mojom.h"
|
||||||
#include "extensions/common/constants.h"
|
#include "extensions/common/constants.h"
|
||||||
|
#include "pdf/pdf_features.h"
|
||||||
#include "printing/buildflags/buildflags.h"
|
#include "printing/buildflags/buildflags.h"
|
||||||
#include "third_party/abseil-cpp/absl/types/optional.h"
|
#include "third_party/abseil-cpp/absl/types/optional.h"
|
||||||
#include "ui/base/resource/resource_bundle.h"
|
#include "ui/base/resource/resource_bundle.h"
|
||||||
@ -85,6 +87,12 @@ absl::optional<GURL> ChromePdfStreamDelegate::MapToOriginalUrl(
|
|||||||
stream->pdf_plugin_attributes()->background_color);
|
stream->pdf_plugin_attributes()->background_color);
|
||||||
info.full_frame = !stream->embedded();
|
info.full_frame = !stream->embedded();
|
||||||
info.allow_javascript = stream->pdf_plugin_attributes()->allow_javascript;
|
info.allow_javascript = stream->pdf_plugin_attributes()->allow_javascript;
|
||||||
|
|
||||||
|
// TODO(crbug.com/1440430): Set `info.use_skia` based on both the feature
|
||||||
|
// flag and the enterprise policy once the policy is available for the Skia
|
||||||
|
// finch experiment.
|
||||||
|
info.use_skia =
|
||||||
|
base::FeatureList::IsEnabled(chrome_pdf::features::kPdfUseSkiaRenderer);
|
||||||
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
|
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
|
||||||
} else if (stream_url.GetWithEmptyPath() ==
|
} else if (stream_url.GetWithEmptyPath() ==
|
||||||
chrome::kChromeUIUntrustedPrintURL) {
|
chrome::kChromeUIUntrustedPrintURL) {
|
||||||
@ -94,6 +102,8 @@ absl::optional<GURL> ChromePdfStreamDelegate::MapToOriginalUrl(
|
|||||||
info.background_color = gfx::kGoogleGrey300;
|
info.background_color = gfx::kGoogleGrey300;
|
||||||
info.full_frame = false;
|
info.full_frame = false;
|
||||||
info.allow_javascript = false;
|
info.allow_javascript = false;
|
||||||
|
info.use_skia =
|
||||||
|
base::FeatureList::IsEnabled(chrome_pdf::features::kPdfUseSkiaRenderer);
|
||||||
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)
|
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)
|
||||||
} else {
|
} else {
|
||||||
return absl::nullopt;
|
return absl::nullopt;
|
||||||
|
@ -43,6 +43,7 @@ class PdfStreamDelegate {
|
|||||||
SkColor background_color = SK_ColorTRANSPARENT;
|
SkColor background_color = SK_ColorTRANSPARENT;
|
||||||
bool full_frame = false;
|
bool full_frame = false;
|
||||||
bool allow_javascript = false;
|
bool allow_javascript = false;
|
||||||
|
bool use_skia = false;
|
||||||
};
|
};
|
||||||
|
|
||||||
PdfStreamDelegate();
|
PdfStreamDelegate();
|
||||||
|
@ -65,7 +65,7 @@ embed {
|
|||||||
</style>
|
</style>
|
||||||
<div id="sizer"></div>
|
<div id="sizer"></div>
|
||||||
<embed type="application/x-google-chrome-pdf" src="$1" original-url="$2"
|
<embed type="application/x-google-chrome-pdf" src="$1" original-url="$2"
|
||||||
background-color="$4" javascript="$5"$6>
|
background-color="$4" javascript="$5"$6$7>
|
||||||
<script type="module">
|
<script type="module">
|
||||||
$3
|
$3
|
||||||
</script>
|
</script>
|
||||||
@ -82,7 +82,8 @@ $3
|
|||||||
stream_info.injected_script ? *stream_info.injected_script : "",
|
stream_info.injected_script ? *stream_info.injected_script : "",
|
||||||
base::NumberToString(stream_info.background_color),
|
base::NumberToString(stream_info.background_color),
|
||||||
stream_info.allow_javascript ? "allow" : "block",
|
stream_info.allow_javascript ? "allow" : "block",
|
||||||
stream_info.full_frame ? " full-frame" : ""},
|
stream_info.full_frame ? " full-frame" : "",
|
||||||
|
stream_info.use_skia ? " use-skia" : ""},
|
||||||
/*offsets=*/nullptr);
|
/*offsets=*/nullptr);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -51,32 +51,38 @@ TEST_F(MimeHandlerServiceImplTest, SetValidPdfPluginAttributes) {
|
|||||||
{
|
{
|
||||||
const double kBackgroundColor = 4292533472.0f;
|
const double kBackgroundColor = 4292533472.0f;
|
||||||
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
||||||
/*background_color=*/kBackgroundColor, /*allow_javascript=*/true));
|
/*background_color=*/kBackgroundColor, /*allow_javascript=*/true,
|
||||||
|
/*use_skia=*/true));
|
||||||
ASSERT_TRUE(stream_container_->pdf_plugin_attributes());
|
ASSERT_TRUE(stream_container_->pdf_plugin_attributes());
|
||||||
|
|
||||||
EXPECT_EQ(kBackgroundColor,
|
EXPECT_EQ(kBackgroundColor,
|
||||||
stream_container_->pdf_plugin_attributes()->background_color);
|
stream_container_->pdf_plugin_attributes()->background_color);
|
||||||
EXPECT_TRUE(stream_container_->pdf_plugin_attributes()->allow_javascript);
|
EXPECT_TRUE(stream_container_->pdf_plugin_attributes()->allow_javascript);
|
||||||
|
EXPECT_TRUE(stream_container_->pdf_plugin_attributes()->use_skia);
|
||||||
}
|
}
|
||||||
|
|
||||||
{
|
{
|
||||||
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
||||||
/*background_color=*/0.0f, /*allow_javascript=*/true));
|
/*background_color=*/0.0f, /*allow_javascript=*/true,
|
||||||
|
/*use_skia=*/false));
|
||||||
ASSERT_TRUE(stream_container_->pdf_plugin_attributes());
|
ASSERT_TRUE(stream_container_->pdf_plugin_attributes());
|
||||||
|
|
||||||
EXPECT_EQ(0.0f,
|
EXPECT_EQ(0.0f,
|
||||||
stream_container_->pdf_plugin_attributes()->background_color);
|
stream_container_->pdf_plugin_attributes()->background_color);
|
||||||
EXPECT_TRUE(stream_container_->pdf_plugin_attributes()->allow_javascript);
|
EXPECT_TRUE(stream_container_->pdf_plugin_attributes()->allow_javascript);
|
||||||
|
EXPECT_FALSE(stream_container_->pdf_plugin_attributes()->use_skia);
|
||||||
}
|
}
|
||||||
|
|
||||||
{
|
{
|
||||||
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
||||||
/*background_color=*/UINT32_MAX, /*allow_javascript=*/false));
|
/*background_color=*/UINT32_MAX, /*allow_javascript=*/false,
|
||||||
|
/*use_skia=*/true));
|
||||||
ASSERT_TRUE(stream_container_->pdf_plugin_attributes());
|
ASSERT_TRUE(stream_container_->pdf_plugin_attributes());
|
||||||
|
|
||||||
EXPECT_EQ(static_cast<double>(UINT32_MAX),
|
EXPECT_EQ(static_cast<double>(UINT32_MAX),
|
||||||
stream_container_->pdf_plugin_attributes()->background_color);
|
stream_container_->pdf_plugin_attributes()->background_color);
|
||||||
EXPECT_FALSE(stream_container_->pdf_plugin_attributes()->allow_javascript);
|
EXPECT_FALSE(stream_container_->pdf_plugin_attributes()->allow_javascript);
|
||||||
|
EXPECT_TRUE(stream_container_->pdf_plugin_attributes()->use_skia);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -85,7 +91,8 @@ TEST_F(MimeHandlerServiceImplTest,
|
|||||||
{
|
{
|
||||||
// Background is not an integer.
|
// Background is not an integer.
|
||||||
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
||||||
/*background_color=*/12.34, /*allow_javascript=*/true));
|
/*background_color=*/12.34, /*allow_javascript=*/true,
|
||||||
|
/*use_skia=*/true));
|
||||||
EXPECT_FALSE(stream_container_->pdf_plugin_attributes());
|
EXPECT_FALSE(stream_container_->pdf_plugin_attributes());
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -93,7 +100,8 @@ TEST_F(MimeHandlerServiceImplTest,
|
|||||||
// Background color is beyond the range of an uint32_t.
|
// Background color is beyond the range of an uint32_t.
|
||||||
uint64_t color_beyond_range = UINT32_MAX + static_cast<uint64_t>(1);
|
uint64_t color_beyond_range = UINT32_MAX + static_cast<uint64_t>(1);
|
||||||
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
service_->SetPdfPluginAttributes(mime_handler::PdfPluginAttributes::New(
|
||||||
static_cast<double>(color_beyond_range), /*allow_javascript=*/true));
|
static_cast<double>(color_beyond_range), /*allow_javascript=*/true,
|
||||||
|
/*use_skia=*/true));
|
||||||
EXPECT_FALSE(stream_container_->pdf_plugin_attributes());
|
EXPECT_FALSE(stream_container_->pdf_plugin_attributes());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -39,6 +39,9 @@ struct PdfPluginAttributes {
|
|||||||
// Indicates whether the plugin should allow running JavaScript. Loading XFA
|
// Indicates whether the plugin should allow running JavaScript. Loading XFA
|
||||||
// for PDF forms will automatically be disabled if this flag is false.
|
// for PDF forms will automatically be disabled if this flag is false.
|
||||||
bool allow_javascript;
|
bool allow_javascript;
|
||||||
|
|
||||||
|
// Indicates whether the PDF viewer should use Skia renderer.
|
||||||
|
bool use_skia;
|
||||||
};
|
};
|
||||||
|
|
||||||
// Provides a mime handler guest methods to get and modify the stream associated
|
// Provides a mime handler guest methods to get and modify the stream associated
|
||||||
|
@ -46,6 +46,8 @@ absl::optional<ParsedParams> ParseWebPluginParams(
|
|||||||
result.script_option = PDFiumFormFiller::ScriptOption::kNoJavaScript;
|
result.script_option = PDFiumFormFiller::ScriptOption::kNoJavaScript;
|
||||||
} else if (params.attribute_names[i] == "has-edits") {
|
} else if (params.attribute_names[i] == "has-edits") {
|
||||||
result.has_edits = true;
|
result.has_edits = true;
|
||||||
|
} else if (params.attribute_names[i] == "use-skia") {
|
||||||
|
result.use_skia = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -46,6 +46,10 @@ struct ParsedParams {
|
|||||||
|
|
||||||
// Whether the PDF was edited previously in annotation mode.
|
// Whether the PDF was edited previously in annotation mode.
|
||||||
bool has_edits = false;
|
bool has_edits = false;
|
||||||
|
|
||||||
|
// Whether the PDF viewer uses Skia renderer. When set to false, the PDF
|
||||||
|
// viewer uses AGG renderer.
|
||||||
|
bool use_skia = false;
|
||||||
};
|
};
|
||||||
|
|
||||||
// Creates an `ParsedParams` by parsing a `blink::WebPluginParams`. If
|
// Creates an `ParsedParams` by parsing a `blink::WebPluginParams`. If
|
||||||
|
@ -46,6 +46,7 @@ TEST(ParsedParamsTest, ParseWebPluginParamsMinimal) {
|
|||||||
EXPECT_EQ(SK_ColorTRANSPARENT, result->background_color);
|
EXPECT_EQ(SK_ColorTRANSPARENT, result->background_color);
|
||||||
EXPECT_EQ(PDFiumFormFiller::DefaultScriptOption(), result->script_option);
|
EXPECT_EQ(PDFiumFormFiller::DefaultScriptOption(), result->script_option);
|
||||||
EXPECT_FALSE(result->has_edits);
|
EXPECT_FALSE(result->has_edits);
|
||||||
|
EXPECT_FALSE(result->use_skia);
|
||||||
}
|
}
|
||||||
|
|
||||||
TEST(ParsedParamsTest, ParseWebPluginParamsWithoutSourceUrl) {
|
TEST(ParsedParamsTest, ParseWebPluginParamsWithoutSourceUrl) {
|
||||||
@ -181,4 +182,26 @@ TEST(ParsedParamsTest, ParseWebPluginParamsWithHasEditsNonEmpty) {
|
|||||||
EXPECT_TRUE(result->has_edits);
|
EXPECT_TRUE(result->has_edits);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST(ParsedParamsTest, ParseWebPluginParamsWithHasUseSkia) {
|
||||||
|
blink::WebPluginParams params = CreateMinimalWebPluginParams();
|
||||||
|
params.attribute_names.push_back(blink::WebString("use-skia"));
|
||||||
|
params.attribute_values.push_back(blink::WebString(""));
|
||||||
|
|
||||||
|
absl::optional<ParsedParams> result = ParseWebPluginParams(params);
|
||||||
|
ASSERT_TRUE(result.has_value());
|
||||||
|
|
||||||
|
EXPECT_TRUE(result->use_skia);
|
||||||
|
}
|
||||||
|
|
||||||
|
TEST(ParsedParamsTest, ParseWebPluginParamsWithHasUseSkiaNonEmpty) {
|
||||||
|
blink::WebPluginParams params = CreateMinimalWebPluginParams();
|
||||||
|
params.attribute_names.push_back(blink::WebString("use-skia"));
|
||||||
|
params.attribute_values.push_back(blink::WebString("false"));
|
||||||
|
|
||||||
|
absl::optional<ParsedParams> result = ParseWebPluginParams(params);
|
||||||
|
ASSERT_TRUE(result.has_value());
|
||||||
|
|
||||||
|
EXPECT_TRUE(result->use_skia);
|
||||||
|
}
|
||||||
|
|
||||||
} // namespace chrome_pdf
|
} // namespace chrome_pdf
|
||||||
|
@ -24,6 +24,9 @@ class ScopedSdkInitializer {
|
|||||||
public:
|
public:
|
||||||
explicit ScopedSdkInitializer(bool enable_v8) {
|
explicit ScopedSdkInitializer(bool enable_v8) {
|
||||||
if (!IsSDKInitializedViaPlugin()) {
|
if (!IsSDKInitializedViaPlugin()) {
|
||||||
|
// TODO(crbug.com/1440430): Modify ScopedSdkInitializer() so that the
|
||||||
|
// option to use Skia can be based on enterprise policy if such policy is
|
||||||
|
// set.
|
||||||
InitializeSDK(enable_v8,
|
InitializeSDK(enable_v8,
|
||||||
base::FeatureList::IsEnabled(features::kPdfUseSkiaRenderer),
|
base::FeatureList::IsEnabled(features::kPdfUseSkiaRenderer),
|
||||||
FontMappingMode::kNoMapping);
|
FontMappingMode::kNoMapping);
|
||||||
|
@ -179,7 +179,7 @@ class PerProcessInitializer final {
|
|||||||
return *instance;
|
return *instance;
|
||||||
}
|
}
|
||||||
|
|
||||||
void Acquire() {
|
void Acquire(bool use_skia) {
|
||||||
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
|
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
|
||||||
|
|
||||||
DCHECK_GE(init_count_, 0);
|
DCHECK_GE(init_count_, 0);
|
||||||
@ -187,9 +187,7 @@ class PerProcessInitializer final {
|
|||||||
return;
|
return;
|
||||||
|
|
||||||
DCHECK(!IsSDKInitializedViaPlugin());
|
DCHECK(!IsSDKInitializedViaPlugin());
|
||||||
InitializeSDK(/*enable_v8=*/true,
|
InitializeSDK(/*enable_v8=*/true, use_skia, FontMappingMode::kBlink);
|
||||||
base::FeatureList::IsEnabled(features::kPdfUseSkiaRenderer),
|
|
||||||
FontMappingMode::kBlink);
|
|
||||||
SetIsSDKInitializedViaPlugin(true);
|
SetIsSDKInitializedViaPlugin(true);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -335,7 +333,7 @@ bool PdfViewWebPlugin::InitializeCommon() {
|
|||||||
base::debug::CrashKeySize::Size256);
|
base::debug::CrashKeySize::Size256);
|
||||||
base::debug::SetCrashKeyString(subresource_url, params->original_url);
|
base::debug::SetCrashKeyString(subresource_url, params->original_url);
|
||||||
|
|
||||||
PerProcessInitializer::GetInstance().Acquire();
|
PerProcessInitializer::GetInstance().Acquire(params->use_skia);
|
||||||
initialized_ = true;
|
initialized_ = true;
|
||||||
|
|
||||||
// Check if the PDF is being loaded in the PDF chrome extension. We only allow
|
// Check if the PDF is being loaded in the PDF chrome extension. We only allow
|
||||||
|
Reference in New Issue
Block a user