Replace SkPicture(SkStream) constructors with a factory.

src/core/SkPicture.cpp

Index: src/core/SkPicture.cpp
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index ab2faea6b6b4eb38b6e0a9a9ac4ae5338cb46c8b..68de2b835fc9d83f5b0c8ad834a61f4407988911 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -264,41 +264,35 @@ void SkPicture::draw(SkCanvas* surface, SkDrawPictureCallback* callback) {
 
 #include "SkStream.h"
 
-SkPicture::SkPicture(SkStream* stream) {
-    this->initFromStream(stream, NULL, NULL);
-}
-
-SkPicture::SkPicture(SkStream* stream, bool* success, InstallPixelRefProc proc) {
-    this->initFromStream(stream, success, proc);
+bool SkPicture::StreamIsSKP(SkStream* stream, SkPictInfo* info) {
+    if (NULL == stream || NULL == info) {

[reed1, 2013/06/24 22:24:14]

Why do we return false on info==null?

1. crashing is one option
2. just retuning a bool is another

Since we can return false *after* reading info, perhaps it would be better to
always perform that on a stack object, and then only if we're going to return
true do we (after checking for null) copy that into the caller's info.

[scroggo, 2013/06/25 15:36:50]

Done.

+        return false;
+    }
+    if (!stream->read(info, sizeof(SkPictInfo))) {
+        return false;
+    }
+    if (PICTURE_VERSION != info->fVersion) {
+        return false;
+    }
+    return stream->readBool();
 }
 
-void SkPicture::initFromStream(SkStream* stream, bool* success, InstallPixelRefProc proc) {
-    if (success) {
-        *success = false;
-    }
-    fRecord = NULL;
-    fPlayback = NULL;
-    fWidth = fHeight = 0;
+SkPicture::SkPicture(SkPicturePlayback* playback, int width, int height)
+    : fPlayback(playback)
+    , fRecord(NULL)
+    , fWidth(width)
+    , fHeight(height) {}
 
+SkPicture* SkPicture::CreateFromStream(SkStream* stream, InstallPixelRefProc proc) {
     SkPictInfo info;
 
-    if (!stream->read(&info, sizeof(info))) {
-        return;
-    }
-    if (PICTURE_VERSION != info.fVersion) {
-        return;
+    if (!StreamIsSKP(stream, &info)) {
+        return NULL;
     }
 
-    if (stream->readBool()) {
-        fPlayback = SkNEW_ARGS(SkPicturePlayback, (stream, info, proc));
-    }
+    SkPicturePlayback* playback = SkNEW_ARGS(SkPicturePlayback, (stream, info, proc));
 
-    // do this at the end, so that they will be zero if we hit an error.
-    fWidth = info.fWidth;
-    fHeight = info.fHeight;
-    if (success) {
-        *success = true;
-    }
+    return SkNEW_ARGS(SkPicture, (playback, info.fWidth, info.fHeight));
 }
 
 void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const {