Double free in ~SkPictureData()

Double free in ~SkPictureData()

On subpicture parsing failures we clean up all fPictureRefs entries
*and* delete the array itself.  But the destructor also deletes the
array => double free.

Alternatively, we can set fPictureCount to the number of successfully
parsed pictures such that the destructor handles all the cleanup.

BUG=515228
R=reed@google.com,mtklein@google.com

Committed: https://skia.googlesource.com/skia/+/5479d3b5690c274bb53c78333c7c4d41cd5f9137

[f(malita), 2015-07-29 21:04:25]

We could also just null the pointer after SkDELETE_ARRAY, but this feels DRYer.

[f(malita), 2015-07-29 21:04:35]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-29 21:04:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1264503011/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1264503011/1

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

lgtm, but my version lbtm.

https://codereview.chromium.org/1264503011/diff/1/src/core/SkPictureData.cpp
File src/core/SkPictureData.cpp (right):

https://codereview.chromium.org/1264503011/diff/1/src/core/SkPictureData.cpp#...
src/core/SkPictureData.cpp:380: for ( ; i < fPictureCount; i++) {
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;

[commit-bot: I haz the power, 2015-07-29 21:13:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-29 21:13:23]

Dry run: This issue passed the CQ dry run.

[f(malita), 2015-07-29 21:20:36]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-29 21:20:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1264503011/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1264503011/20001

[f(malita), 2015-07-29 21:21:25]

On 2015/07/29 21:09:22, mtklein wrote:
> lgtm, but my version lbtm.

Thanks, lbtm2. Done.

[mtklein, 2015-07-29 21:22:43]

https://codereview.chromium.org/1264503011/diff/20001/src/core/SkPictureData.cpp
File src/core/SkPictureData.cpp (right):

https://codereview.chromium.org/1264503011/diff/20001/src/core/SkPictureData....
src/core/SkPictureData.cpp:380: if (!fPictureRefs[i]) {
Is it just codereview or is there an extra space of indent starting here?

[f(malita), 2015-07-29 21:24:47]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-29 21:24:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1264503011/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1264503011/40001

[f(malita), 2015-07-29 21:26:08]

https://codereview.chromium.org/1264503011/diff/20001/src/core/SkPictureData.cpp
File src/core/SkPictureData.cpp (right):

https://codereview.chromium.org/1264503011/diff/20001/src/core/SkPictureData....
src/core/SkPictureData.cpp:380: if (!fPictureRefs[i]) {
On 2015/07/29 21:22:43, mtklein wrote:
> Is it just codereview or is there an extra space of indent starting here?

Extra space from me being too lazy to type :P

[mtklein, 2015-07-29 21:27:27]

lgtm

[commit-bot: I haz the power, 2015-07-29 21:33:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-29 21:33:02]

Dry run: This issue passed the CQ dry run.

[f(malita), 2015-07-29 21:39:08]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2015-07-29 21:39:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1264503011/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1264503011/40001

[commit-bot: I haz the power, 2015-07-29 21:40:10]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/5479d3b5690c274bb53c78333c7c4d41cd5f9137