AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Scrub SkPictureData/SkPictureRecord

Browse Source 
- replace manual ref fiddling with smart types
- remove unused SkPictureData::fBitmapImageRefs
- remove the preallocation in new_array_from_buffer (to prevent
  fuzzer OOMs)

Bug: skia:7937
Change-Id: I50f49fa8e594a138ea09c22f7bf73cf57ec006ff
Reviewed-on: https://skia-review.googlesource.com/128309
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
This commit is contained in:
Florin Malita 2018-05-15 14:57:12 -04:00 committed by Skia Commit-Bot
parent 724afe8b2d
commit 8fd15d8c7e
4 changed files with 116 additions and 237 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

231
src/core/SkPictureData.cpp
View File

@ -9,6 +9,7 @@
#include "SkAutoMalloc.h"
#include "SkImageGenerator.h"
#include "SkMakeUnique.h"
#include "SkPictureData.h"
#include "SkPictureRecord.h"
#include "SkReadBuffer.h"
@ -25,9 +26,7 @@ template <typename T> int SafeCount(const T* obj) {
}
SkPictureData::SkPictureData(const SkPictInfo& info)
: fInfo(info) {
this->init();
}
: fInfo(info) {}
void SkPictureData::initForPlayback() const {
// ensure that the paths bounds are pre-computed
@ -38,9 +37,12 @@ void SkPictureData::initForPlayback() const {
SkPictureData::SkPictureData(const SkPictureRecord& record,
const SkPictInfo& info)
: fInfo(info) {
this->init();
: fPictures(record.getPictures())
, fDrawables(record.getDrawables())
, fTextBlobs(record.getTextBlobs())
, fVertices(record.getVertices())
, fImages(record.getImages())
, fInfo(info) {
fOpData = record.opData();
@ -54,102 +56,6 @@ SkPictureData::SkPictureData(const SkPictureRecord& record,
});
this->initForPlayback();
const SkTDArray<const SkPicture* >& pictures = record.getPictureRefs();
fPictureCount = pictures.count();
if (fPictureCount > 0) {
fPictureRefs = new const SkPicture* [fPictureCount];
for (int i = 0; i < fPictureCount; i++) {
fPictureRefs[i] = pictures[i];
fPictureRefs[i]->ref();
}
}
const SkTDArray<SkDrawable* >& drawables = record.getDrawableRefs();
fDrawableCount = drawables.count();
if (fDrawableCount > 0) {
fDrawableRefs = new SkDrawable* [fDrawableCount];
for (int i = 0; i < fDrawableCount; i++) {
fDrawableRefs[i] = drawables[i];
fDrawableRefs[i]->ref();
}
}
// templatize to consolidate with similar picture logic?
const SkTDArray<const SkTextBlob*>& blobs = record.getTextBlobRefs();
fTextBlobCount = blobs.count();
if (fTextBlobCount > 0) {
fTextBlobRefs = new const SkTextBlob* [fTextBlobCount];
for (int i = 0; i < fTextBlobCount; ++i) {
fTextBlobRefs[i] = SkRef(blobs[i]);
}
}
const SkTDArray<const SkVertices*>& verts = record.getVerticesRefs();
fVerticesCount = verts.count();
if (fVerticesCount > 0) {
fVerticesRefs = new const SkVertices* [fVerticesCount];
for (int i = 0; i < fVerticesCount; ++i) {
fVerticesRefs[i] = SkRef(verts[i]);
}
}
const SkTDArray<const SkImage*>& imgs = record.getImageRefs();
fImageCount = imgs.count();
if (fImageCount > 0) {
fImageRefs = new const SkImage* [fImageCount];
for (int i = 0; i < fImageCount; ++i) {
fImageRefs[i] = SkRef(imgs[i]);
}
}
}
void SkPictureData::init() {
fPictureRefs = nullptr;
fPictureCount = 0;
fDrawableRefs = nullptr;
fDrawableCount = 0;
fTextBlobRefs = nullptr;
fTextBlobCount = 0;
fVerticesRefs = nullptr;
fVerticesCount = 0;
fImageRefs = nullptr;
fImageCount = 0;
fBitmapImageCount = 0;
fBitmapImageRefs = nullptr;
fFactoryPlayback = nullptr;
}
SkPictureData::~SkPictureData() {
for (int i = 0; i < fPictureCount; i++) {
fPictureRefs[i]->unref();
}
delete[] fPictureRefs;
for (int i = 0; i < fDrawableCount; i++) {
fDrawableRefs[i]->unref();
}
if (fDrawableCount > 0) {
SkASSERT(fDrawableRefs);
delete[] fDrawableRefs;
}
for (int i = 0; i < fTextBlobCount; i++) {
fTextBlobRefs[i]->unref();
}
delete[] fTextBlobRefs;
for (int i = 0; i < fVerticesCount; i++) {
fVerticesRefs[i]->unref();
}
delete[] fVerticesRefs;
for (int i = 0; i < fImageCount; i++) {
fImageRefs[i]->unref();
}
delete[] fImageRefs;
delete fFactoryPlayback;
}
///////////////////////////////////////////////////////////////////////////////
@ -244,24 +150,24 @@ void SkPictureData::flattenToBuffer(SkWriteBuffer& buffer) const {
}
}
if (fTextBlobCount > 0) {
write_tag_size(buffer, SK_PICT_TEXTBLOB_BUFFER_TAG, fTextBlobCount);
for (i = 0; i < fTextBlobCount; ++i) {
fTextBlobRefs[i]->flatten(buffer);
if (!fTextBlobs.empty()) {
write_tag_size(buffer, SK_PICT_TEXTBLOB_BUFFER_TAG, fTextBlobs.count());
for (const auto& blob : fTextBlobs) {
blob->flatten(buffer);
}
}
if (fVerticesCount > 0) {
write_tag_size(buffer, SK_PICT_VERTICES_BUFFER_TAG, fVerticesCount);
for (i = 0; i < fVerticesCount; ++i) {
buffer.writeDataAsByteArray(fVerticesRefs[i]->encode().get());
if (!fVertices.empty()) {
write_tag_size(buffer, SK_PICT_VERTICES_BUFFER_TAG, fVertices.count());
for (const auto& vert : fVertices) {
buffer.writeDataAsByteArray(vert->encode().get());
}
}
if (fImageCount > 0) {
write_tag_size(buffer, SK_PICT_IMAGE_BUFFER_TAG, fImageCount);
for (i = 0; i < fImageCount; ++i) {
buffer.writeImage(fImageRefs[i]);
if (!fImages.empty()) {
write_tag_size(buffer, SK_PICT_IMAGE_BUFFER_TAG, fImages.count());
for (const auto& img : fImages) {
buffer.writeImage(img.get());
}
}
}
@ -293,8 +199,8 @@ void SkPictureData::serialize(SkWStream* stream, const SkSerialProcs& procs,
bool write(const void*, size_t size) override { fBytesWritten += size; return true; }
size_t bytesWritten() const override { return fBytesWritten; }
} devnull;
for (int i = 0; i < fPictureCount; i++) {
fPictureRefs[i]->serialize(&devnull, nullptr, typefaceSet);
for (const auto& pic : fPictures) {
pic->serialize(&devnull, nullptr, typefaceSet);
}
// We need to write factories before we write the buffer.
@ -309,10 +215,10 @@ void SkPictureData::serialize(SkWStream* stream, const SkSerialProcs& procs,
buffer.writeToStream(stream);
// Write sub-pictures by calling serialize again.
if (fPictureCount > 0) {
write_tag_size(stream, SK_PICT_PICTURE_TAG, fPictureCount);
for (int i = 0; i < fPictureCount; i++) {
fPictureRefs[i]->serialize(stream, &procs, typefaceSet);
if (!fPictures.empty()) {
write_tag_size(stream, SK_PICT_PICTURE_TAG, fPictures.count());
for (const auto& pic : fPictures) {
pic->serialize(stream, &procs, typefaceSet);
}
}
@ -323,17 +229,17 @@ void SkPictureData::flatten(SkWriteBuffer& buffer) const {
write_tag_size(buffer, SK_PICT_READER_TAG, fOpData->size());
buffer.writeByteArray(fOpData->bytes(), fOpData->size());
if (fPictureCount > 0) {
write_tag_size(buffer, SK_PICT_PICTURE_TAG, fPictureCount);
for (int i = 0; i < fPictureCount; i++) {
fPictureRefs[i]->flatten(buffer);
if (!fPictures.empty()) {
write_tag_size(buffer, SK_PICT_PICTURE_TAG, fPictures.count());
for (const auto& pic : fPictures) {
pic->flatten(buffer);
}
}
if (fDrawableCount > 0) {
write_tag_size(buffer, SK_PICT_DRAWABLE_TAG, fDrawableCount);
for (int i = 0; i < fDrawableCount; i++) {
buffer.writeFlattenable(fDrawableRefs[i]);
if (!fDrawables.empty()) {
write_tag_size(buffer, SK_PICT_DRAWABLE_TAG, fDrawables.count());
for (const auto& draw : fDrawables) {
buffer.writeFlattenable(draw.get());
}
}
@ -371,7 +277,7 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
case SK_PICT_FACTORY_TAG: {
SkASSERT(!haveBuffer);
size = stream->readU32();
fFactoryPlayback = new SkFactoryPlayback(size);
fFactoryPlayback = skstd::make_unique<SkFactoryPlayback>(size);
for (size_t i = 0; i < size; i++) {
SkString str;
const size_t len = stream->readPackedUInt();
@ -397,14 +303,15 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
}
} break;
case SK_PICT_PICTURE_TAG: {
fPictureCount = 0;
fPictureRefs = new const SkPicture* [size];
SkASSERT(fPictures.empty());
fPictures.reserve(SkToInt(size));
for (uint32_t i = 0; i < size; i++) {
fPictureRefs[i] = SkPicture::MakeFromStream(stream, &procs, topLevelTFPlayback).release();
if (!fPictureRefs[i]) {
auto pic = SkPicture::MakeFromStream(stream, &procs, topLevelTFPlayback);
if (!pic) {
return false;
}
fPictureCount++;
fPictures.push_back(std::move(pic));
}
} break;
case SK_PICT_BUFFER_SIZE_TAG: {
@ -456,41 +363,28 @@ static sk_sp<SkDrawable> create_drawable_from_buffer(SkReadBuffer& buffer) {
return sk_sp<SkDrawable>((SkDrawable*)buffer.readFlattenable(SkFlattenable::kSkDrawable_Type));
}
template <typename T>
// We need two types 'cause SkDrawable is const-variant.
template <typename T, typename U>
bool new_array_from_buffer(SkReadBuffer& buffer, uint32_t inCount,
const T*** array, int* outCount, sk_sp<T> (*factory)(SkReadBuffer&)) {
if (!buffer.validate((0 == *outCount) && (nullptr == *array))) {
SkTArray<sk_sp<T>>& array, sk_sp<U> (*factory)(SkReadBuffer&)) {
if (!buffer.validate(array.empty() && SkTFitsIn<int>(inCount))) {
return false;
}
if (0 == inCount) {
return true;
}
if (!buffer.validate(SkTFitsIn<int>(inCount))) {
return false;
for (uint32_t i = 0; i < inCount; ++i) {
auto obj = factory(buffer);
if (!buffer.validate(obj)) {
array.reset();
return false;
}
array.push_back(std::move(obj));
}
*outCount = inCount;
*array = new const T* [*outCount];
bool success = true;
int i = 0;
for (; i < *outCount; i++) {
(*array)[i] = factory(buffer).release();
if (nullptr == (*array)[i]) {
success = false;
break;
}
}
if (!success) {
// Delete all of the blobs that were already created (up to but excluding i):
for (int j = 0; j < i; j++) {
(*array)[j]->unref();
}
// Delete the array
delete[] * array;
*array = nullptr;
*outCount = 0;
return buffer.validate(false);
}
return true;
}
@ -520,16 +414,13 @@ void SkPictureData::parseBufferTag(SkReadBuffer& buffer, uint32_t tag, uint32_t
}
} break;
case SK_PICT_TEXTBLOB_BUFFER_TAG:
new_array_from_buffer(buffer, size, &fTextBlobRefs, &fTextBlobCount,
SkTextBlob::MakeFromBuffer);
new_array_from_buffer(buffer, size, fTextBlobs, SkTextBlob::MakeFromBuffer);
break;
case SK_PICT_VERTICES_BUFFER_TAG:
new_array_from_buffer(buffer, size, &fVerticesRefs, &fVerticesCount,
create_vertices_from_buffer);
new_array_from_buffer(buffer, size, fVertices, create_vertices_from_buffer);
break;
case SK_PICT_IMAGE_BUFFER_TAG:
new_array_from_buffer(buffer, size, &fImageRefs, &fImageCount,
create_image_from_buffer);
new_array_from_buffer(buffer, size, fImages, create_image_from_buffer);
break;
case SK_PICT_READER_TAG: {
auto data(SkData::MakeUninitialized(size));
@ -541,12 +432,10 @@ void SkPictureData::parseBufferTag(SkReadBuffer& buffer, uint32_t tag, uint32_t
fOpData = std::move(data);
} break;
case SK_PICT_PICTURE_TAG:
new_array_from_buffer(buffer, size, &fPictureRefs, &fPictureCount,
SkPicture::MakeFromBuffer);
new_array_from_buffer(buffer, size, fPictures, SkPicture::MakeFromBuffer);
break;
case SK_PICT_DRAWABLE_TAG:
new_array_from_buffer(buffer, size, (const SkDrawable***)&fDrawableRefs,
&fDrawableCount, create_drawable_from_buffer);
new_array_from_buffer(buffer, size, fDrawables, create_drawable_from_buffer);
break;
default:
buffer.validate(false); // The tag was invalid.

43
src/core/SkPictureData.h
View File

@ -12,6 +12,9 @@
#include "SkDrawable.h"
#include "SkPicture.h"
#include "SkPictureFlat.h"
#include "SkTArray.h"
#include <memory>
class SkData;
class SkPictureRecord;
@ -65,9 +68,10 @@ public:
// Always write this guy last (with no length field afterwards)
#define SK_PICT_EOF_TAG SkSetFourByteTag('e', 'o', 'f', ' ')
template <typename T> T* read_index_base_1_or_null(SkReadBuffer* reader, int count, T* array[]) {
template <typename T>
T* read_index_base_1_or_null(SkReadBuffer* reader, const SkTArray<sk_sp<T>>& array) {
int index = reader->readInt();
return reader->validate(index > 0 && index <= count) ? array[index - 1] : nullptr;
return reader->validate(index > 0 && index <= array.count()) ? array[index - 1].get() : nullptr;
}
class SkPictureData {
@ -80,8 +84,6 @@ public:
SkTypefacePlayback*);
static SkPictureData* CreateFromBuffer(SkReadBuffer&, const SkPictInfo&);
virtual ~SkPictureData();
void serialize(SkWStream*, const SkSerialProcs&, SkRefCntSet*) const;
void flatten(SkWriteBuffer&) const;
@ -98,7 +100,7 @@ public:
const SkImage* getImage(SkReadBuffer* reader) const {
// images are written base-0, unlike paths, pictures, drawables, etc.
const int index = reader->readInt();
return reader->validateIndex(index, fImageCount) ? fImageRefs[index] : nullptr;
return reader->validateIndex(index, fImages.count()) ? fImages[index].get() : nullptr;
}
const SkPath& getPath(SkReadBuffer* reader) const {
@ -108,11 +110,11 @@ public:
}
const SkPicture* getPicture(SkReadBuffer* reader) const {
return read_index_base_1_or_null(reader, fPictureCount, fPictureRefs);
return read_index_base_1_or_null(reader, fPictures);
}
SkDrawable* getDrawable(SkReadBuffer* reader) const {
return read_index_base_1_or_null(reader, fDrawableCount, fDrawableRefs);
return read_index_base_1_or_null(reader, fDrawables);
}
const SkPaint* getPaint(SkReadBuffer* reader) const {
@ -125,16 +127,14 @@ public:
}
const SkTextBlob* getTextBlob(SkReadBuffer* reader) const {
return read_index_base_1_or_null(reader, fTextBlobCount, fTextBlobRefs);
return read_index_base_1_or_null(reader, fTextBlobs);
}
const SkVertices* getVertices(SkReadBuffer* reader) const {
return read_index_base_1_or_null(reader, fVerticesCount, fVerticesRefs);
return read_index_base_1_or_null(reader, fVertices);
}
private:
void init();
// these help us with reading/writing
// Does not affect ownership of SkStream.
bool parseStreamTag(SkStream*, uint32_t tag, uint32_t size,
@ -150,21 +150,14 @@ private:
const SkPath fEmptyPath;
const SkBitmap fEmptyBitmap;
const SkPicture** fPictureRefs;
int fPictureCount;
SkDrawable** fDrawableRefs;
int fDrawableCount;
const SkTextBlob** fTextBlobRefs;
int fTextBlobCount;
const SkVertices** fVerticesRefs;
int fVerticesCount;
const SkImage** fImageRefs;
int fImageCount;
const SkImage** fBitmapImageRefs;
int fBitmapImageCount;
SkTArray<sk_sp<const SkPicture>> fPictures;
SkTArray<sk_sp<SkDrawable>> fDrawables;
SkTArray<sk_sp<const SkTextBlob>> fTextBlobs;
SkTArray<sk_sp<const SkVertices>> fVertices;
SkTArray<sk_sp<const SkImage>> fImages;
SkTypefacePlayback fTFPlayback;
SkFactoryPlayback* fFactoryPlayback;
SkTypefacePlayback fTFPlayback;
std::unique_ptr<SkFactoryPlayback> fFactoryPlayback;
const SkPictInfo fInfo;

47
src/core/SkPictureRecord.cpp
View File

@ -34,14 +34,6 @@ SkPictureRecord::SkPictureRecord(const SkISize& dimensions, uint32_t flags)
, fInitialSaveCount(kNoInitialSave) {
}
SkPictureRecord::~SkPictureRecord() {
fImageRefs.unrefAll();
fPictureRefs.unrefAll();
fDrawableRefs.unrefAll();
fTextBlobRefs.unrefAll();
fVerticesRefs.unrefAll();
}
///////////////////////////////////////////////////////////////////////////////
void SkPictureRecord::onFlush() {
@ -804,15 +796,28 @@ void SkPictureRecord::onDrawAnnotation(const SkRect& rect, const char key[], SkD
///////////////////////////////////////////////////////////////////////////////
template <typename T> int find_or_append_uniqueID(SkTDArray<const T*>& array, const T* obj) {
// De-duping helper.
template <typename T>
static bool equals(T* a, T* b) { return a->uniqueID() == b->uniqueID(); }
template <>
bool equals(SkDrawable* a, SkDrawable* b) {
// SkDrawable's generationID is not a stable unique identifier.
return a == b;
}
template <typename T>
static int find_or_append(SkTArray<sk_sp<T>>& array, T* obj) {
for (int i = 0; i < array.count(); i++) {
if (array[i]->uniqueID() == obj->uniqueID()) {
if (equals(array[i].get(), obj)) {
return i;
}
}
int i = array.count();
*array.append() = SkRef(obj);
return i;
array.push_back(sk_ref_sp(obj));
return array.count() - 1;
}
sk_sp<SkSurface> SkPictureRecord::onNewSurface(const SkImageInfo& info, const SkSurfaceProps&) {
@ -821,7 +826,7 @@ sk_sp<SkSurface> SkPictureRecord::onNewSurface(const SkImageInfo& info, const Sk
void SkPictureRecord::addImage(const SkImage* image) {
// convention for images is 0-based index
this->addInt(find_or_append_uniqueID(fImageRefs, image));
this->addInt(find_or_append(fImages, image));
}
void SkPictureRecord::addMatrix(const SkMatrix& matrix) {
@ -856,18 +861,12 @@ void SkPictureRecord::addPatch(const SkPoint cubics[12]) {
void SkPictureRecord::addPicture(const SkPicture* picture) {
// follow the convention of recording a 1-based index
this->addInt(find_or_append_uniqueID(fPictureRefs, picture) + 1);
this->addInt(find_or_append(fPictures, picture) + 1);
}
void SkPictureRecord::addDrawable(SkDrawable* drawable) {
int index = fDrawableRefs.find(drawable);
if (index < 0) { // not found
index = fDrawableRefs.count();
*fDrawableRefs.append() = drawable;
drawable->ref();
}
// follow the convention of recording a 1-based index
this->addInt(index + 1);
this->addInt(find_or_append(fDrawables, drawable) + 1);
}
void SkPictureRecord::addPoint(const SkPoint& point) {
@ -918,12 +917,12 @@ void SkPictureRecord::addText(const void* text, size_t byteLength) {
void SkPictureRecord::addTextBlob(const SkTextBlob* blob) {
// follow the convention of recording a 1-based index
this->addInt(find_or_append_uniqueID(fTextBlobRefs, blob) + 1);
this->addInt(find_or_append(fTextBlobs, blob) + 1);
}
void SkPictureRecord::addVertices(const SkVertices* vertices) {
// follow the convention of recording a 1-based index
this->addInt(find_or_append_uniqueID(fVerticesRefs, vertices) + 1);
this->addInt(find_or_append(fVertices, vertices) + 1);
}
///////////////////////////////////////////////////////////////////////////////

32
src/core/SkPictureRecord.h
View File

@ -31,26 +31,25 @@
class SkPictureRecord : public SkCanvasVirtualEnforcer<SkCanvas> {
public:
SkPictureRecord(const SkISize& dimensions, uint32_t recordFlags);
~SkPictureRecord() override;
const SkTDArray<const SkPicture* >& getPictureRefs() const {
return fPictureRefs;
const SkTArray<sk_sp<const SkPicture>>& getPictures() const {
return fPictures;
}
const SkTDArray<SkDrawable* >& getDrawableRefs() const {
return fDrawableRefs;
const SkTArray<sk_sp<SkDrawable>>& getDrawables() const {
return fDrawables;
}
const SkTDArray<const SkTextBlob* >& getTextBlobRefs() const {
return fTextBlobRefs;
const SkTArray<sk_sp<const SkTextBlob>>& getTextBlobs() const {
return fTextBlobs;
}
const SkTDArray<const SkVertices* >& getVerticesRefs() const {
return fVerticesRefs;
const SkTArray<sk_sp<const SkVertices>>& getVertices() const {
return fVertices;
}
const SkTDArray<const SkImage* >& getImageRefs() const {
return fImageRefs;
const SkTArray<sk_sp<const SkImage>>& getImages() const {
return fImages;
}
sk_sp<SkData> opData() const {
@ -251,12 +250,11 @@ private:
SkWriter32 fWriter;
// we ref each item in these arrays
SkTDArray<const SkImage*> fImageRefs;
SkTDArray<const SkPicture*> fPictureRefs;
SkTDArray<SkDrawable*> fDrawableRefs;
SkTDArray<const SkTextBlob*> fTextBlobRefs;
SkTDArray<const SkVertices*> fVerticesRefs;
SkTArray<sk_sp<const SkImage>> fImages;
SkTArray<sk_sp<const SkPicture>> fPictures;
SkTArray<sk_sp<SkDrawable>> fDrawables;
SkTArray<sk_sp<const SkTextBlob>> fTextBlobs;
SkTArray<sk_sp<const SkVertices>> fVertices;
uint32_t fRecordFlags;
int fInitialSaveCount;