Fixing leaky handling of SkImage in SkDeferredCanvas.

Long lived SkImageHeap objects currently accumulate refs indefinitely.
This leads to massive memory leaks in the gpu-accelerated 2D canvas
code path. This CL does not implement a general fix for SkGPipe, but
it resolves the leak in SkDeferredCanvas (currently the only user
of SkGPipe) by resetting the image heap when the deferral queue is
flushed. This change also fixes the accounting of bytes allocated
by referenced images in order to trigger flushing heuristics
appropriately.

BUG=crbug.com/494148

Review URL: https://codereview.chromium.org/1145893007

include/pipe/SkGPipe.h

@@ -87,6 +87,12 @@ public:
     virtual void notifyWritten(size_t bytes) = 0;
     virtual int numberOfReaders() const { return 1; }
 
+    /**
+     *  Release resource references that are held in internal caches.
+     *  This must only be called after the pipe has been completely flushed.
+     */
+    void purgeCaches();
+
 private:
     friend class SkGPipeWriter;
     void setCanvas(SkGPipeCanvas*);

src/pipe/SkGPipePriv.h

@@ -222,6 +222,8 @@ public:
     SkImageHeap();
     virtual ~SkImageHeap();
 
+    size_t bytesInCache() const { return fBytesInCache; }
+    void reset();
     // slot must be "valid" -- 0 is never valid
     const SkImage* get(int32_t slot) const;
     // returns 0 if not found, else returns slot
@@ -231,6 +233,7 @@ public:
 
 private:
     SkTDArray<const SkImage*> fArray;
+    size_t fBytesInCache;
 };
 
 ///////////////////////////////////////////////////////////////////////////////

src/pipe/SkGPipeWrite.cpp

@@ -228,7 +228,14 @@ public:
     size_t freeMemoryIfPossible(size_t bytesToFree);
 
     size_t storageAllocatedForRecording() {
-        return (NULL == fBitmapHeap) ? 0 : fBitmapHeap->bytesAllocated();
+        size_t bytesAllocated = 0;
+        if (NULL != fBitmapHeap) {
+            bytesAllocated += fBitmapHeap->bytesAllocated();
+        }
+        if (NULL != fImageHeap) {
+            bytesAllocated += fImageHeap->bytesInCache();
+        }
+        return bytesAllocated;
     }
 
     void beginCommentGroup(const char* description) override;
@@ -241,6 +248,8 @@ public:
      */
     bool shuttleBitmap(const SkBitmap&, int32_t slot);
 
+    void resetImageHeap();
+
 protected:
     void willSave() override;
     SaveLayerStrategy willSaveLayer(const SkRect*, const SkPaint*, SaveFlags) override;
@@ -1157,6 +1166,12 @@ void SkGPipeCanvas::flushRecording(bool detachCurrentBlock) {
     }
 }
 
+void SkGPipeCanvas::resetImageHeap() {
+    if (fImageHeap) {
+        fImageHeap->reset();
+    }
+}
+
 size_t SkGPipeCanvas::freeMemoryIfPossible(size_t bytesToFree) {
     return (NULL == fBitmapHeap) ? 0 : fBitmapHeap->freeMemoryIfPossible(bytesToFree);
 }
@@ -1320,6 +1335,14 @@ void SkGPipeController::setCanvas(SkGPipeCanvas* canvas) {
     SkRefCnt_SafeAssign(fCanvas, canvas);
 }
 
+void SkGPipeController::purgeCaches()
+{
+    fCanvas->resetImageHeap();
+    // Other caches are self-purging with a small MRU pool
+    // We could purge them as well, but it is not clear whether
+    // that would be a win.
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 SkGPipeWriter::SkGPipeWriter()
@@ -1393,12 +1416,18 @@ void BitmapShuttle::removeCanvas() {
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-SkImageHeap::SkImageHeap() {}
+SkImageHeap::SkImageHeap() : fBytesInCache (0) {}
 
 SkImageHeap::~SkImageHeap() {
     fArray.unrefAll();
 }
 
+void SkImageHeap::reset() {
+    fArray.unrefAll();
+    fArray.rewind();
+    fBytesInCache = 0;
+}
+
 const SkImage* SkImageHeap::get(int32_t slot) const {
     SkASSERT(slot > 0);
     return fArray[slot - 1];
@@ -1417,7 +1446,10 @@ int32_t SkImageHeap::insert(const SkImage* img) {
     if (slot) {
         return slot;
     }
+    // TODO: SkImage does not expose bytes per pixel, 4 is just a best guess.
+    fBytesInCache += img->width() * img->height() * 4;
     *fArray.append() = SkRef(img);
+    printf("Images reff'ed: %d \n", fArray.count());
     return fArray.count();  // slot is always index+1
 }
 

src/utils/SkDeferredCanvas.cpp

@@ -151,6 +151,8 @@ void DeferredPipeController::playback(bool silent) {
 
     // Release all allocated blocks
     fAllocator.reset();
+
+    this->purgeCaches();
 }
 
 //-----------------------------------------------------------------------------

tests/DeferredCanvasTest.cpp

@@ -689,6 +689,22 @@ static void TestDeferredCanvasBitmapSizeThreshold(skiatest::Reporter* reporter)
     }
 }
 
+static void TestDeferredCanvasImageFreeAfterFlush(skiatest::Reporter* reporter) {
+    SkAutoTUnref<SkSurface> surface(SkSurface::NewRasterN32Premul(100, 100));
+    SkAutoTUnref<SkSurface> sourceSurface(SkSurface::NewRasterN32Premul(100, 100));
+    SkAutoTUnref<SkImage> sourceImage(sourceSurface->newImageSnapshot());
+    SkAutoTUnref<SkDeferredCanvas> canvas(SkDeferredCanvas::Create(surface.get()));
+
+    canvas->drawImage(sourceImage, 0, 0, NULL);
+
+    size_t newBytesAllocated = canvas->storageAllocatedForRecording();
+    REPORTER_ASSERT(reporter, newBytesAllocated > 0);
+
+    canvas->flush();
+    
+    newBytesAllocated = canvas->storageAllocatedForRecording();
+    REPORTER_ASSERT(reporter, newBytesAllocated == 0);
+}
 
 typedef const void* PixelPtr;
 // Returns an opaque pointer which, either points to a GrTexture or RAM pixel
@@ -930,6 +946,7 @@ DEF_TEST(DeferredCanvas_CPU, reporter) {
     TestDeferredCanvasSkip(reporter);
     TestDeferredCanvasBitmapShaderNoLeak(reporter);
     TestDeferredCanvasBitmapSizeThreshold(reporter);
+    TestDeferredCanvasImageFreeAfterFlush(reporter);    
     TestDeferredCanvasCreateCompatibleDevice(reporter);
     TestDeferredCanvasWritePixelsToSurface(reporter);
     TestDeferredCanvasGetCanvasSize(reporter);