Switch SkLocalMatrixImageFilter and SkPaintImageFilter over to sk_sp

samplecode/SampleFilterFuzz.cpp

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 #include "SampleCode.h"
 #include "Sk1DPathEffect.h"
 #include "Sk2DPathEffect.h"
 #include "SkAlphaThresholdFilter.h"
@@ skipping 469 matching lines @@
                                                     light);
         }
         case 2:
         default:
             maskFilter = nullptr;
             break;
     }
     return maskFilter;
 }
 
-static SkImageFilter* make_image_filter(bool canBeNull = true);
+static sk_sp<SkImageFilter> make_image_filter(bool canBeNull = true);
 
 static SkPaint make_paint() {
     SkPaint paint;
     paint.setHinting(make_paint_hinting());
     paint.setAntiAlias(make_bool());
     paint.setDither(make_bool());
     paint.setLinearText(make_bool());
     paint.setSubpixelText(make_bool());
     paint.setLCDRenderText(make_bool());
     paint.setEmbeddedBitmapText(make_bool());
@@ skipping 32 matching lines @@
     paint.setImageFilter(make_image_filter());
     sk_sp<SkData> data(make_3Dlut(nullptr, make_bool(), make_bool(), make_bool()));
     paint.setTextAlign(make_paint_align());
     paint.setTextSize(make_scalar());
     paint.setTextScaleX(make_scalar());
     paint.setTextSkewX(make_scalar());
     paint.setTextEncoding(make_paint_text_encoding());
     return paint;
 }
 
