Double free in ~SkPictureData()

src/core/SkPictureData.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 #include <new>
 #include "SkPictureData.h"
 #include "SkPictureRecord.h"
 #include "SkReadBuffer.h"
@@ skipping 359 matching lines @@
                     tf.reset(SkTypeface::RefDefault());
                 }
                 fTFPlayback.set(i, tf);
             }
         } break;
         case SK_PICT_PICTURE_TAG: {
             fPictureCount = size;
             fPictureRefs = SkNEW_ARRAY(const SkPicture*, fPictureCount);
             bool success = true;
             int i = 0;
             for ( ; i < fPictureCount; i++) {

[mtklein, 2015/07/29 21:09:22]

This seems fine, but I don't know if I'd have written it like this.  I think I'd
have written this:

fPictureRefs = SkNEW_ARRAY(const SkPicture*, size);
fPictureCount = 0;
for (int i = 0; i < size; i++) {
  fPictureRefs[i] = SkPicture::CreateFromStream(stream, proc);
  if (!fPictureRefs[i]) {
     return false;
  }
  fPictureCount++;
}
break;

                 fPictureRefs[i] = SkPicture::CreateFromStream(stream, proc);
                 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;
+                // The destructor unrefs fPictureCount fPictureRefs, and deletes the array.
+                fPictureCount = i;
                 return false;
             }
         } break;
         case SK_PICT_BUFFER_SIZE_TAG: {
             SkAutoMalloc storage(size);
             if (stream->read(storage.get(), size) != size) {
                 return false;
             }
 
             /* Should we use SkValidatingReadBuffer instead? */
@@ skipping 217 matching lines @@
     }
 }
 
 bool SkPictureData::suitableForLayerOptimization() const {
     return fContentInfo.numLayers() > 0;
 }
 #endif
 ///////////////////////////////////////////////////////////////////////////////