Trying to be consistent when members are allowed to be NULL

src/effects/SkRectShaderImageFilter.cpp

Index: src/effects/SkRectShaderImageFilter.cpp
diff --git a/src/effects/SkRectShaderImageFilter.cpp b/src/effects/SkRectShaderImageFilter.cpp
index fb34ed0007152f124ae557a5567aad4effa3bed5..f02961128a22abb2f71cbad173c0e00c049e1741 100644
--- a/src/effects/SkRectShaderImageFilter.cpp
+++ b/src/effects/SkRectShaderImageFilter.cpp
@@ -33,7 +33,7 @@ SkRectShaderImageFilter::SkRectShaderImageFilter(SkShader* s, const CropRect* cr
   : INHERITED(0, NULL, cropRect, uniqueID)
   , fShader(s) {
     SkASSERT(s);
-    s->ref();
+    SkSafeRef(s);

[sugoi1, 2014/10/29 15:26:12]

This class' destructor calls SkSafeUnref(), so the constructor should call
SkSafeRef().

[reed1, 2014/10/29 15:43:39]

How did we get called with a NULL. Is this guy only access via a factory (e.g.
Create)? If so, seems better to have that guy just return NULL.

[sugoi1, 2014/10/29 18:21:00]

Done.

 }
 
 #ifdef SK_SUPPORT_LEGACY_DEEPFLATTENING
@@ -78,7 +78,7 @@ bool SkRectShaderImageFilter::onFilterImage(Proxy* proxy,
     SkPaint paint;
     SkMatrix matrix(ctx.ctm());
     matrix.postTranslate(SkIntToScalar(-bounds.left()), SkIntToScalar(-bounds.top()));
-    paint.setShader(SkShader::CreateLocalMatrixShader(fShader, matrix))->unref();
+    SkSafeUnref(paint.setShader(SkShader::CreateLocalMatrixShader(fShader, matrix)));

[sugoi1, 2014/10/29 15:26:12]

If fShader can be NULL, then SkShader::CreateLocalMatrixShader() may return
NULL.

 
     SkRect rect = SkRect::MakeWH(SkIntToScalar(bounds.width()), SkIntToScalar(bounds.height()));
     canvas.drawRect(rect, paint);