Remove SkPicture pointer from SkPicturePlayback

debugger/QT/SkDebuggerGUI.cpp

Index: debugger/QT/SkDebuggerGUI.cpp
diff --git a/debugger/QT/SkDebuggerGUI.cpp b/debugger/QT/SkDebuggerGUI.cpp
index 0f816eec7cebd55d8b726e903f180158b9642437..1873afca678d972cf7f68e9ccc125916041736c9 100644
--- a/debugger/QT/SkDebuggerGUI.cpp
+++ b/debugger/QT/SkDebuggerGUI.cpp
@@ -160,23 +160,21 @@ void SkDebuggerGUI::showDeletes() {
 // offsets to individual commands.
 class SkTimedPicturePlayback : public SkPicturePlayback {
 public:
-    static SkTimedPicturePlayback* CreateFromStream(SkPicture* picture,
-                                                    SkStream* stream, const SkPictInfo& info,
+    static SkTimedPicturePlayback* CreateFromStream(SkStream* stream, const SkPictInfo& info,
                                                     SkPicture::InstallPixelRefProc proc,
                                                     const SkTDArray<bool>& deletedCommands) {
         // Mimics SkPicturePlayback::CreateFromStream
         SkAutoTDelete<SkTimedPicturePlayback> playback(SkNEW_ARGS(SkTimedPicturePlayback,
-                                                               (picture, deletedCommands, info)));
-        if (!playback->parseStream(picture, stream, proc)) {
+                                                               (deletedCommands, info)));
+        if (!playback->parseStream(stream, proc)) {
             return NULL; // we're invalid
         }
         return playback.detach();
     }
 
-    SkTimedPicturePlayback(SkPicture* picture,
-                           const SkTDArray<bool>& deletedCommands,
+    SkTimedPicturePlayback(const SkTDArray<bool>& deletedCommands,
                            const SkPictInfo& info)
-        : INHERITED(picture, info)
+        : INHERITED(info)
         , fSkipCommands(deletedCommands)
         , fTot(0.0)
         , fCurCommand(0) {
@@ -272,21 +270,20 @@ public:
             return NULL;
         }
 
-        SkTimedPicture* newPict = SkNEW_ARGS(SkTimedPicture, (NULL, info.fWidth, info.fHeight));
         // Check to see if there is a playback to recreate.
         if (stream->readBool()) {
             SkTimedPicturePlayback* playback = SkTimedPicturePlayback::CreateFromStream(
-                                                                newPict, stream,
+                                                                stream,
                                                                 info, proc,
                                                                 deletedCommands);
             if (NULL == playback) {
-                SkDELETE(newPict);
                 return NULL;
             }
-            newPict->fPlayback = playback;
+
+            return SkNEW_ARGS(SkTimedPicture, (playback, info.fWidth, info.fHeight));
         }
 
-        return newPict;
+        return NULL;

[mtklein, 2014/06/11 21:12:58]

This used to return a SkTimedPicture(NULL, w, h) if readBool failed.  Does that
matter?

[robertphillips, 2014/06/12 12:27:29]

My thinking on this is that the caller already has be prepared to deal with a
NULL return value. In the case where there is no SkPicturePlayback backing the
SkPicture it (the picture) is pretty useless. If we were to continue with the
current SkPicture backend, I think I would try for a world where an SkPicture
always has an SkPicturePlayback or doesn't exist at all.

     }
 
     void resetTimes() { ((SkTimedPicturePlayback*) fPlayback)->resetTimes(); }