Fixing SkPicture serialization

src/core/SkPicturePlayback.cpp

Index: src/core/SkPicturePlayback.cpp
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 08e53fab8c443152ad1fb5b708174b8689aa8109..45f1d3abc71ea8553e1617eea3453ba302723285 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -270,7 +270,7 @@ void SkPicturePlayback::init() {
 }
 
 SkPicturePlayback::~SkPicturePlayback() {
-    fOpData->unref();
+    SkSafeUnref(fOpData);
 
     SkSafeUnref(fBitmaps);
     SkSafeUnref(fPaints);
@@ -612,6 +612,37 @@ bool SkPicturePlayback::parseBufferTag(SkReadBuffer& buffer,
                 fPathHeap.reset(SkNEW_ARGS(SkPathHeap, (buffer)));
             }
             break;
+        case SK_PICT_READER_TAG: {
+            SkAutoMalloc storage(size);
+            if (!buffer.readByteArray(storage.get(), size)) {
+                return false;
+            }
+            SkASSERT(NULL == fOpData);

[Stephen White, 2014/03/11 18:14:43]

It looks like if a picture has two SK_PICT_READER_TAGs, it'll leak one of them
in Release builds. Should we just early-out or abort with a validation failure
instead, in that case? (and maybe add a test for this case somewhere?)

[sugoi1, 2014/03/11 18:54:04]

Done.

+            fOpData = SkData::NewFromMalloc(storage.detach(), size);
+        } break;
+        case SK_PICT_PICTURE_TAG: {
+            fPictureCount = size;

[Stephen White, 2014/03/11 18:14:43]

As above, what happens if a picture has two SK_PICT_PICTURE_TAGs?

[sugoi1, 2014/03/11 18:54:04]

Done.

+            fPictureRefs = SkNEW_ARRAY(SkPicture*, fPictureCount);
+            bool success = true;
+            int i = 0;
+            for ( ; i < fPictureCount; i++) {
+                fPictureRefs[i] = SkPicture::CreateFromBuffer(buffer);
+                if (NULL == fPictureRefs[i]) {
+                    success = false;
+                    break;
+                }
+            }
+            if (!success) {
+                // Delete all of the pictures that were already created (up to but excluding i):
+                for (int j = 0; j < i; j++) {
+                    fPictureRefs[j]->unref();
+                }
+                // Delete the array
+                SkDELETE_ARRAY(fPictureRefs);
+                fPictureCount = 0;
+                return false;
+            }
+        } break;
         default:
             // The tag was invalid.
             return false;