Allow creating a picture from skp to fail.

src/core/SkPicturePlayback.cpp

Index: src/core/SkPicturePlayback.cpp
diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp
index 08097a988ee06f466809b495faf74ebec4aa34fa..4a4a2d8a4337800abb1b13a4b31bc2558084982f 100644
--- a/src/core/SkPicturePlayback.cpp
+++ b/src/core/SkPicturePlayback.cpp
@@ -473,7 +473,7 @@ static uint32_t pictInfoFlagsToReadBufferFlags(uint32_t pictInfoFlags) {
     return rbMask;
 }
 
-void SkPicturePlayback::parseStreamTag(SkStream* stream, const SkPictInfo& info, uint32_t tag,
+bool SkPicturePlayback::parseStreamTag(SkStream* stream, const SkPictInfo& info, uint32_t tag,
                                        size_t size, SkPicture::InstallPixelRefProc proc) {
     /*
      *  By the time we encounter BUFFER_SIZE_TAG, we need to have already seen
@@ -520,14 +520,24 @@ void SkPicturePlayback::parseStreamTag(SkStream* stream, const SkPictInfo& info,
         case PICT_PICTURE_TAG: {
             fPictureCount = size;
             fPictureRefs = SkNEW_ARRAY(SkPicture*, fPictureCount);
-            for (int i = 0; i < fPictureCount; i++) {
+            bool success = true;
+            int i = 0;
+            for ( ; i < fPictureCount; i++) {
                 fPictureRefs[i] = SkPicture::CreateFromStream(stream, proc);
-                // CreateFromStream can only fail if PICTURE_VERSION does not match
-                // (which should never happen from here, since a sub picture will
-                // have the same PICTURE_VERSION as its parent) or if stream->read
-                // returns 0. In the latter case, we have a bug when writing the
-                // picture to begin with, which will be alerted to here.
-                SkASSERT(fPictureRefs[i] != NULL);
+                if (NULL == fPictureRefs[i]) {

[scroggo, 2013/09/26 21:06:28]

This bit is slightly different from a straight revert. The old code was calling
the SkPicture constructor with a pointer to bool for success, while a more
recent change switched to using a factory which can return NULL. So now we
compare against NULL and only need to delete up to i-1 down below.

+                    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;
         case PICT_BUFFER_SIZE_TAG: {
@@ -544,14 +554,17 @@ void SkPicturePlayback::parseStreamTag(SkStream* stream, const SkPictInfo& info,
             while (!buffer.eof()) {
                 tag = buffer.readUInt();
                 size = buffer.readUInt();
-                this->parseBufferTag(buffer, tag, size);
+                if (!this->parseBufferTag(buffer, tag, size)) {
+                    return false;
+                }
             }
             SkDEBUGCODE(haveBuffer = true;)
         } break;
     }
+    return true;    // success
 }
 
-void SkPicturePlayback::parseBufferTag(SkOrderedReadBuffer& buffer,
+bool SkPicturePlayback::parseBufferTag(SkOrderedReadBuffer& buffer,
                                        uint32_t tag, size_t size) {
     switch (tag) {
         case PICT_BITMAP_BUFFER_TAG: {
@@ -586,12 +599,14 @@ void SkPicturePlayback::parseBufferTag(SkOrderedReadBuffer& buffer,
             }
         } break;
     }
+    return true;    // success
 }
 
-SkPicturePlayback::SkPicturePlayback(SkStream* stream, const SkPictInfo& info,
+SkPicturePlayback::SkPicturePlayback(SkStream* stream, const SkPictInfo& info, bool* isValid,
                                      SkPicture::InstallPixelRefProc proc) {
     this->init();
 
+    *isValid = false;   // wait until we're done parsing to mark as true
     for (;;) {
         uint32_t tag = stream->readU32();
         if (PICT_EOF_TAG == tag) {
@@ -599,8 +614,11 @@ SkPicturePlayback::SkPicturePlayback(SkStream* stream, const SkPictInfo& info,
         }
 
         uint32_t size = stream->readU32();
-        this->parseStreamTag(stream, info, tag, size, proc);
+        if (!this->parseStreamTag(stream, info, tag, size, proc)) {
+            return; // we're invalid
+        }
     }
+    *isValid = true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////