Commit de3e4055 authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Paint Preview] Restrict max image size

This CL restricts the maximum image size to avoid memory expensive copy
and encoding operations.

It also limits the total amount of image data to be <= the max on disk
size.

Bug: 1144768
Change-Id: Ic01f68ba6c35cfaa53d499b18052b647917c647b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515179
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: default avatarMehran Mahmoudi <mahmoudi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823741}
parent e9283935
...@@ -92,6 +92,9 @@ class PaintPreviewTracker { ...@@ -92,6 +92,9 @@ class PaintPreviewTracker {
return &picture_context_; return &picture_context_;
} }
TypefaceUsageMap* GetTypefaceUsageMap() { return &typeface_glyph_usage_; } TypefaceUsageMap* GetTypefaceUsageMap() { return &typeface_glyph_usage_; }
ImageSerializationContext* GetImageSerializationContext() {
return &image_context_;
}
// Expose links for serialization to a PaintPreviewFrameProto. // Expose links for serialization to a PaintPreviewFrameProto.
const std::vector<mojom::LinkDataPtr>& GetLinks() { return links_; } const std::vector<mojom::LinkDataPtr>& GetLinks() { return links_; }
...@@ -113,6 +116,7 @@ class PaintPreviewTracker { ...@@ -113,6 +116,7 @@ class PaintPreviewTracker {
std::vector<SkMatrix> states_; std::vector<SkMatrix> states_;
std::vector<mojom::LinkDataPtr> links_; std::vector<mojom::LinkDataPtr> links_;
ImageSerializationContext image_context_;
PictureSerializationContext picture_context_; PictureSerializationContext picture_context_;
TypefaceUsageMap typeface_glyph_usage_; TypefaceUsageMap typeface_glyph_usage_;
base::flat_map<uint32_t, sk_sp<SkPicture>> subframe_pics_; base::flat_map<uint32_t, sk_sp<SkPicture>> subframe_pics_;
......
...@@ -73,6 +73,43 @@ sk_sp<SkData> SerializeTypeface(SkTypeface* typeface, void* ctx) { ...@@ -73,6 +73,43 @@ sk_sp<SkData> SerializeTypeface(SkTypeface* typeface, void* ctx) {
return subset_data; return subset_data;
} }
sk_sp<SkData> SerializeImage(SkImage* image, void* ctx) {
ImageSerializationContext* context =
reinterpret_cast<ImageSerializationContext*>(ctx);
// If there already exists encoded data and it is of a reasonable size use it
// directly.
sk_sp<SkData> encoded_data = image->refEncodedData();
if (!encoded_data) {
const SkImageInfo& image_info = image->imageInfo();
// If encoding the image will result in it exceeding the allowable size,
// effectively delete it by providing no data.
if (context->max_representation_size != 0 &&
image_info.computeMinByteSize() > context->max_representation_size) {
return SkData::MakeEmpty();
}
encoded_data = image->encodeToData();
}
// If encoding fails then no-op.
if (!encoded_data)
return SkData::MakeEmpty();
// Ensure the encoded data fits in the restrictions if they are present.
if ((context->remaining_image_size == std::numeric_limits<uint64_t>::max() ||
context->remaining_image_size >= encoded_data->size()) &&
(context->max_representation_size == 0 ||
encoded_data->size() < context->max_representation_size)) {
if (context->remaining_image_size != std::numeric_limits<uint64_t>::max())
context->remaining_image_size -= encoded_data->size();
return encoded_data;
}
return SkData::MakeEmpty();
}
// Deserializes a clip rect for a subframe within the main SkPicture. These // Deserializes a clip rect for a subframe within the main SkPicture. These
// represent subframes and require special decoding as they are custom data // represent subframes and require special decoding as they are custom data
// rather than a valid SkPicture. // rather than a valid SkPicture.
...@@ -155,14 +192,24 @@ sk_sp<SkPicture> MakeEmptyPicture() { ...@@ -155,14 +192,24 @@ sk_sp<SkPicture> MakeEmptyPicture() {
} }
SkSerialProcs MakeSerialProcs(PictureSerializationContext* picture_ctx, SkSerialProcs MakeSerialProcs(PictureSerializationContext* picture_ctx,
TypefaceSerializationContext* typeface_ctx) { TypefaceSerializationContext* typeface_ctx,
ImageSerializationContext* image_ctx) {
SkSerialProcs procs; SkSerialProcs procs;
procs.fPictureProc = SerializePictureAsRectData; procs.fPictureProc = SerializePictureAsRectData;
procs.fPictureCtx = picture_ctx; procs.fPictureCtx = picture_ctx;
procs.fTypefaceProc = SerializeTypeface; procs.fTypefaceProc = SerializeTypeface;
procs.fTypefaceCtx = typeface_ctx; procs.fTypefaceCtx = typeface_ctx;
// TODO(crbug/1008875): find a consistently smaller and low-memory overhead // TODO(crbug/1008875): find a consistently smaller and low-memory overhead
// image downsampling method to use as fImageProc. // image downsampling method to use as fImageProc.
//
// At present this uses the native representation, but skips serializing if
// loading to a bitmap for encoding might cause an OOM.
if (image_ctx->max_representation_size > 0 ||
image_ctx->remaining_image_size != std::numeric_limits<uint64_t>::max()) {
procs.fImageProc = SerializeImage;
procs.fImageCtx = image_ctx;
}
return procs; return procs;
} }
......
...@@ -52,6 +52,18 @@ struct TypefaceSerializationContext { ...@@ -52,6 +52,18 @@ struct TypefaceSerializationContext {
base::flat_set<SkFontID> finished; // Should be empty on first use. base::flat_set<SkFontID> finished; // Should be empty on first use.
}; };
struct ImageSerializationContext {
// The remaining memory budget for images. This is ignored if the value is the
// max value of uint64_t.
uint64_t remaining_image_size{std::numeric_limits<uint64_t>::max()};
// The maximum size of the representation for serialization. Images
// that are larger than this when encoded or will need to be inflated to a
// bitmap larger than this are skipped to avoid OOMs. If this value is 0 image
// procs are skipped and the default behavior is used.
uint64_t max_representation_size{0};
};
// Maps a content ID to a clip rect. // Maps a content ID to a clip rect.
using DeserializationContext = base::flat_map<uint32_t, gfx::Rect>; using DeserializationContext = base::flat_map<uint32_t, gfx::Rect>;
...@@ -81,7 +93,8 @@ sk_sp<SkPicture> MakeEmptyPicture(); ...@@ -81,7 +93,8 @@ sk_sp<SkPicture> MakeEmptyPicture();
// Creates a SkSerialProcs object. The object *does not* copy |picture_ctx| or // Creates a SkSerialProcs object. The object *does not* copy |picture_ctx| or
// |typeface_ctx| so they must outlive the use of the returned object. // |typeface_ctx| so they must outlive the use of the returned object.
SkSerialProcs MakeSerialProcs(PictureSerializationContext* picture_ctx, SkSerialProcs MakeSerialProcs(PictureSerializationContext* picture_ctx,
TypefaceSerializationContext* typeface_ctx); TypefaceSerializationContext* typeface_ctx,
ImageSerializationContext* image_ctx);
// Creates a SkDeserialProcs object. The object *does not* copy |ctx| so |ctx| // Creates a SkDeserialProcs object. The object *does not* copy |ctx| so |ctx|
// must outlive the use of the returned object. |ctx| will be filled as pictures // must outlive the use of the returned object. |ctx| will be filled as pictures
......
...@@ -5,7 +5,12 @@ ...@@ -5,7 +5,12 @@
#include "components/paint_preview/common/serial_utils.h" #include "components/paint_preview/common/serial_utils.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkColor.h"
#include "third_party/skia/include/core/SkPaint.h"
#include "third_party/skia/include/core/SkPicture.h" #include "third_party/skia/include/core/SkPicture.h"
#include "third_party/skia/include/core/SkPictureRecorder.h"
#include "third_party/skia/include/core/SkRect.h"
#include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/skia/include/core/SkRefCnt.h"
#include "third_party/skia/include/core/SkSerialProcs.h" #include "third_party/skia/include/core/SkSerialProcs.h"
#include "third_party/skia/include/core/SkTypeface.h" #include "third_party/skia/include/core/SkTypeface.h"
...@@ -35,10 +40,13 @@ TEST(PaintPreviewSerialUtils, TestTransformedPictureProcs) { ...@@ -35,10 +40,13 @@ TEST(PaintPreviewSerialUtils, TestTransformedPictureProcs) {
TypefaceUsageMap usage_map; TypefaceUsageMap usage_map;
TypefaceSerializationContext typeface_ctx(&usage_map); TypefaceSerializationContext typeface_ctx(&usage_map);
ImageSerializationContext ictx;
SkSerialProcs serial_procs = MakeSerialProcs(&picture_ctx, &typeface_ctx); SkSerialProcs serial_procs =
MakeSerialProcs(&picture_ctx, &typeface_ctx, &ictx);
EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx); EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx);
EXPECT_EQ(serial_procs.fTypefaceCtx, &typeface_ctx); EXPECT_EQ(serial_procs.fTypefaceCtx, &typeface_ctx);
EXPECT_EQ(serial_procs.fImageCtx, nullptr);
DeserializationContext deserial_ctx; DeserializationContext deserial_ctx;
SkDeserialProcs deserial_procs = MakeDeserialProcs(&deserial_ctx); SkDeserialProcs deserial_procs = MakeDeserialProcs(&deserial_ctx);
...@@ -63,10 +71,13 @@ TEST(PaintPreviewSerialUtils, TestSerialPictureNotInMap) { ...@@ -63,10 +71,13 @@ TEST(PaintPreviewSerialUtils, TestSerialPictureNotInMap) {
TypefaceUsageMap usage_map; TypefaceUsageMap usage_map;
TypefaceSerializationContext typeface_ctx(&usage_map); TypefaceSerializationContext typeface_ctx(&usage_map);
ImageSerializationContext ictx;
SkSerialProcs serial_procs = MakeSerialProcs(&picture_ctx, &typeface_ctx); SkSerialProcs serial_procs =
MakeSerialProcs(&picture_ctx, &typeface_ctx, &ictx);
EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx); EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx);
EXPECT_EQ(serial_procs.fTypefaceCtx, &typeface_ctx); EXPECT_EQ(serial_procs.fTypefaceCtx, &typeface_ctx);
EXPECT_EQ(serial_procs.fImageCtx, nullptr);
auto pic = MakeEmptyPicture(); auto pic = MakeEmptyPicture();
EXPECT_EQ(serial_procs.fPictureProc(pic.get(), serial_procs.fPictureCtx), EXPECT_EQ(serial_procs.fPictureProc(pic.get(), serial_procs.fPictureCtx),
...@@ -87,10 +98,13 @@ TEST(PaintPreviewSerialUtils, TestSerialTypeface) { ...@@ -87,10 +98,13 @@ TEST(PaintPreviewSerialUtils, TestSerialTypeface) {
usage_map.insert(std::make_pair(typeface->uniqueID(), std::move(usage))) usage_map.insert(std::make_pair(typeface->uniqueID(), std::move(usage)))
.second); .second);
TypefaceSerializationContext typeface_ctx(&usage_map); TypefaceSerializationContext typeface_ctx(&usage_map);
ImageSerializationContext ictx;
SkSerialProcs serial_procs = MakeSerialProcs(&picture_ctx, &typeface_ctx); SkSerialProcs serial_procs =
MakeSerialProcs(&picture_ctx, &typeface_ctx, &ictx);
EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx); EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx);
EXPECT_EQ(serial_procs.fTypefaceCtx, &typeface_ctx); EXPECT_EQ(serial_procs.fTypefaceCtx, &typeface_ctx);
EXPECT_EQ(serial_procs.fImageCtx, nullptr);
EXPECT_NE( EXPECT_NE(
serial_procs.fTypefaceProc(typeface.get(), serial_procs.fTypefaceCtx), serial_procs.fTypefaceProc(typeface.get(), serial_procs.fTypefaceCtx),
...@@ -104,10 +118,13 @@ TEST(PaintPreviewSerialUtils, TestSerialNoTypefaceInMap) { ...@@ -104,10 +118,13 @@ TEST(PaintPreviewSerialUtils, TestSerialNoTypefaceInMap) {
auto typeface = SkTypeface::MakeDefault(); auto typeface = SkTypeface::MakeDefault();
TypefaceUsageMap usage_map; TypefaceUsageMap usage_map;
TypefaceSerializationContext typeface_ctx(&usage_map); TypefaceSerializationContext typeface_ctx(&usage_map);
ImageSerializationContext ictx;
SkSerialProcs serial_procs = MakeSerialProcs(&picture_ctx, &typeface_ctx); SkSerialProcs serial_procs =
MakeSerialProcs(&picture_ctx, &typeface_ctx, &ictx);
EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx); EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx);
EXPECT_EQ(serial_procs.fTypefaceCtx, &typeface_ctx); EXPECT_EQ(serial_procs.fTypefaceCtx, &typeface_ctx);
EXPECT_EQ(serial_procs.fImageCtx, nullptr);
EXPECT_NE( EXPECT_NE(
serial_procs.fTypefaceProc(typeface.get(), serial_procs.fTypefaceCtx), serial_procs.fTypefaceProc(typeface.get(), serial_procs.fTypefaceCtx),
...@@ -119,4 +136,95 @@ TEST(PaintPreviewSerialUtils, TestSerialNoTypefaceInMap) { ...@@ -119,4 +136,95 @@ TEST(PaintPreviewSerialUtils, TestSerialNoTypefaceInMap) {
nullptr); nullptr);
} }
TEST(PaintPreviewSerialUtils, TestImageContextLimitBudget) {
SkBitmap bitmap1;
bitmap1.allocN32Pixels(1, 19);
SkCanvas canvas1(bitmap1);
SkPaint paint;
paint.setStyle(SkPaint::kFill_Style);
paint.setColor(SK_ColorRED);
canvas1.drawRect(SkRect::MakeWH(1, 4), paint);
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(SkRect::MakeWH(40, 40));
canvas->drawBitmap(bitmap1, 0, 0);
canvas->drawBitmap(bitmap1, 0, 0);
canvas->drawBitmap(bitmap1, 0, 0);
auto pic = recorder.finishRecordingAsPicture();
PictureSerializationContext picture_ctx;
TypefaceUsageMap usage_map;
TypefaceSerializationContext typeface_ctx(&usage_map);
ImageSerializationContext ictx;
ictx.remaining_image_size = 200;
SkSerialProcs serial_procs =
MakeSerialProcs(&picture_ctx, &typeface_ctx, &ictx);
EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx);
EXPECT_EQ(serial_procs.fImageCtx, &ictx);
EXPECT_EQ(serial_procs.fImageCtx, &ictx);
sk_sp<SkData> data = pic->serialize(&serial_procs);
EXPECT_NE(data, nullptr);
SkDeserialProcs deserial_procs;
size_t deserialized_images = 0;
deserial_procs.fImageCtx = &deserialized_images;
deserial_procs.fImageProc = [](const void* data, size_t length,
void* ctx) -> sk_sp<SkImage> {
if (length > 0U) {
size_t* images = reinterpret_cast<size_t*>(ctx);
*images += 1;
}
return nullptr;
};
SkPicture::MakeFromData(data.get(), &deserial_procs);
EXPECT_EQ(deserialized_images, 2U);
}
TEST(PaintPreviewSerialUtils, TestImageContextLimitSize) {
SkBitmap bitmap1;
bitmap1.allocN32Pixels(1, 19);
SkCanvas canvas1(bitmap1);
SkPaint paint;
paint.setStyle(SkPaint::kFill_Style);
paint.setColor(SK_ColorRED);
canvas1.drawRect(SkRect::MakeWH(1, 4), paint);
SkBitmap bitmap2;
bitmap2.allocN32Pixels(20, 20);
SkCanvas canvas2(bitmap2);
canvas2.drawRect(SkRect::MakeWH(20, 5), paint);
SkPictureRecorder recorder;
SkCanvas* canvas = recorder.beginRecording(SkRect::MakeWH(40, 40));
canvas->drawBitmap(bitmap1, 0, 0);
canvas->drawBitmap(bitmap2, 0, 0);
auto pic = recorder.finishRecordingAsPicture();
PictureSerializationContext picture_ctx;
TypefaceUsageMap usage_map;
TypefaceSerializationContext typeface_ctx(&usage_map);
ImageSerializationContext ictx;
ictx.max_representation_size = 200;
SkSerialProcs serial_procs =
MakeSerialProcs(&picture_ctx, &typeface_ctx, &ictx);
EXPECT_EQ(serial_procs.fPictureCtx, &picture_ctx);
EXPECT_EQ(serial_procs.fImageCtx, &ictx);
EXPECT_EQ(serial_procs.fImageCtx, &ictx);
sk_sp<SkData> data = pic->serialize(&serial_procs);
EXPECT_NE(data, nullptr);
SkDeserialProcs deserial_procs;
size_t deserialized_images = 0;
deserial_procs.fImageCtx = &deserialized_images;
deserial_procs.fImageProc = [](const void* data, size_t length,
void* ctx) -> sk_sp<SkImage> {
if (length > 0U) {
size_t* images = reinterpret_cast<size_t*>(ctx);
*images += 1;
}
return nullptr;
};
SkPicture::MakeFromData(data.get(), &deserial_procs);
EXPECT_EQ(deserialized_images, 1U);
}
} // namespace paint_preview } // namespace paint_preview
...@@ -25,7 +25,8 @@ bool SerializeSkPicture(sk_sp<const SkPicture> skp, ...@@ -25,7 +25,8 @@ bool SerializeSkPicture(sk_sp<const SkPicture> skp,
SkWStream* out_stream) { SkWStream* out_stream) {
TypefaceSerializationContext typeface_context(tracker->GetTypefaceUsageMap()); TypefaceSerializationContext typeface_context(tracker->GetTypefaceUsageMap());
auto serial_procs = MakeSerialProcs(tracker->GetPictureSerializationContext(), auto serial_procs = MakeSerialProcs(tracker->GetPictureSerializationContext(),
&typeface_context); &typeface_context,
tracker->GetImageSerializationContext());
skp->serialize(out_stream, &serial_procs); skp->serialize(out_stream, &serial_procs);
out_stream->flush(); out_stream->flush();
......
...@@ -84,7 +84,8 @@ bool WriteSkp(sk_sp<SkPicture> skp, ...@@ -84,7 +84,8 @@ bool WriteSkp(sk_sp<SkPicture> skp,
skp_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE)); skp_path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE));
TypefaceUsageMap typeface_map; TypefaceUsageMap typeface_map;
TypefaceSerializationContext tctx(&typeface_map); TypefaceSerializationContext tctx(&typeface_map);
auto procs = MakeSerialProcs(pctx, &tctx); ImageSerializationContext ictx;
auto procs = MakeSerialProcs(pctx, &tctx, &ictx);
skp->serialize(&wstream, &procs); skp->serialize(&wstream, &procs);
wstream.Close(); wstream.Close();
if (wstream.DidWriteFail()) { if (wstream.DidWriteFail()) {
......
...@@ -243,6 +243,9 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( ...@@ -243,6 +243,9 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal(
max_capture_size = base::nullopt; max_capture_size = base::nullopt;
} else { } else {
max_capture_size = params->max_capture_size; max_capture_size = params->max_capture_size;
auto* image_ctx = tracker->GetImageSerializationContext();
image_ctx->remaining_image_size = params->max_capture_size;
image_ctx->max_representation_size = params->max_capture_size;
} }
// This cannot be done async if the recording contains a GPU accelerated // This cannot be done async if the recording contains a GPU accelerated
......
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