Remove SkPicture pointer from SkPicturePlayback

debugger/QT/SkDebuggerGUI.cpp

 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkDebuggerGUI.h"
 #include "SkForceLinking.h"
 #include "SkGraphics.h"
@@ skipping 142 matching lines @@
         QListWidgetItem *item = fListWidget.item(row);
         item->setHidden(fDebugger.isCommandVisible(row) && fDeletesActivated);
     }
 }
 
 // The timed picture playback uses the SkPicturePlayback's profiling stubs
 // to time individual commands. The offsets are needed to map SkPicture
 // 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) {
         fTimes.setCount(deletedCommands.count());
         fTypeTimes.setCount(LAST_DRAWTYPE_ENUM+1);
         this->resetTimes();
     }
 
     void resetTimes() {
         for (int i = 0; i < fTimes.count(); ++i) {
@@ skipping 75 matching lines @@
 class SkTimedPicture : public SkPicture {
 public:
     static SkTimedPicture* CreateTimedPicture(SkStream* stream,
                                               SkPicture::InstallPixelRefProc proc,
                                               const SkTDArray<bool>& deletedCommands) {
         SkPictInfo info;
         if (!InternalOnly_StreamIsSKP(stream, &info)) {
             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(); }
 
     int count() const { return ((SkTimedPicturePlayback*) fPlayback)->count(); }
 
     // return the fraction of the total time this command consumed
     double time(int index) const { return ((SkTimedPicturePlayback*) fPlayback)->time(index); }
 
     const SkTDArray<double>* typeTimes() const { return ((SkTimedPicturePlayback*) fPlayback)->typeTimes(); }
@@ skipping 772 matching lines @@
     }
 
     // NOTE(chudy): Makes first item unselectable.
     QStandardItemModel* model = qobject_cast<QStandardItemModel*>(
             fFilter.model());
     QModelIndex firstIndex = model->index(0, fFilter.modelColumn(),
             fFilter.rootModelIndex());
     QStandardItem* firstItem = model->itemFromIndex(firstIndex);
     firstItem->setSelectable(false);
 }