Merge "Fix SkShader leak for Gradient VectorDrawable and test" into nyc-mr1-dev

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()));

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