Replace SkPicture(SkStream) constructors with a factory.

debugger/QT/SkDebuggerGUI.cpp

Index: debugger/QT/SkDebuggerGUI.cpp
diff --git a/debugger/QT/SkDebuggerGUI.cpp b/debugger/QT/SkDebuggerGUI.cpp
index f5f9bc63312999d1bf753f7334e32b946e4a2048..fa0e7503ae5780be2d728708cd02f749db22601b 100644
--- a/debugger/QT/SkDebuggerGUI.cpp
+++ b/debugger/QT/SkDebuggerGUI.cpp
@@ -237,35 +237,22 @@ private:
 // Wrap SkPicture to allow installation of an SkTimedPicturePlayback object
 class SkTimedPicture : public SkPicture {
 public:
-    explicit SkTimedPicture(SkStream* stream, bool* success, SkPicture::InstallPixelRefProc proc,
-                            const SkTDArray<bool>& deletedCommands) {
-        if (success) {
-            *success = false;
-        }
-        fRecord = NULL;
-        fPlayback = NULL;
-        fWidth = fHeight = 0;
-
+    static SkTimedPicture* CreateTimedPicture(SkStream* stream,
+                                              SkPicture::InstallPixelRefProc proc,
+                                              const SkTDArray<bool>& deletedCommands) {
         SkPictInfo info;
-
-        if (!stream->read(&info, sizeof(info))) {
-            return;
-        }
-        if (SkPicture::PICTURE_VERSION != info.fVersion) {
-            return;
+        if (!StreamIsSKP(stream, &info)) {
+            return NULL;
         }
 
-        if (stream->readBool()) {
-            fPlayback = SkNEW_ARGS(SkTimedPicturePlayback,
-                                   (stream, info, proc, deletedCommands));
-        }
+        SkPicturePlayback* playback = SkNEW_ARGS(SkTimedPicturePlayback,
+                                                 (stream, info, proc, deletedCommands));
 

[robertphillips, 2013/06/24 17:22:09]

Does it make sense for there to be a "(playback, width, height)" flavor of
SkTimedPicture ctor?

[scroggo, 2013/06/24 18:22:19]

Absolutely! Done.

-        // 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;
-        }
+        SkTimedPicture* pic = SkNEW(SkTimedPicture);
+        pic->fPlayback = playback;
+        pic->fWidth = info.fWidth;
+        pic->fHeight = info.fHeight;
+        return pic;
     }
 
     void resetTimes() { ((SkTimedPicturePlayback*) fPlayback)->resetTimes(); }
@@ -280,8 +267,8 @@ public:
     double totTime() const { return ((SkTimedPicturePlayback*) fPlayback)->totTime(); }
 
 private:
-    // disallow default ctor b.c. we don't have a good way to setup the fPlayback ptr
-    SkTimedPicture();
+    // Default ctor is only used by CreateTimedPicture, which sets the necessary fields.
+    SkTimedPicture() : INHERITED() {}
     // disallow the copy ctor - enabling would require copying code from SkPicture
     SkTimedPicture(const SkTimedPicture& src);
 
@@ -338,10 +325,9 @@ void SkDebuggerGUI::actionProfile() {
         return;
     }
 
-    bool success = false;
-    SkTimedPicture picture(&inputStream, &success, &SkImageDecoder::DecodeMemory,
-                           fSkipCommands);
-    if (!success) {
+    SkAutoTUnref<SkTimedPicture> picture(SkTimedPicture::CreateTimedPicture(&inputStream,
+                                         &SkImageDecoder::DecodeMemory, fSkipCommands));
+    if (NULL == picture.get()) {
         return;
     }
 
@@ -377,20 +363,20 @@ void SkDebuggerGUI::actionProfile() {
 
     static const int kNumRepeats = 10;
 
-    run(&picture, renderer, kNumRepeats);
+    run(picture.get(), renderer, kNumRepeats);
 
-    SkASSERT(picture.count() == fListWidget.count());
+    SkASSERT(picture->count() == fListWidget.count());
 
     // extract the individual command times from the SkTimedPlaybackPicture
-    for (int i = 0; i < picture.count(); ++i) {
-        double temp = picture.time(i);
+    for (int i = 0; i < picture->count(); ++i) {
+        double temp = picture->time(i);
 
         QListWidgetItem* item = fListWidget.item(i);
 
         item->setData(Qt::UserRole + 4, 100.0*temp);
     }
 
-    setupOverviewText(picture.typeTimes(), picture.totTime(), kNumRepeats);
+    setupOverviewText(picture->typeTimes(), picture->totTime(), kNumRepeats);
 }
 
 void SkDebuggerGUI::actionCancel() {
@@ -905,12 +891,9 @@ void SkDebuggerGUI::loadPicture(const SkString& fileName) {
     fLoading = true;
     SkStream* stream = SkNEW_ARGS(SkFILEStream, (fileName.c_str()));
 
-    bool success = false;
-
-    SkPicture* picture = SkNEW_ARGS(SkPicture,
-                                    (stream, &success, &SkImageDecoder::DecodeMemory));
+    SkPicture* picture = SkPicture::CreateFromStream(stream);
 
-    if (!success) {
+    if (NULL == picture) {
         QMessageBox::critical(this, "Error loading file", "Couldn't read file, sorry.");
         SkSafeUnref(stream);
         return;