Handle zero-length encoded images gracefully during deserialization

Image encoding may fail during serialization, resulting in zero-length
encoded data in the SKP.

Instead of invalidating the stream (and preventing deserialization of
the whole picture) we can instantiate placeholder images.

BUG=skia:4285
R=reed@google.com,robertphillips@google.com

Review URL: https://codereview.chromium.org/1308273011

diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h
index 774b5424428d2684cbfaa7359090148e4fa09b46..61a584b61d9bd365847713a95b0e0c85a2126b9f 100644 (file)
--- a/include/core/SkPicture.h
+++ b/include/core/SkPicture.h
@@ -103,9 +103,7 @@ public:
 
     /**
      *  Serialize to a stream. If non NULL, serializer will be used to serialize
-     *  any bitmaps in the picture.
-     *
-     *  TODO: Use serializer to serialize SkImages as well.
+     *  bitmaps and images in the picture.
      */
     void serialize(SkWStream*, SkPixelSerializer* = NULL) const;
 

diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp
index 5089e5d8d6240f7d3542a933ddef3d7c9e688373..9d497f676aba19faf14579eb2e42f05a4c679a6d 100644 (file)
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -5,6 +5,7 @@
  * found in the LICENSE file.
  */
 #include <new>
+#include "SkImageGenerator.h"
 #include "SkPictureData.h"
 #include "SkPictureRecord.h"
 #include "SkReadBuffer.h"
@@ -433,6 +434,20 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
     return true;    // success
 }
 
