In SkBitmapHeap, defer adding owners for new bitmaps.

When using an SkFlatDictionary to flatten shaders, the
dictionary can try to insert a duplicate bitmap shader
that uses a bitmap which has been removed from the
bitmap heap.

This change was originally suggested by junov in
https://codereview.appspot.com/6713048/.

Add a test to verify that deferring the owners works.
Without the change to bitmap heap the test would fail
(and crash in debug mode).

Also remove an unused function from SkFlatDictionary.

BUG=http://code.google.com/p/chromium/issues/detail?id=143923

Review URL: https://codereview.appspot.com/6842051

git-svn-id: http://skia.googlecode.com/svn/trunk@6471 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/gyp/tests.gyp b/gyp/tests.gyp
index 18dc558..d1c0f80 100644 (file)
--- a/gyp/tests.gyp
+++ b/gyp/tests.gyp
@@ -21,6 +21,7 @@
         '../tests/AtomicTest.cpp',
         '../tests/BitmapCopyTest.cpp',
         '../tests/BitmapGetColorTest.cpp',
+        '../tests/BitmapHeapTest.cpp',
         '../tests/BitSetTest.cpp',
         '../tests/BlitRowTest.cpp',
         '../tests/BlurTest.cpp',

diff --git a/src/core/SkBitmapHeap.cpp b/src/core/SkBitmapHeap.cpp
index 0263814..48194b1 100644 (file)
--- a/src/core/SkBitmapHeap.cpp
+++ b/src/core/SkBitmapHeap.cpp
@@ -373,7 +373,11 @@ int32_t SkBitmapHeap::insert(const SkBitmap& originalBitmap) {
     fBytesAllocated += entry->fBytesAllocated;
 
     if (fOwnerCount != IGNORE_OWNERS) {
-        entry->addReferences(fOwnerCount);
+        if (fDeferAddingOwners) {
+            *fDeferredEntries.append() = entry->fSlot;
+        } else {
+            entry->addReferences(fOwnerCount);
+        }
     }
     if (fPreferredCount != UNLIMITED_SIZE) {
         this->appendToLRU(fLookupTable[searchIndex]);

diff --git a/src/core/SkBitmapHeap.h b/src/core/SkBitmapHeap.h
index badbc9c..be99e19 100644 (file)
--- a/src/core/SkBitmapHeap.h
+++ b/src/core/SkBitmapHeap.h
@@ -47,6 +47,7 @@ private:
     size_t fBytesAllocated;
 
     friend class SkBitmapHeap;
+    friend class SkBitmapHeapTester;
 };
 
 

diff --git a/src/core/SkPictureFlat.h b/src/core/SkPictureFlat.h
index 2c8aa2c..a00596f 100644 (file)
--- a/src/core/SkPictureFlat.h
+++ b/src/core/SkPictureFlat.h
@@ -448,20 +448,6 @@ public:
     }
 
     /**
-     * Given a pointer to an array of type T we allocate the array and fill it
-     * with the unflattened dictionary contents. The return value is the size of
-     * the allocated array.
-     */
-    int unflattenDictionary(T*& array) const {
-        int elementCount = fData.count();
-        if (elementCount > 0) {
-            array = SkNEW_ARRAY(T, elementCount);
-            this->unflattenIntoArray(array);
-        }
-        return elementCount;
-    }
-
-    /**
      *  Unflatten the objects and return them in SkTRefArray, or return NULL
      *  if there no objects (instead of an empty array).
      */

diff --git a/tests/BitmapHeapTest.cpp b/tests/BitmapHeapTest.cpp
new file mode 100644 (file)
index 0000000..34a2984
--- /dev/null
+++ b/tests/BitmapHeapTest.cpp
@@ -0,0 +1,96 @@
+
+/*
+ * 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 "SkBitmap.h"
+#include "SkBitmapHeap.h"
+#include "SkColor.h"
+#include "SkFlattenable.h"
+#include "SkOrderedWriteBuffer.h"
+#include "SkPictureFlat.h"
+#include "SkRefCnt.h"
+#include "SkShader.h"
+#include "Test.h"
+
+class FlatDictionary : public SkFlatDictionary<SkShader> {
+
+public:
+    FlatDictionary(SkFlatController* controller)
+    : SkFlatDictionary<SkShader>(controller) {
+        fFlattenProc = &flattenFlattenableProc;
+        // No need for an unflattenProc
+    }
+    static void flattenFlattenableProc(SkOrderedWriteBuffer& buffer, const void* obj) {
+        buffer.writeFlattenable((SkFlattenable*)obj);
+    }
+};
+
+class SkBitmapHeapTester {
+
+public:
+    static int32_t GetRefCount(const SkBitmapHeapEntry* entry) {
+        return entry->fRefCount;
+    }
+};
+
+static void TestBitmapHeap(skiatest::Reporter* reporter) {
+    // Create a bitmap shader.
+    SkBitmap bm;
+    bm.setConfig(SkBitmap::kARGB_8888_Config, 2, 2);
+    bm.allocPixels();
+    bm.eraseColor(SK_ColorRED);
+    uint32_t* pixel = bm.getAddr32(1,0);
+    *pixel = SK_ColorBLUE;
+
+    SkShader* bitmapShader = SkShader::CreateBitmapShader(bm, SkShader::kRepeat_TileMode,
+                                                          SkShader::kRepeat_TileMode);
+    SkAutoTUnref<SkShader> aur(bitmapShader);
+
+    // Flatten, storing it in the bitmap heap.
+    SkBitmapHeap heap(1, 1);
+    SkChunkFlatController controller(1024);
+    controller.setBitmapStorage(&heap);
+    FlatDictionary dictionary(&controller);
+
+    // Dictionary and heap start off empty.
+    REPORTER_ASSERT(reporter, heap.count() == 0);
+    REPORTER_ASSERT(reporter, dictionary.count() == 0);
+
+    heap.deferAddingOwners();
+    int index = dictionary.find(*bitmapShader);
+    heap.endAddingOwnersDeferral(true);
+
+    // The dictionary and heap should now each have one entry.
+    REPORTER_ASSERT(reporter, 1 == index);
+    REPORTER_ASSERT(reporter, heap.count() == 1);
+    REPORTER_ASSERT(reporter, dictionary.count() == 1);
+
+    // The bitmap entry's refcount should be 1, then 0 after release.
+    SkBitmapHeapEntry* entry = heap.getEntry(0);
+    REPORTER_ASSERT(reporter, SkBitmapHeapTester::GetRefCount(entry) == 1);
+
+    entry->releaseRef();
+    REPORTER_ASSERT(reporter, SkBitmapHeapTester::GetRefCount(entry) == 0);
+
+    // Now clear out the heap, after which it should be empty.
+    heap.freeMemoryIfPossible(~0U);
+    REPORTER_ASSERT(reporter, heap.count() == 0);
+
+    // Now attempt to flatten the shader again.
+    heap.deferAddingOwners();
+    index = dictionary.find(*bitmapShader);
+    heap.endAddingOwnersDeferral(false);
+
+    // The dictionary should report the same index since the new entry is identical.
+    // The bitmap heap should contain the bitmap, but with no references.
+    REPORTER_ASSERT(reporter, 1 == index);
+    REPORTER_ASSERT(reporter, heap.count() == 1);
+    REPORTER_ASSERT(reporter, SkBitmapHeapTester::GetRefCount(heap.getEntry(0)) == 0);
+}
+
+#include "TestClassDef.h"
+DEFINE_TESTCLASS("BitmapHeap", TestBitmapHeapClass, TestBitmapHeap)