Fixing refcount leak in SkBitmapHeap caused by collisions in SkFlatDictionary
BUG=http://code.google.com/p/chromium/issues/detail?id=155875 TEST=DeferredCanvas unit test, subtest TestDeferredCanvasBitmapShaderNoLeak Review URL: https://codereview.appspot.com/6713048 git-svn-id: http://skia.googlecode.com/svn/trunk@5982 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
parent
fb2fdcca20
commit
ce65f385a0
@ -367,7 +367,7 @@ int32_t SkBitmapHeap::insert(const SkBitmap& originalBitmap) {
|
||||
// TODO if there is a shared pixel ref don't count it
|
||||
// If the SkBitmap does not share an SkPixelRef with an SkBitmap already
|
||||
// in the SharedHeap, also include the size of its pixels.
|
||||
entry->fBytesAllocated += originalBitmap.getSize();
|
||||
entry->fBytesAllocated = originalBitmap.getSize();
|
||||
|
||||
// add the bytes from this entry to the total count
|
||||
fBytesAllocated += entry->fBytesAllocated;
|
||||
|
@ -187,6 +187,7 @@ void SkOrderedReadBuffer::readBitmap(SkBitmap* bitmap) {
|
||||
} else {
|
||||
if (fBitmapStorage) {
|
||||
const uint32_t index = fReader.readU32();
|
||||
fReader.readU32(); // bitmap generation ID (see SkOrderedWriteBuffer::writeBitmap)
|
||||
*bitmap = *fBitmapStorage->getBitmap(index);
|
||||
fBitmapStorage->releaseRef(index);
|
||||
} else {
|
||||
|
@ -7,6 +7,7 @@
|
||||
*/
|
||||
|
||||
#include "SkOrderedWriteBuffer.h"
|
||||
#include "SkBitmap.h"
|
||||
#include "SkPtrRecorder.h"
|
||||
#include "SkStream.h"
|
||||
#include "SkTypeface.h"
|
||||
@ -164,7 +165,15 @@ void SkOrderedWriteBuffer::writeBitmap(const SkBitmap& bitmap) {
|
||||
// Bitmap was not encoded. Record a zero, implying that the reader need not decode.
|
||||
this->writeUInt(0);
|
||||
if (fBitmapHeap) {
|
||||
fWriter.write32(fBitmapHeap->insert(bitmap));
|
||||
int32_t slot = fBitmapHeap->insert(bitmap);
|
||||
fWriter.write32(slot);
|
||||
// crbug.com/155875
|
||||
// The generation ID is not required information. We write it to prevent collisions
|
||||
// in SkFlatDictionary. It is possible to get a collision when a previously
|
||||
// unflattened (i.e. stale) instance of a similar flattenable is in the dictionary
|
||||
// and the instance currently being written is re-using the same slot from the
|
||||
// bitmap heap.
|
||||
fWriter.write32(bitmap.getGenerationID());
|
||||
} else {
|
||||
bitmap.flatten(*this);
|
||||
}
|
||||
|
@ -12,12 +12,12 @@
|
||||
#include "SkFlattenableBuffers.h"
|
||||
|
||||
#include "SkRefCnt.h"
|
||||
#include "SkBitmap.h"
|
||||
#include "SkBitmapHeap.h"
|
||||
#include "SkPath.h"
|
||||
#include "SkSerializationHelpers.h"
|
||||
#include "SkWriter32.h"
|
||||
|
||||
class SkBitmap;
|
||||
class SkFlattenable;
|
||||
class SkFactorySet;
|
||||
class SkNamedFactorySet;
|
||||
|
@ -7,6 +7,7 @@
|
||||
*/
|
||||
#include "Test.h"
|
||||
#include "SkBitmap.h"
|
||||
#include "SkBitmapProcShader.h"
|
||||
#include "SkDeferredCanvas.h"
|
||||
#include "SkDevice.h"
|
||||
#include "SkShader.h"
|
||||
@ -348,6 +349,48 @@ static void TestDeferredCanvasSkip(skiatest::Reporter* reporter) {
|
||||
|
||||
}
|
||||
|
||||
static void TestDeferredCanvasBitmapShaderNoLeak(skiatest::Reporter* reporter) {
|
||||
// This is a regression test for crbug.com/155875
|
||||
// This test covers a code path that inserts bitmaps into the bitmap heap through the
|
||||
// flattening of SkBitmapProcShaders. The refcount in the bitmap heap is maintained through
|
||||
// the flattening and unflattening of the shader.
|
||||
SkBitmap store;
|
||||
store.setConfig(SkBitmap::kARGB_8888_Config, 100, 100);
|
||||
store.allocPixels();
|
||||
SkDevice device(store);
|
||||
SkDeferredCanvas canvas(&device);
|
||||
// test will fail if nbIterations is not in sync with
|
||||
// BITMAPS_TO_KEEP in SkGPipeWrite.cpp
|
||||
const int nbIterations = 5;
|
||||
size_t bytesAllocated = 0;
|
||||
for(int pass = 0; pass < 2; ++pass) {
|
||||
for(int i = 0; i < nbIterations; ++i) {
|
||||
SkPaint paint;
|
||||
SkBitmap paintPattern;
|
||||
paintPattern.setConfig(SkBitmap::kARGB_8888_Config, 10, 10);
|
||||
paintPattern.allocPixels();
|
||||
paint.setShader(SkNEW_ARGS(SkBitmapProcShader,
|
||||
(paintPattern, SkShader::kClamp_TileMode, SkShader::kClamp_TileMode)))->unref();
|
||||
canvas.drawPaint(paint);
|
||||
canvas.flush();
|
||||
|
||||
// In the first pass, memory allocation should be monotonically increasing as
|
||||
// the bitmap heap slots fill up. In the second pass memory allocation should be
|
||||
// stable as bitmap heap slots get recycled.
|
||||
size_t newBytesAllocated = canvas.storageAllocatedForRecording();
|
||||
if (pass == 0) {
|
||||
REPORTER_ASSERT(reporter, newBytesAllocated > bytesAllocated);
|
||||
bytesAllocated = newBytesAllocated;
|
||||
} else {
|
||||
REPORTER_ASSERT(reporter, newBytesAllocated == bytesAllocated);
|
||||
}
|
||||
}
|
||||
}
|
||||
// All cached resources should be evictable since last canvas call was flush()
|
||||
canvas.freeMemoryIfPossible(~0);
|
||||
REPORTER_ASSERT(reporter, 0 == canvas.storageAllocatedForRecording());
|
||||
}
|
||||
|
||||
static void TestDeferredCanvas(skiatest::Reporter* reporter) {
|
||||
TestDeferredCanvasBitmapAccess(reporter);
|
||||
TestDeferredCanvasFlush(reporter);
|
||||
@ -355,6 +398,7 @@ static void TestDeferredCanvas(skiatest::Reporter* reporter) {
|
||||
TestDeferredCanvasMemoryLimit(reporter);
|
||||
TestDeferredCanvasBitmapCaching(reporter);
|
||||
TestDeferredCanvasSkip(reporter);
|
||||
TestDeferredCanvasBitmapShaderNoLeak(reporter);
|
||||
}
|
||||
|
||||
#include "TestClassDef.h"
|
||||
|
Loading…
Reference in New Issue
Block a user