-static SkImageFilter* make_image_filter(bool canBeNull) {
-    SkImageFilter* filter = 0;
+static sk_sp<SkImageFilter> make_image_filter(bool canBeNull) {
+    sk_sp<SkImageFilter> filter;
 
     // Add a 1 in 3 chance to get a nullptr input
-    if (canBeNull && (R(3) == 1)) { return filter; }
+    if (canBeNull && (R(3) == 1)) {
+        return filter;
+    }
 
     enum { ALPHA_THRESHOLD, MERGE, COLOR, LUT3D, BLUR, MAGNIFIER,
            DOWN_SAMPLE, XFERMODE, OFFSET, MATRIX, MATRIX_CONVOLUTION, COMPOSE,
            DISTANT_LIGHT, POINT_LIGHT, SPOT_LIGHT, NOISE, DROP_SHADOW,
            MORPHOLOGY, BITMAP, DISPLACE, TILE, PICTURE, PAINT, NUM_FILTERS };
 
     switch (R(NUM_FILTERS)) {
     case ALPHA_THRESHOLD:
-        filter = SkAlphaThresholdFilter::Create(make_region(), make_scalar(), make_scalar());
+        filter = sk_sp<SkImageFilter>(SkAlphaThresholdFilter::Create(make_region(),
+                                                                     make_scalar(),
+                                                                     make_scalar()));
         break;
     case MERGE:
-        filter = SkMergeImageFilter::Create(make_image_filter(), make_image_filter(), make_xfermode());
+        filter = sk_sp<SkImageFilter>(SkMergeImageFilter::Create(make_image_filter().release(), 

[f(malita), 2016/03/29 18:10:06]

Hmm...

Mixing old-style Create methods with sk_sps is a bit subtle: Create doesn't
normally adopt a ref but grabs a new one, so doing something like
Create(sp.release()) usually results in a leak.

Now granted this CL is correct in that it doesn't seem to change ownership
semantics, and preserves what I suspect is an existing leak.  Do we have ASAN
coverage for this code at all?

I think the safe pattern is something like

  sk_sp<SkImageFilter> subfilter = make_image_filter();
  filter = sk_sk<SkImageFilter>(SkFooImageFilter::Create(subfilter.get(), ...));

(same ref scoping as the other Create args)

[robertphillips, 2016/03/29 19:31:40]

Done.

+                                                                 make_image_filter().release(),
+                                                                 make_xfermode()));
         break;
-    case COLOR:
-    {
+    case COLOR: {
         sk_sp<SkColorFilter> cf(make_color_filter());
-        filter = cf ? SkColorFilterImageFilter::Create(cf.get(), make_image_filter()) : 0;
+        filter = cf ? sk_sp<SkImageFilter>(SkColorFilterImageFilter::Create(cf.get(),
+                                                                            make_image_filter().release())) 
+                    : nullptr;
+        break;
     }
-        break;
-    case LUT3D:
-    {
+    case LUT3D: {
         int cubeDimension;
         sk_sp<SkData> lut3D(make_3Dlut(&cubeDimension, (R(2) == 1), (R(2) == 1), (R(2) == 1)));
         sk_sp<SkColorFilter> cf(SkColorCubeFilter::Make(lut3D, cubeDimension));
-        filter = cf ? SkColorFilterImageFilter::Create(cf.get(), make_image_filter()) : 0;
+        filter = cf ? sk_sp<SkImageFilter>(SkColorFilterImageFilter::Create(cf.get(), 
+                                                                            make_image_filter().release()))
+                    : nullptr;
+        break;
     }
-        break;
     case BLUR:
-        filter = SkBlurImageFilter::Create(make_scalar(true), make_scalar(true), make_image_filter());
+        filter = sk_sp<SkImageFilter>(SkBlurImageFilter::Create(make_scalar(true),
+                                                                make_scalar(true),
+                                                                make_image_filter().release()));
         break;
     case MAGNIFIER:
-        filter = SkMagnifierImageFilter::Create(make_rect(), make_scalar(true));
+        filter = sk_sp<SkImageFilter>(SkMagnifierImageFilter::Create(make_rect(), 
+                                                                     make_scalar(true)));
         break;
     case DOWN_SAMPLE:
-        filter = SkDownSampleImageFilter::Create(make_scalar());
+        filter = sk_sp<SkImageFilter>(SkDownSampleImageFilter::Create(make_scalar()));
         break;
-    case XFERMODE:
-    {
+    case XFERMODE: {
         SkAutoTUnref<SkXfermode> mode(SkXfermode::Create(make_xfermode()));
-        filter = SkXfermodeImageFilter::Create(mode, make_image_filter(), make_image_filter());
+        filter = sk_sp<SkImageFilter>(SkXfermodeImageFilter::Create(mode,
+                                                                    make_image_filter().release(),
+                                                                    make_image_filter().release()));
+        break;
     }
-        break;
     case OFFSET:
-        filter = SkOffsetImageFilter::Create(make_scalar(), make_scalar(), make_image_filter());
+        filter = sk_sp<SkImageFilter>(SkOffsetImageFilter::Create(make_scalar(), make_scalar(),
+                                                                  make_image_filter().release()));
         break;
     case MATRIX:
-        filter = SkImageFilter::CreateMatrixFilter(make_matrix(),
-                                                   (SkFilterQuality)R(4),
-                                                   make_image_filter());
+        filter = sk_sp<SkImageFilter>(SkImageFilter::CreateMatrixFilter(make_matrix(),
+                                                                        (SkFilterQuality)R(4),
+                                                                        make_image_filter().release()));
         break;
-    case MATRIX_CONVOLUTION:
-    {
+    case MATRIX_CONVOLUTION: {
         SkImageFilter::CropRect cropR(SkRect::MakeWH(SkIntToScalar(kBitmapSize),
                                                      SkIntToScalar(kBitmapSize)));
         SkISize size = SkISize::Make(R(10)+1, R(10)+1);
         int arraySize = size.width() * size.height();
         SkTArray<SkScalar> kernel(arraySize);
         for (int i = 0; i < arraySize; ++i) {
             kernel.push_back() = make_scalar();
         }
         SkIPoint kernelOffset = SkIPoint::Make(R(SkIntToScalar(size.width())),
                                                R(SkIntToScalar(size.height())));
-        filter = SkMatrixConvolutionImageFilter::Create(size,
+        filter = sk_sp<SkImageFilter>(SkMatrixConvolutionImageFilter::Create(
+                                                        size,
                                                         kernel.begin(),
                                                         make_scalar(),
                                                         make_scalar(),
                                                         kernelOffset,
                                                         (SkMatrixConvolutionImageFilter::TileMode)R(3),
                                                         R(2) == 1,
-                                                        make_image_filter(),
-                                                        &cropR);
+                                                        make_image_filter().release(),
+                                                        &cropR));
+        break;
     }
-        break;
     case COMPOSE:
-        filter = SkComposeImageFilter::Create(make_image_filter(), make_image_filter());
+        filter = sk_sp<SkImageFilter>(SkComposeImageFilter::Create(make_image_filter().release(),
+                                                                   make_image_filter().release()));
         break;
     case DISTANT_LIGHT:
-        filter = (R(2) == 1) ?
-                 SkLightingImageFilter::CreateDistantLitDiffuse(make_point(),
-                 make_color(), make_scalar(), make_scalar(), make_image_filter()) :
-                 SkLightingImageFilter::CreateDistantLitSpecular(make_point(),
-                 make_color(), make_scalar(), make_scalar(), SkIntToScalar(R(10)),
-                 make_image_filter());
+        filter = (R(2) == 1)
+                 ? sk_sp<SkImageFilter>(SkLightingImageFilter::CreateDistantLitDiffuse(make_point(),
+                                                make_color(), make_scalar(), make_scalar(),
+                                                make_image_filter().release()))
+                 : sk_sp<SkImageFilter>(SkLightingImageFilter::CreateDistantLitSpecular(make_point(),
+                                                make_color(), make_scalar(),
+                                                make_scalar(), SkIntToScalar(R(10)),
+                                                make_image_filter().release()));
         break;
     case POINT_LIGHT:
-        filter = (R(2) == 1) ?
-                 SkLightingImageFilter::CreatePointLitDiffuse(make_point(),
-                 make_color(), make_scalar(), make_scalar(), make_image_filter()) :
-                 SkLightingImageFilter::CreatePointLitSpecular(make_point(),
-                 make_color(), make_scalar(), make_scalar(), SkIntToScalar(R(10)),
-                 make_image_filter());
+        filter = (R(2) == 1)
+                 ? sk_sp<SkImageFilter>(SkLightingImageFilter::CreatePointLitDiffuse(make_point(),
+                                                make_color(), make_scalar(), make_scalar(),
+                                                make_image_filter().release()))
+                 : sk_sp<SkImageFilter>(SkLightingImageFilter::CreatePointLitSpecular(make_point(),
+                                                make_color(), make_scalar(), make_scalar(),
+                                                SkIntToScalar(R(10)),
+                                                make_image_filter().release()));
         break;
     case SPOT_LIGHT:
-        filter = (R(2) == 1) ?
-                 SkLightingImageFilter::CreateSpotLitDiffuse(SkPoint3::Make(0, 0, 0),
-                 make_point(), make_scalar(), make_scalar(), make_color(),
-                 make_scalar(), make_scalar(), make_image_filter()) :
-                 SkLightingImageFilter::CreateSpotLitSpecular(SkPoint3::Make(0, 0, 0),
-                 make_point(), make_scalar(), make_scalar(), make_color(),
-                 make_scalar(), make_scalar(), SkIntToScalar(R(10)), make_image_filter());
+        filter = (R(2) == 1)
+                 ? sk_sp<SkImageFilter>(SkLightingImageFilter::CreateSpotLitDiffuse(SkPoint3::Make(0, 0, 0),
+                                                 make_point(), make_scalar(), make_scalar(),
+                                                 make_color(), make_scalar(), make_scalar(),
+                                                 make_image_filter().release()))
+                 : sk_sp<SkImageFilter>(SkLightingImageFilter::CreateSpotLitSpecular(SkPoint3::Make(0, 0, 0),
+                                                 make_point(), make_scalar(), make_scalar(),
+                                                 make_color(), make_scalar(), make_scalar(),
+                                                 SkIntToScalar(R(10)), make_image_filter().release()));
         break;
-    case NOISE:
-    {
+    case NOISE: {
         sk_sp<SkShader> shader((R(2) == 1) ?
             SkPerlinNoiseShader::MakeFractalNoise(
                 make_scalar(true), make_scalar(true), R(10.0f), make_scalar()) :
             SkPerlinNoiseShader::MakeTurbulence(
                 make_scalar(true), make_scalar(true), R(10.0f), make_scalar()));
         SkPaint paint;
         paint.setShader(shader);
         SkImageFilter::CropRect cropR(SkRect::MakeWH(SkIntToScalar(kBitmapSize),
                                                      SkIntToScalar(kBitmapSize)));
-        filter = SkPaintImageFilter::Create(paint, &cropR);
+        filter = SkPaintImageFilter::Make(paint, &cropR);
+        break;
     }
-        break;
     case DROP_SHADOW:
-        filter = SkDropShadowImageFilter::Create(make_scalar(), make_scalar(), make_scalar(true),
-                    make_scalar(true), make_color(), make_shadow_mode(), make_image_filter(),
-                    nullptr);
+        filter = sk_sp<SkImageFilter>(SkDropShadowImageFilter::Create(make_scalar(),
+                                                                      make_scalar(),
+                                                                      make_scalar(true),
+                                                                      make_scalar(true),
+                                                                      make_color(),
+                                                                      make_shadow_mode(),
+                                                                      make_image_filter().release(),
+                                                                      nullptr));
         break;
     case MORPHOLOGY:
         if (R(2) == 1) {
-            filter = SkDilateImageFilter::Create(R(static_cast<float>(kBitmapSize)),
-                R(static_cast<float>(kBitmapSize)), make_image_filter());
+            filter = sk_sp<SkImageFilter>(SkDilateImageFilter::Create(R(static_cast<float>(kBitmapSize)),
+                                                R(static_cast<float>(kBitmapSize)),
+                                                make_image_filter().release()));
         } else {
-            filter = SkErodeImageFilter::Create(R(static_cast<float>(kBitmapSize)),
-                R(static_cast<float>(kBitmapSize)), make_image_filter());
+            filter = sk_sp<SkImageFilter>(SkErodeImageFilter::Create(R(static_cast<float>(kBitmapSize)),
+                                                R(static_cast<float>(kBitmapSize)),
+                                                make_image_filter().release()));
         }
         break;
-    case BITMAP:
-    {
+    case BITMAP: {
         sk_sp<SkImage> image(SkImage::MakeFromBitmap(make_bitmap()));
         if (R(2) == 1) {
-            filter = SkImageSource::Create(image.get(), make_rect(), make_rect(), kHigh_SkFilterQuality);
+            filter = sk_sp<SkImageFilter>(SkImageSource::Create(image.get(),
+                                                                make_rect(),
+                                                                make_rect(),
+                                                                kHigh_SkFilterQuality));
         } else {
-            filter = SkImageSource::Create(image.get());
+            filter = sk_sp<SkImageFilter>(SkImageSource::Create(image.get()));
         }
+        break;
     }
-        break;
     case DISPLACE:
-        filter = SkDisplacementMapEffect::Create(make_channel_selector_type(),
-                                                 make_channel_selector_type(), make_scalar(),
-                                                 make_image_filter(false), make_image_filter());
+        filter = sk_sp<SkImageFilter>(SkDisplacementMapEffect::Create(make_channel_selector_type(),
+                                                                      make_channel_selector_type(),
+                                                                      make_scalar(),
+                                                                      make_image_filter(false).release(),
+                                                                      make_image_filter().release()));
         break;
     case TILE:
-        filter = SkTileImageFilter::Create(make_rect(), make_rect(), make_image_filter(false));
+        filter = sk_sp<SkImageFilter>(SkTileImageFilter::Create(make_rect(),
+                                                                make_rect(),
+                                                                make_image_filter(false).release()));
         break;
-    case PICTURE:
-    {
+    case PICTURE: {
         SkRTreeFactory factory;
         SkPictureRecorder recorder;
         SkCanvas* recordingCanvas = recorder.beginRecording(SkIntToScalar(kBitmapSize), 
                                                             SkIntToScalar(kBitmapSize), 
                                                             &factory, 0);
         drawSomething(recordingCanvas);
         sk_sp<SkPicture> pict(recorder.finishRecordingAsPicture());
-        filter = SkPictureImageFilter::Make(pict, make_rect()).release();
+        filter = SkPictureImageFilter::Make(pict, make_rect());
+        break;
     }
+    case PAINT: {
+        SkImageFilter::CropRect cropR(make_rect());
+        filter = SkPaintImageFilter::Make(make_paint(), &cropR);
         break;
-    case PAINT:
-    {
-        SkImageFilter::CropRect cropR(make_rect());
-        filter = SkPaintImageFilter::Create(make_paint(), &cropR);
     }
     default:
         break;
     }
     return (filter || canBeNull) ? filter : make_image_filter(canBeNull);
 }
 
 static SkImageFilter* make_serialized_image_filter() {
-    SkAutoTUnref<SkImageFilter> filter(make_image_filter(false));
-    SkAutoTUnref<SkData> data(SkValidatingSerializeFlattenable(filter));
+    sk_sp<SkImageFilter> filter(make_image_filter(false));
+    SkAutoTUnref<SkData> data(SkValidatingSerializeFlattenable(filter.get()));
     const unsigned char* ptr = static_cast<const unsigned char*>(data->data());
     size_t len = data->size();
 #ifdef SK_ADD_RANDOM_BIT_FLIPS
     unsigned char* p = const_cast<unsigned char*>(ptr);
     for (size_t i = 0; i < len; ++i, ++p) {
         if (R(250) == 1) { // 0.4% of the time, flip a bit or byte
             if (R(10) == 1) { // Then 10% of the time, change a whole byte
                 switch(R(3)) {
                 case 0:
                     *p ^= 0xFF; // Flip entire byte
@@ skipping 74 matching lines @@
     }
 
 private:
     typedef SkView INHERITED;
 };
 
 //////////////////////////////////////////////////////////////////////////////
 
 static SkView* MyFactory() { return new ImageFilterFuzzView; }
 static SkViewRegister reg(MyFactory);