summaryrefslogtreecommitdiff
path: root/libs
diff options
context:
space:
mode:
authorDoris Liu <tianliu@google.com>2016-10-10 15:50:01 -0700
committerDoris Liu <tianliu@google.com>2016-10-12 01:22:11 +0000
commitfc9cf72339c7ce61adb11ceb3b247f112577fb6b (patch)
tree3ba78d43724d731fe47fbda1c0b2fbd3db27129e /libs
parent6c816c6707a5539a7a951617a8b621887c36eb38 (diff)
downloadandroid-frameworks-base-fc9cf72339c7ce61adb11ceb3b247f112577fb6b.tar.gz
android-frameworks-base-fc9cf72339c7ce61adb11ceb3b247f112577fb6b.tar.xz
Fix SkShader leak for Gradient VectorDrawable and test
This CL fixes a SkShader leak in VD when applying local matrix to the shader. Specifically, the usage of newWithLocalMatrix(...) increments the shader's ref count in every draw() call for Gradient VectorDrawable, whereas there's no balancing call to decrement the ref count in draw(). In this CL, we assume the ownership of the shader returned from newWithLocalMatrix(...) to ensure the correct ref count management. Also, add test to verify that shader is no longer being leaked BUG: 32067647 Test: this CL Change-Id: Ic15fe46cde06a73d81b44e2d3c56b51907344cc0
Diffstat (limited to 'libs')
-rw-r--r--libs/hwui/VectorDrawable.cpp8
-rw-r--r--libs/hwui/tests/unit/VectorDrawableTests.cpp44
2 files changed, 50 insertions, 2 deletions
diff --git a/libs/hwui/VectorDrawable.cpp b/libs/hwui/VectorDrawable.cpp
index 2b79941..aeee661 100644
--- a/libs/hwui/VectorDrawable.cpp
+++ b/libs/hwui/VectorDrawable.cpp
@@ -202,7 +202,9 @@ void FullPath::drawPath(SkCanvas* outCanvas, SkPath& renderPath, float strokeSca
if (properties.getFillGradient() != nullptr) {
paint.setColor(applyAlpha(SK_ColorBLACK, properties.getFillAlpha()));
SkShader* newShader = properties.getFillGradient()->newWithLocalMatrix(matrix);
- paint.setShader(newShader);
+ // newWithLocalMatrix(...) creates a new SkShader and returns a bare pointer. We need to
+ // remove the extra ref so that the ref count is correctly managed.
+ paint.setShader(newShader)->unref();
needsFill = true;
} else if (properties.getFillColor() != SK_ColorTRANSPARENT) {
paint.setColor(applyAlpha(properties.getFillColor(), properties.getFillAlpha()));
@@ -222,7 +224,9 @@ void FullPath::drawPath(SkCanvas* outCanvas, SkPath& renderPath, float strokeSca
if (properties.getStrokeGradient() != nullptr) {
paint.setColor(applyAlpha(SK_ColorBLACK, properties.getStrokeAlpha()));
SkShader* newShader = properties.getStrokeGradient()->newWithLocalMatrix(matrix);
- paint.setShader(newShader);
+ // newWithLocalMatrix(...) creates a new SkShader and returns a bare pointer. We need to
+ // remove the extra ref so that the ref count is correctly managed.
+ paint.setShader(newShader)->unref();
needsStroke = true;
} else if (properties.getStrokeColor() != SK_ColorTRANSPARENT) {
paint.setColor(applyAlpha(properties.getStrokeColor(), properties.getStrokeAlpha()));
diff --git a/libs/hwui/tests/unit/VectorDrawableTests.cpp b/libs/hwui/tests/unit/VectorDrawableTests.cpp
index 83b485f..8e0d3ee 100644
--- a/libs/hwui/tests/unit/VectorDrawableTests.cpp
+++ b/libs/hwui/tests/unit/VectorDrawableTests.cpp
@@ -426,5 +426,49 @@ TEST(VectorDrawable, groupProperties) {
EXPECT_EQ(1.0f, properties->getPivotY());
}
+
+static SkShader* createShader(bool* isDestroyed) {
+ class TestShader : public SkShader {
+ public:
+ TestShader(bool* isDestroyed) : SkShader(), mDestroyed(isDestroyed) {
+ }
+ ~TestShader() {
+ *mDestroyed = true;
+ }
+
+ Factory getFactory() const override { return nullptr; }
+ private:
+ bool* mDestroyed;
+ };
+ return new TestShader(isDestroyed);
+}
+
+TEST(VectorDrawable, drawPathWithoutIncrementingShaderRefCount) {
+ VectorDrawable::FullPath path("m1 1", 4);
+ SkBitmap bitmap;
+ SkImageInfo info = SkImageInfo::Make(5, 5, kN32_SkColorType, kPremul_SkAlphaType);
+ bitmap.setInfo(info);
+ bitmap.allocPixels(info);
+ SkCanvas canvas(bitmap);
+
+ bool shaderIsDestroyed = false;
+
+ // Initial ref count is 1
+ SkShader* shader = createShader(&shaderIsDestroyed);
+
+ // Setting the fill gradient increments the ref count of the shader by 1
+ path.mutateStagingProperties()->setFillGradient(shader);
+ path.draw(&canvas, SkMatrix::I(), 1.0f, 1.0f, true);
+ // Resetting the fill gradient decrements the ref count of the shader by 1
+ path.mutateStagingProperties()->setFillGradient(nullptr);
+
+ // Expect ref count to be 1 again, i.e. nothing else to have a ref to the shader now. Unref()
+ // again should bring the ref count to zero and consequently trigger detor.
+ shader->unref();
+
+ // Verify that detor is called.
+ EXPECT_TRUE(shaderIsDestroyed);
+}
+
}; // namespace uirenderer
}; // namespace android