+namespace {
+
+// This generator intentionally should always fail on all attempts to get its pixels,
+// simulating a bad or empty codec stream.
+class EmptyImageGenerator final : public SkImageGenerator {
+public:
+    EmptyImageGenerator(const SkImageInfo& info) : INHERITED(info) { }
+
+private:
+    typedef SkImageGenerator INHERITED;
+};
+
+} // anonymous namespace
+
 static const SkImage* create_image_from_buffer(SkReadBuffer& buffer) {
     int width = buffer.read32();
     int height = buffer.read32();
@@ -442,9 +457,15 @@ static const SkImage* create_image_from_buffer(SkReadBuffer& buffer) {
     }
 
     SkAutoTUnref<SkData> encoded(buffer.readByteArrayAsData());
+    if (encoded->size() == 0) {
+        // The image could not be encoded at serialization time - return an empty placeholder.
+        return SkImage::NewFromGenerator(
+            new EmptyImageGenerator(SkImageInfo::MakeN32Premul(width, height)));
+    }
+
     int originX = buffer.read32();
     int originY = buffer.read32();
-    if (0 == encoded->size() || originX < 0 || originY < 0) {
+    if (originX < 0 || originY < 0) {
         buffer.validate(false);
         return nullptr;
     }

diff --git a/src/core/SkWriteBuffer.cpp b/src/core/SkWriteBuffer.cpp
index ac3d678dbc011a70b9c38cdd91ce4199f633750c..1dfe0b3972c62b6b8e68cc6f6d92dc8782935b97 100644 (file)
--- a/src/core/SkWriteBuffer.cpp
+++ b/src/core/SkWriteBuffer.cpp
@@ -223,7 +223,7 @@ void SkWriteBuffer::writeImage(const SkImage* image) {
     this->writeInt(image->height());
 
     SkAutoTUnref<SkData> encoded(image->encode(this->getPixelSerializer()));
-    if (encoded) {
+    if (encoded && encoded->size() > 0) {
         write_encoded_bitmap(this, encoded, SkIPoint::Make(0, 0));
         return;
     }

diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp
index d271966979a036325841658011bf5ca18e706edc..30e477ab53edbcb5508c8200a837ad1feaf2902e 100644 (file)
--- a/tests/ImageTest.cpp
+++ b/tests/ImageTest.cpp
@@ -10,8 +10,11 @@
 #include "SkDevice.h"
 #include "SkImageEncoder.h"
 #include "SkImage_Base.h"
+#include "SkPicture.h"
+#include "SkPictureRecorder.h"
 #include "SkPixelSerializer.h"
 #include "SkRRect.h"
+#include "SkStream.h"
 #include "SkSurface.h"
 #include "SkUtils.h"
 #include "Test.h"
@@ -110,31 +113,75 @@ namespace {
 const char* kSerializedData = "serialized";
 
 class MockSerializer : public SkPixelSerializer {
+public:
+    MockSerializer(SkData* (*func)()) : fFunc(func), fDidEncode(false) { }
+
+    bool didEncode() const { return fDidEncode; }
+
 protected:
     bool onUseEncodedData(const void*, size_t) override {
         return false;
     }
 
     SkData* onEncodePixels(const SkImageInfo&, const void*, size_t) override {
-        return SkData::NewWithCString(kSerializedData);
+        fDidEncode = true;
+        return fFunc();
     }
+
+private:
+    SkData* (*fFunc)();
+    bool fDidEncode;
+
+    typedef SkPixelSerializer INHERITED;
 };
 
 } // anonymous namespace
 
 // Test that SkImage encoding observes custom pixel serializers.
 DEF_TEST(Image_Encode_Serializer, reporter) {
-    MockSerializer serializer;
+    MockSerializer serializer([]() -> SkData* { return SkData::NewWithCString(kSerializedData); });
     const SkIRect ir = SkIRect::MakeXYWH(5, 5, 10, 10);
     SkAutoTUnref<SkImage> image(make_image(nullptr, 20, 20, ir));
     SkAutoTUnref<SkData> encoded(image->encode(&serializer));
     SkAutoTUnref<SkData> reference(SkData::NewWithCString(kSerializedData));
 
+    REPORTER_ASSERT(reporter, serializer.didEncode());
     REPORTER_ASSERT(reporter, encoded);
     REPORTER_ASSERT(reporter, encoded->size() > 0);
     REPORTER_ASSERT(reporter, encoded->equals(reference));
 }
 
+// Test that image encoding failures do not break picture serialization/deserialization.
+DEF_TEST(Image_Serialize_Encoding_Failure, reporter) {
+    SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(100, 100));
+    surface->getCanvas()->clear(SK_ColorGREEN);
+    SkAutoTUnref<SkImage> image(surface->newImageSnapshot());
+    REPORTER_ASSERT(reporter, image);
+
+    SkPictureRecorder recorder;
+    SkCanvas* canvas = recorder.beginRecording(100, 100);
+    canvas->drawImage(image, 0, 0);
+    SkAutoTUnref<SkPicture> picture(recorder.endRecording());
+    REPORTER_ASSERT(reporter, picture);
+    REPORTER_ASSERT(reporter, picture->approximateOpCount() > 0);
+
+    MockSerializer emptySerializer([]() -> SkData* { return SkData::NewEmpty(); });
+    MockSerializer nullSerializer([]() -> SkData* { return nullptr; });
+    MockSerializer* serializers[] = { &emptySerializer, &nullSerializer };
+
+    for (size_t i = 0; i < SK_ARRAY_COUNT(serializers); ++i) {
+        SkDynamicMemoryWStream wstream;
+        REPORTER_ASSERT(reporter, !serializers[i]->didEncode());
+        picture->serialize(&wstream, serializers[i]);
+        REPORTER_ASSERT(reporter, serializers[i]->didEncode());
+
+        SkAutoTDelete<SkStream> rstream(wstream.detachAsStream());
+        SkAutoTUnref<SkPicture> deserialized(SkPicture::CreateFromStream(rstream));
+        REPORTER_ASSERT(reporter, deserialized);
+        REPORTER_ASSERT(reporter, deserialized->approximateOpCount() > 0);
+    }
+}
+
 DEF_TEST(Image_NewRasterCopy, reporter) {
     const SkPMColor red =   SkPackARGB32(0xFF, 0xFF, 0, 0);
     const SkPMColor green = SkPackARGB32(0xFF, 0, 0xFF, 0);