Commit 37fa4e59 authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Paint Preview] Restrict capture size over mojo

This CL adds support for plumbing a maximum capture size restriction
to the PaintPreviewRecorde returning the size of the output.

For now, TabbedPaintPreviewPlayer will use this to cap per SkPicture
capture size to 5 MB.

In a follow up the returned size information will be used to restrict
the total capture size rather than the size per renderer.

Bug: 1071446
Change-Id: Ib809d3156c11e6a8ee6144663794fe261f89f635
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2152957Reviewed-by: default avatarMehran Mahmoudi <mahmoudi@chromium.org>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761053}
parent 1075b89a
...@@ -103,7 +103,7 @@ void PaintPreviewDemoService::CaptureTabInternal( ...@@ -103,7 +103,7 @@ void PaintPreviewDemoService::CaptureTabInternal(
} }
PaintPreviewBaseService::CapturePaintPreview( PaintPreviewBaseService::CapturePaintPreview(
web_contents, file_path.value(), gfx::Rect(), web_contents, file_path.value(), gfx::Rect(), /*max_per_capture_size=*/0,
base::BindOnce(&PaintPreviewDemoService::OnCaptured, base::BindOnce(&PaintPreviewDemoService::OnCaptured,
weak_ptr_factory_.GetWeakPtr(), std::move(callback), key)); weak_ptr_factory_.GetWeakPtr(), std::move(callback), key));
} }
......
...@@ -26,6 +26,8 @@ namespace paint_preview { ...@@ -26,6 +26,8 @@ namespace paint_preview {
namespace { namespace {
constexpr size_t kMaxPerCaptureSizeBytes = 5 * 1000L * 1000L; // 5 MB.
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
void JavaBooleanCallbackAdapter(base::OnceCallback<void(bool)> callback, void JavaBooleanCallbackAdapter(base::OnceCallback<void(bool)> callback,
PaintPreviewTabService::Status status) { PaintPreviewTabService::Status status) {
...@@ -204,7 +206,7 @@ void PaintPreviewTabService::CaptureTabInternal( ...@@ -204,7 +206,7 @@ void PaintPreviewTabService::CaptureTabInternal(
return; return;
} }
CapturePaintPreview( CapturePaintPreview(
contents, file_path.value(), gfx::Rect(0, 0, 0, 0), contents, file_path.value(), gfx::Rect(), kMaxPerCaptureSizeBytes,
base::BindOnce(&PaintPreviewTabService::OnCaptured, base::BindOnce(&PaintPreviewTabService::OnCaptured,
weak_ptr_factory_.GetWeakPtr(), tab_id, key, weak_ptr_factory_.GetWeakPtr(), tab_id, key,
frame_tree_node_id, std::move(callback))); frame_tree_node_id, std::move(callback)));
......
...@@ -149,6 +149,7 @@ void InitiateCapture(scoped_refptr<FileManager> manager, ...@@ -149,6 +149,7 @@ void InitiateCapture(scoped_refptr<FileManager> manager,
params.document_guid = base::UnguessableToken::Create(); params.document_guid = base::UnguessableToken::Create();
params.is_main_frame = true; params.is_main_frame = true;
params.root_dir = url_path.value(); params.root_dir = url_path.value();
params.max_per_capture_size = 0; // Unlimited size.
ukm::SourceId source_id = ukm::GetSourceIdForWebContentsDocument(contents); ukm::SourceId source_id = ukm::GetSourceIdForWebContentsDocument(contents);
auto start_time = base::TimeTicks::Now(); auto start_time = base::TimeTicks::Now();
......
...@@ -63,9 +63,10 @@ void PaintPreviewBaseService::CapturePaintPreview( ...@@ -63,9 +63,10 @@ void PaintPreviewBaseService::CapturePaintPreview(
content::WebContents* web_contents, content::WebContents* web_contents,
const base::FilePath& root_dir, const base::FilePath& root_dir,
gfx::Rect clip_rect, gfx::Rect clip_rect,
size_t max_per_capture_size,
OnCapturedCallback callback) { OnCapturedCallback callback) {
CapturePaintPreview(web_contents, web_contents->GetMainFrame(), root_dir, CapturePaintPreview(web_contents, web_contents->GetMainFrame(), root_dir,
clip_rect, std::move(callback)); clip_rect, max_per_capture_size, std::move(callback));
} }
void PaintPreviewBaseService::CapturePaintPreview( void PaintPreviewBaseService::CapturePaintPreview(
...@@ -73,6 +74,7 @@ void PaintPreviewBaseService::CapturePaintPreview( ...@@ -73,6 +74,7 @@ void PaintPreviewBaseService::CapturePaintPreview(
content::RenderFrameHost* render_frame_host, content::RenderFrameHost* render_frame_host,
const base::FilePath& root_dir, const base::FilePath& root_dir,
gfx::Rect clip_rect, gfx::Rect clip_rect,
size_t max_per_capture_size,
OnCapturedCallback callback) { OnCapturedCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (policy_ && !policy_->SupportedForContents(web_contents)) { if (policy_ && !policy_->SupportedForContents(web_contents)) {
...@@ -92,6 +94,7 @@ void PaintPreviewBaseService::CapturePaintPreview( ...@@ -92,6 +94,7 @@ void PaintPreviewBaseService::CapturePaintPreview(
params.clip_rect = clip_rect; params.clip_rect = clip_rect;
params.is_main_frame = (render_frame_host == web_contents->GetMainFrame()); params.is_main_frame = (render_frame_host == web_contents->GetMainFrame());
params.root_dir = root_dir; params.root_dir = root_dir;
params.max_per_capture_size = max_per_capture_size;
// TODO(crbug/1064253): Consider moving to client so that this always happens. // TODO(crbug/1064253): Consider moving to client so that this always happens.
// Although, it is harder to get this right in the client due to its // Although, it is harder to get this right in the client due to its
......
...@@ -109,11 +109,13 @@ class PaintPreviewBaseService : public KeyedService { ...@@ -109,11 +109,13 @@ class PaintPreviewBaseService : public KeyedService {
// Captures the main frame of |web_contents| (an observer for capturing Paint // Captures the main frame of |web_contents| (an observer for capturing Paint
// Previews is created for web contents if it does not exist). The capture is // Previews is created for web contents if it does not exist). The capture is
// attributed to the URL of the main frame and is stored in |root_dir|. The // attributed to the URL of the main frame and is stored in |root_dir|. The
// captured area is clipped to |clip_rect| if it is non-zero. On completion // captured area is clipped to |clip_rect| if it is non-zero. Caps the per
// frame SkPicture size to |max_per_capture_size| if non-zero. On completion
// the status of the capture is provided via |callback|. // the status of the capture is provided via |callback|.
void CapturePaintPreview(content::WebContents* web_contents, void CapturePaintPreview(content::WebContents* web_contents,
const base::FilePath& root_dir, const base::FilePath& root_dir,
gfx::Rect clip_rect, gfx::Rect clip_rect,
size_t max_per_capture_size,
OnCapturedCallback callback); OnCapturedCallback callback);
// Same as above except |render_frame_host| is directly captured rather than // Same as above except |render_frame_host| is directly captured rather than
// the main frame. // the main frame.
...@@ -121,6 +123,7 @@ class PaintPreviewBaseService : public KeyedService { ...@@ -121,6 +123,7 @@ class PaintPreviewBaseService : public KeyedService {
content::RenderFrameHost* render_frame_host, content::RenderFrameHost* render_frame_host,
const base::FilePath& root_dir, const base::FilePath& root_dir,
gfx::Rect clip_rect, gfx::Rect clip_rect,
size_t max_per_capture_size,
OnCapturedCallback callback); OnCapturedCallback callback);
// Starts the compositor service in a utility process. // Starts the compositor service in a utility process.
......
...@@ -171,6 +171,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { ...@@ -171,6 +171,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) {
auto params = mojom::PaintPreviewCaptureParams::New(); auto params = mojom::PaintPreviewCaptureParams::New();
params->clip_rect = gfx::Rect(0, 0, 0, 0); params->clip_rect = gfx::Rect(0, 0, 0, 0);
params->is_main_frame = true; params->is_main_frame = true;
params->max_capture_size = 50;
recorder.SetExpectedParams(std::move(params)); recorder.SetExpectedParams(std::move(params));
auto response = mojom::PaintPreviewCaptureResponse::New(); auto response = mojom::PaintPreviewCaptureResponse::New();
response->embedding_token = base::nullopt; response->embedding_token = base::nullopt;
...@@ -185,7 +186,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { ...@@ -185,7 +186,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) {
base::RunLoop loop; base::RunLoop loop;
service->CapturePaintPreview( service->CapturePaintPreview(
web_contents(), path, gfx::Rect(0, 0, 0, 0), web_contents(), path, gfx::Rect(0, 0, 0, 0), 50,
base::BindOnce( base::BindOnce(
[](base::OnceClosure quit_closure, [](base::OnceClosure quit_closure,
PaintPreviewBaseService::CaptureStatus expected_status, PaintPreviewBaseService::CaptureStatus expected_status,
...@@ -224,6 +225,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) { ...@@ -224,6 +225,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) {
auto params = mojom::PaintPreviewCaptureParams::New(); auto params = mojom::PaintPreviewCaptureParams::New();
params->clip_rect = gfx::Rect(0, 0, 0, 0); params->clip_rect = gfx::Rect(0, 0, 0, 0);
params->is_main_frame = true; params->is_main_frame = true;
params->max_capture_size = 0;
recorder.SetExpectedParams(std::move(params)); recorder.SetExpectedParams(std::move(params));
auto response = mojom::PaintPreviewCaptureResponse::New(); auto response = mojom::PaintPreviewCaptureResponse::New();
response->embedding_token = base::nullopt; response->embedding_token = base::nullopt;
...@@ -238,7 +240,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) { ...@@ -238,7 +240,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) {
base::RunLoop loop; base::RunLoop loop;
service->CapturePaintPreview( service->CapturePaintPreview(
web_contents(), path, gfx::Rect(0, 0, 0, 0), web_contents(), path, gfx::Rect(0, 0, 0, 0), 0,
base::BindOnce( base::BindOnce(
[](base::OnceClosure quit_closure, [](base::OnceClosure quit_closure,
PaintPreviewBaseService::CaptureStatus expected_status, PaintPreviewBaseService::CaptureStatus expected_status,
...@@ -258,6 +260,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) { ...@@ -258,6 +260,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) {
auto params = mojom::PaintPreviewCaptureParams::New(); auto params = mojom::PaintPreviewCaptureParams::New();
params->clip_rect = gfx::Rect(0, 0, 0, 0); params->clip_rect = gfx::Rect(0, 0, 0, 0);
params->is_main_frame = true; params->is_main_frame = true;
params->max_capture_size = 0;
recorder.SetExpectedParams(std::move(params)); recorder.SetExpectedParams(std::move(params));
auto response = mojom::PaintPreviewCaptureResponse::New(); auto response = mojom::PaintPreviewCaptureResponse::New();
response->embedding_token = base::nullopt; response->embedding_token = base::nullopt;
...@@ -272,7 +275,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) { ...@@ -272,7 +275,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) {
base::RunLoop loop; base::RunLoop loop;
service->CapturePaintPreview( service->CapturePaintPreview(
web_contents(), path, gfx::Rect(0, 0, 0, 0), web_contents(), path, gfx::Rect(0, 0, 0, 0), 0,
base::BindOnce( base::BindOnce(
[](base::OnceClosure quit_closure, [](base::OnceClosure quit_closure,
PaintPreviewBaseService::CaptureStatus expected_status, PaintPreviewBaseService::CaptureStatus expected_status,
......
...@@ -102,8 +102,9 @@ PaintPreviewClient::PaintPreviewData::PaintPreviewData() = default; ...@@ -102,8 +102,9 @@ PaintPreviewClient::PaintPreviewData::PaintPreviewData() = default;
PaintPreviewClient::PaintPreviewData::~PaintPreviewData() = default; PaintPreviewClient::PaintPreviewData::~PaintPreviewData() = default;
PaintPreviewClient::PaintPreviewData& PaintPreviewClient::PaintPreviewData:: PaintPreviewClient::PaintPreviewData&
operator=(PaintPreviewData&& rhs) = default; PaintPreviewClient::PaintPreviewData::operator=(PaintPreviewData&& rhs) =
default;
PaintPreviewClient::PaintPreviewData::PaintPreviewData( PaintPreviewClient::PaintPreviewData::PaintPreviewData(
PaintPreviewData&& other) noexcept = default; PaintPreviewData&& other) noexcept = default;
...@@ -212,6 +213,7 @@ mojom::PaintPreviewCaptureParamsPtr PaintPreviewClient::CreateMojoParams( ...@@ -212,6 +213,7 @@ mojom::PaintPreviewCaptureParamsPtr PaintPreviewClient::CreateMojoParams(
mojo_params->clip_rect = params.clip_rect; mojo_params->clip_rect = params.clip_rect;
mojo_params->is_main_frame = params.is_main_frame; mojo_params->is_main_frame = params.is_main_frame;
mojo_params->file = std::move(file); mojo_params->file = std::move(file);
mojo_params->max_capture_size = params.max_per_capture_size;
return mojo_params; return mojo_params;
} }
......
...@@ -24,7 +24,7 @@ ...@@ -24,7 +24,7 @@
namespace paint_preview { namespace paint_preview {
// Client responsible for making requests to the mojom::PaintPreviewService. A // Client responsible for making requests to the mojom::PaintPreviewRecorder. A
// client coordinates between multiple frames and handles capture and // client coordinates between multiple frames and handles capture and
// aggreagation of data from both the main frame and subframes. // aggreagation of data from both the main frame and subframes.
// //
...@@ -56,6 +56,14 @@ class PaintPreviewClient ...@@ -56,6 +56,14 @@ class PaintPreviewClient
// Whether the capture is for the main frame or an OOP subframe. // Whether the capture is for the main frame or an OOP subframe.
bool is_main_frame; bool is_main_frame;
// The maximum capture size allowed per SkPicture captured. A size of 0 is
// unlimited.
// TODO(crbug/1071446): Ideally, this would cap the total size rather than
// being a per SkPicture limit. However, that is non-trivial due to the
// async ordering of captures from different frames making it hard to keep
// track of available headroom at the time of each capture triggering.
size_t max_per_capture_size;
}; };
~PaintPreviewClient() override; ~PaintPreviewClient() override;
......
...@@ -50,6 +50,10 @@ struct PaintPreviewCaptureParams { ...@@ -50,6 +50,10 @@ struct PaintPreviewCaptureParams {
// File to write the SkPicture to (write-only). A separate file should be // File to write the SkPicture to (write-only). A separate file should be
// created for each RenderFrame. // created for each RenderFrame.
mojo_base.mojom.File file; mojo_base.mojom.File file;
// The maximum allowed size of a capture that can be produced. A value of
// 0 means the size is unrestricted.
uint64 max_capture_size;
}; };
struct LinkData { struct LinkData {
...@@ -72,6 +76,9 @@ struct PaintPreviewCaptureResponse { ...@@ -72,6 +76,9 @@ struct PaintPreviewCaptureResponse {
// The time spent capturing in Blink. // The time spent capturing in Blink.
mojo_base.mojom.TimeDelta blink_recording_time; mojo_base.mojom.TimeDelta blink_recording_time;
// The size of the resulting serialized capture.
uint64 serialized_size;
}; };
// Service for capturing a paint preview of a RenderFrame's contents. This // Service for capturing a paint preview of a RenderFrame's contents. This
......
...@@ -22,21 +22,21 @@ namespace paint_preview { ...@@ -22,21 +22,21 @@ namespace paint_preview {
namespace { namespace {
// TODO(crbug/1071446): Provide this value over mojo in the capture params.
constexpr size_t kDefaultMaxCaptureSizeBytes = 0; // 0 = unlimited.
mojom::PaintPreviewStatus FinishRecording( mojom::PaintPreviewStatus FinishRecording(
sk_sp<const cc::PaintRecord> recording, sk_sp<const cc::PaintRecord> recording,
const gfx::Rect& bounds, const gfx::Rect& bounds,
PaintPreviewTracker* tracker, PaintPreviewTracker* tracker,
base::File skp_file, base::File skp_file,
size_t max_capture_size,
mojom::PaintPreviewCaptureResponse* response) { mojom::PaintPreviewCaptureResponse* response) {
ParseGlyphs(recording.get(), tracker); ParseGlyphs(recording.get(), tracker);
size_t serialized_size = 0;
if (!SerializeAsSkPicture(recording, tracker, bounds, std::move(skp_file), if (!SerializeAsSkPicture(recording, tracker, bounds, std::move(skp_file),
kDefaultMaxCaptureSizeBytes)) max_capture_size, &serialized_size))
return mojom::PaintPreviewStatus::kCaptureFailed; return mojom::PaintPreviewStatus::kCaptureFailed;
BuildResponse(tracker, response); BuildResponse(tracker, response);
response->serialized_size = serialized_size;
return mojom::PaintPreviewStatus::kOk; return mojom::PaintPreviewStatus::kOk;
} }
...@@ -166,7 +166,8 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( ...@@ -166,7 +166,8 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal(
// TODO(crbug/1011896): Determine if making this async would be beneficial. // TODO(crbug/1011896): Determine if making this async would be beneficial.
*status = FinishRecording(recorder.finishRecordingAsPicture(), bounds, *status = FinishRecording(recorder.finishRecordingAsPicture(), bounds,
&tracker, std::move(params->file), response); &tracker, std::move(params->file),
params->max_capture_size, response);
} }
} // namespace paint_preview } // namespace paint_preview
...@@ -32,7 +32,8 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record, ...@@ -32,7 +32,8 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
PaintPreviewTracker* tracker, PaintPreviewTracker* tracker,
const gfx::Rect& dimensions, const gfx::Rect& dimensions,
base::File file, base::File file,
size_t max_size) { size_t max_size,
size_t* serialized_size) {
if (!file.IsValid()) if (!file.IsValid())
return false; return false;
...@@ -54,6 +55,8 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record, ...@@ -54,6 +55,8 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
skp->serialize(&stream, &serial_procs); skp->serialize(&stream, &serial_procs);
stream.flush(); stream.flush();
stream.Close(); stream.Close();
DCHECK(serialized_size);
*serialized_size = stream.ActualBytesWritten();
return !stream.DidWriteFail(); return !stream.DidWriteFail();
} }
......
...@@ -26,15 +26,17 @@ void ParseGlyphs(const cc::PaintOpBuffer* buffer, PaintPreviewTracker* tracker); ...@@ -26,15 +26,17 @@ void ParseGlyphs(const cc::PaintOpBuffer* buffer, PaintPreviewTracker* tracker);
// Serializes |record| to |file| as an SkPicture of size |dimensions|. |tracker| // Serializes |record| to |file| as an SkPicture of size |dimensions|. |tracker|
// supplies metadata required during serialization. |max_size| is a limit on the // supplies metadata required during serialization. |max_size| is a limit on the
// total serialized size although 0 means the size is unrestricted. If // total serialized size although 0 means the size is unrestricted. If
// |max_size| is exceeded the serialization will fail. // |max_size| is exceeded the serialization will fail. The size of the
// serialized output is set as |serialized_size|.
bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record, bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record,
PaintPreviewTracker* tracker, PaintPreviewTracker* tracker,
const gfx::Rect& dimensions, const gfx::Rect& dimensions,
base::File file, base::File file,
size_t max_size); size_t max_size,
size_t* serialized_size);
// Builds a mojom::PaintPreviewCaptureResponse |response| using the data // Builds a mojom::PaintPreviewCaptureResponse |response| using the data
// contained in |tracker|. Returns true on success. // contained in |tracker|.
// NOTE: |tracker| is effectively const here despite being passed by pointer. // NOTE: |tracker| is effectively const here despite being passed by pointer.
void BuildResponse(PaintPreviewTracker* tracker, void BuildResponse(PaintPreviewTracker* tracker,
mojom::PaintPreviewCaptureResponse* response); mojom::PaintPreviewCaptureResponse* response);
......
...@@ -93,8 +93,9 @@ TEST(PaintPreviewRecorderUtilsTest, TestSerializeAsSkPicture) { ...@@ -93,8 +93,9 @@ TEST(PaintPreviewRecorderUtilsTest, TestSerializeAsSkPicture) {
file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
auto record = recorder.finishRecordingAsPicture(); auto record = recorder.finishRecordingAsPicture();
size_t out_size = 0;
EXPECT_TRUE(SerializeAsSkPicture(record, &tracker, dimensions, EXPECT_TRUE(SerializeAsSkPicture(record, &tracker, dimensions,
std::move(write_file), 0)); std::move(write_file), 0, &out_size));
base::File read_file(file_path, base::File::FLAG_OPEN | base::File read_file(file_path, base::File::FLAG_OPEN |
base::File::FLAG_READ | base::File::FLAG_READ |
base::File::FLAG_EXCLUSIVE_READ); base::File::FLAG_EXCLUSIVE_READ);
...@@ -134,8 +135,10 @@ TEST(PaintPreviewRecorderUtilsTest, TestSerializeAsSkPictureFail) { ...@@ -134,8 +135,10 @@ TEST(PaintPreviewRecorderUtilsTest, TestSerializeAsSkPictureFail) {
file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); file_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
auto record = recorder.finishRecordingAsPicture(); auto record = recorder.finishRecordingAsPicture();
size_t out_size = 2;
EXPECT_FALSE(SerializeAsSkPicture(record, &tracker, dimensions, EXPECT_FALSE(SerializeAsSkPicture(record, &tracker, dimensions,
std::move(write_file), 1)); std::move(write_file), 1, &out_size));
EXPECT_LE(out_size, 1U);
} }
TEST(PaintPreviewRecorderUtilsTest, TestBuildResponse) { TEST(PaintPreviewRecorderUtilsTest, TestBuildResponse) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment