Speculative "fix" for crash in analyzeProcessors

From the bug it looks like a null fragment processors may be getting into the processor set. This CL tries to plug any gaps in our fragmentProcessor handling.

The only real substantive part to this CL is the addition of some "if (!fp) { return nullptr; }" blocks.

Everything else is just to add chokepoints for processor allocation.

Bug: 734076
Change-Id: I4952b1a05bc6690d5aa09de977fa6dc54c80338a
Reviewed-on: https://skia-review.googlesource.com/21267
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

19 files changed, 218 insertions, 148 deletions

diff --git a/src/core/SkNormalFlatSource.cpp b/src/core/SkNormalFlatSource.cpp
index c7cde03a62..382bd094e3 100644
--- a/src/core/SkNormalFlatSource.cpp
+++ b/src/core/SkNormalFlatSource.cpp
@@ -19,8 +19,8 @@
 
 class NormalFlatFP : public GrFragmentProcessor {
 public:
-    NormalFlatFP() : INHERITED(kConstantOutputForConstantInput_OptimizationFlag) {
-        this->initClassID<NormalFlatFP>();
+    static sk_sp<GrFragmentProcessor> Make() {
+        return sk_sp<GrFragmentProcessor>(new NormalFlatFP());
     }
 
     class GLSLNormalFlatFP : public GrGLSLFragmentProcessor {
@@ -40,6 +40,10 @@ public:
     const char* name() const override { return "NormalFlatFP"; }
 
 private:
+    NormalFlatFP() : INHERITED(kConstantOutputForConstantInput_OptimizationFlag) {
+        this->initClassID<NormalFlatFP>();
+    }
+
     void onGetGLSLProcessorKey(const GrShaderCaps& caps, GrProcessorKeyBuilder* b) const override {}
 
     GrColor4f constantOutputForConstantInput(GrColor4f) const override {
@@ -55,7 +59,7 @@ private:
 sk_sp<GrFragmentProcessor> SkNormalFlatSourceImpl::asFragmentProcessor(
         const SkShaderBase::AsFPArgs&) const {
 
-    return sk_make_sp<NormalFlatFP>();
+    return NormalFlatFP::Make();
 }
 
 #endif // SK_SUPPORT_GPU
diff --git a/src/core/SkNormalMapSource.cpp b/src/core/SkNormalMapSource.cpp
index 741db66e64..fd92d941cf 100644
--- a/src/core/SkNormalMapSource.cpp
+++ b/src/core/SkNormalMapSource.cpp
@@ -24,11 +24,9 @@
 
 class NormalMapFP : public GrFragmentProcessor {
 public:
-    NormalMapFP(sk_sp<GrFragmentProcessor> mapFP, const SkMatrix& invCTM)
-            : INHERITED(kNone_OptimizationFlags), fInvCTM(invCTM) {
-        this->registerChildProcessor(mapFP);
-
-        this->initClassID<NormalMapFP>();
+    static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor> mapFP,
+                                           const SkMatrix& invCTM) {
+        return sk_sp<GrFragmentProcessor>(new NormalMapFP(std::move(mapFP), invCTM));
     }
 
     class GLSLNormalMapFP : public GrGLSLFragmentProcessor {
@@ -104,6 +102,13 @@ public:
     const SkMatrix& invCTM() const { return fInvCTM; }
 
 private:
+    NormalMapFP(sk_sp<GrFragmentProcessor> mapFP, const SkMatrix& invCTM)
+            : INHERITED(kNone_OptimizationFlags), fInvCTM(invCTM) {
+        this->registerChildProcessor(mapFP);
+
+        this->initClassID<NormalMapFP>();
+    }
+
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override { return new GLSLNormalMapFP; }
 
     bool onIsEqual(const GrFragmentProcessor& proc) const override {
@@ -123,7 +128,7 @@ sk_sp<GrFragmentProcessor> SkNormalMapSourceImpl::asFragmentProcessor(
         return nullptr;
     }
 
-    return sk_make_sp<NormalMapFP>(std::move(mapFP), fInvCTM);
+    return NormalMapFP::Make(std::move(mapFP), fInvCTM);
 }
 
 #endif // SK_SUPPORT_GPU
diff --git a/src/effects/GrCircleBlurFragmentProcessor.h b/src/effects/GrCircleBlurFragmentProcessor.h
index f25bd8c085..e8f9723380 100644
--- a/src/effects/GrCircleBlurFragmentProcessor.h
+++ b/src/effects/GrCircleBlurFragmentProcessor.h
@@ -22,6 +22,8 @@ class GrResourceProvider;
 // profile that is just rotated about the origin of the circle.
 class GrCircleBlurFragmentProcessor : public GrFragmentProcessor {
 public:
+    static sk_sp<GrFragmentProcessor> Make(GrResourceProvider*, const SkRect& circle, float sigma);
+
     ~GrCircleBlurFragmentProcessor() override {}
 
     const char* name() const override { return "CircleBlur"; }
@@ -34,8 +36,6 @@ public:
         return str;
     }
 
-    static sk_sp<GrFragmentProcessor> Make(GrResourceProvider*, const SkRect& circle, float sigma);
-
 private:
     // This nested GLSL processor implementation is defined in the cpp file.
     class GLSLProcessor;
diff --git a/src/effects/SkArithmeticImageFilter.cpp b/src/effects/SkArithmeticImageFilter.cpp
index 94be346122..e6ae335623 100644
--- a/src/effects/SkArithmeticImageFilter.cpp
+++ b/src/effects/SkArithmeticImageFilter.cpp
@@ -366,13 +366,11 @@ sk_sp<SkSpecialImage> ArithmeticImageFilterImpl::filterImageGPU(
                                                         -SkIntToScalar(foregroundOffset.fY));
         sk_sp<GrColorSpaceXform> fgXform =
                 GrColorSpaceXform::Make(foreground->getColorSpace(), outputProperties.colorSpace());
-        sk_sp<GrFragmentProcessor> foregroundFP;
-
-        foregroundFP = GrTextureDomainEffect::Make(
-                std::move(foregroundProxy), std::move(fgXform),
-                foregroundMatrix, GrTextureDomain::MakeTexelDomain(foreground->subset()),
-                GrTextureDomain::kDecal_Mode, GrSamplerParams::kNone_FilterMode);
-
+        sk_sp<GrFragmentProcessor> foregroundFP(GrTextureDomainEffect::Make(
+                                std::move(foregroundProxy), std::move(fgXform),
+                                foregroundMatrix,
+                                GrTextureDomain::MakeTexelDomain(foreground->subset()),
+                                GrTextureDomain::kDecal_Mode, GrSamplerParams::kNone_FilterMode));
         paint.addColorFragmentProcessor(std::move(foregroundFP));
 
         sk_sp<GrFragmentProcessor> xferFP =
diff --git a/src/effects/SkBlurMaskFilter.cpp b/src/effects/SkBlurMaskFilter.cpp
index 665abcb742..df60b53288 100644
--- a/src/effects/SkBlurMaskFilter.cpp
+++ b/src/effects/SkBlurMaskFilter.cpp
@@ -1068,7 +1068,6 @@ bool SkBlurMaskFilterImpl::directFilterMaskGPU(GrContext* context,
 
 class GrRRectBlurEffect : public GrFragmentProcessor {
 public:
-
     static sk_sp<GrFragmentProcessor> Make(GrContext*,
                                            float sigma, float xformedSigma,
                                            const SkRRect& srcRRect, const SkRRect& devRRect);
diff --git a/src/effects/SkLightingImageFilter.cpp b/src/effects/SkLightingImageFilter.cpp
index 78f58993e0..0e8612480b 100644
--- a/src/effects/SkLightingImageFilter.cpp
+++ b/src/effects/SkLightingImageFilter.cpp
@@ -595,9 +595,6 @@ private:
 
 class GrLightingEffect : public GrSingleTextureEffect {
 public:
-    GrLightingEffect(sk_sp<GrTextureProxy>,
-                     const SkImageFilterLight* light, SkScalar surfaceScale,
-                     const SkMatrix& matrix, BoundaryMode boundaryMode, const SkIRect* srcBounds);
     ~GrLightingEffect() override;
 
     const SkImageFilterLight* light() const { return fLight; }
@@ -607,8 +604,11 @@ public:
     const GrTextureDomain& domain() const { return fDomain; }
 
 protected:
-    bool onIsEqual(const GrFragmentProcessor&) const override;
+    GrLightingEffect(sk_sp<GrTextureProxy>,
+                     const SkImageFilterLight* light, SkScalar surfaceScale,
+                     const SkMatrix& matrix, BoundaryMode boundaryMode, const SkIRect* srcBounds);
 
+    bool onIsEqual(const GrFragmentProcessor&) const override;
 
 private:
     const SkImageFilterLight* fLight;
diff --git a/src/effects/SkRRectsGaussianEdgeMaskFilter.cpp b/src/effects/SkRRectsGaussianEdgeMaskFilter.cpp
index 2cae9a4631..f5555d6240 100644
--- a/src/effects/SkRRectsGaussianEdgeMaskFilter.cpp
+++ b/src/effects/SkRRectsGaussianEdgeMaskFilter.cpp
@@ -204,15 +204,9 @@ public:
         kSimpleCircular_Mode,
     };
 
-    RRectsGaussianEdgeFP(const SkRRect& first, const SkRRect& second, SkScalar radius)
-            : INHERITED(kCompatibleWithCoverageAsAlpha_OptimizationFlag)
-            , fFirst(first)
-            , fSecond(second)
-            , fRadius(radius) {
-        this->initClassID<RRectsGaussianEdgeFP>();
-
-        fFirstMode = ComputeMode(fFirst);
-        fSecondMode = ComputeMode(fSecond);
+    static sk_sp<GrFragmentProcessor> Make(const SkRRect& first, const SkRRect& second,
+                                           SkScalar radius) {
+        return sk_sp<GrFragmentProcessor>(new RRectsGaussianEdgeFP(first, second, radius));
     }
 
     class GLSLRRectsGaussianEdgeFP : public GrGLSLFragmentProcessor {
@@ -462,6 +456,17 @@ public:
     SkScalar radius() const { return fRadius; }
 
 private:
+    RRectsGaussianEdgeFP(const SkRRect& first, const SkRRect& second, SkScalar radius)
+            : INHERITED(kCompatibleWithCoverageAsAlpha_OptimizationFlag)
+            , fFirst(first)
+            , fSecond(second)
+            , fRadius(radius) {
+        this->initClassID<RRectsGaussianEdgeFP>();
+
+        fFirstMode = ComputeMode(fFirst);
+        fSecondMode = ComputeMode(fSecond);
+    }
+
     static Mode ComputeMode(const SkRRect& rr) {
         if (rr.isCircle()) {
             return kCircle_Mode;
@@ -496,7 +501,7 @@ private:
 ////////////////////////////////////////////////////////////////////////////
 bool SkRRectsGaussianEdgeMaskFilterImpl::asFragmentProcessor(GrFragmentProcessor** fp) const {
     if (fp) {
-        *fp = new RRectsGaussianEdgeFP(fFirst, fSecond, fRadius);
+        *fp = RRectsGaussianEdgeFP::Make(fFirst, fSecond, fRadius).release();
     }
 
     return true;
diff --git a/src/effects/SkXfermodeImageFilter.cpp b/src/effects/SkXfermodeImageFilter.cpp
index 976d2d1e4d..18d52f98af 100644
--- a/src/effects/SkXfermodeImageFilter.cpp
+++ b/src/effects/SkXfermodeImageFilter.cpp
@@ -269,18 +269,15 @@ sk_sp<SkSpecialImage> SkXfermodeImageFilter_Base::filterImageGPU(
                                                 -SkIntToScalar(foregroundOffset.fY));
         sk_sp<GrColorSpaceXform> fgXform = GrColorSpaceXform::Make(foreground->getColorSpace(),
                                                                    outputProperties.colorSpace());
-        sk_sp<GrFragmentProcessor> foregroundFP;
-
-        foregroundFP = GrTextureDomainEffect::Make(
-                            std::move(foregroundProxy),
-                            std::move(fgXform), fgMatrix,
-                            GrTextureDomain::MakeTexelDomain(foreground->subset()),
-                            GrTextureDomain::kDecal_Mode,
-                            GrSamplerParams::kNone_FilterMode);
-
+        sk_sp<GrFragmentProcessor> foregroundFP(GrTextureDomainEffect::Make(
+                                        std::move(foregroundProxy),
+                                        std::move(fgXform), fgMatrix,
+                                        GrTextureDomain::MakeTexelDomain(foreground->subset()),
+                                        GrTextureDomain::kDecal_Mode,
+                                        GrSamplerParams::kNone_FilterMode));
         paint.addColorFragmentProcessor(std::move(foregroundFP));
 
-        sk_sp<GrFragmentProcessor> xferFP = this->makeFGFrag(bgFP);
+        sk_sp<GrFragmentProcessor> xferFP = this->makeFGFrag(std::move(bgFP));
 
         // A null 'xferFP' here means kSrc_Mode was used in which case we can just proceed
         if (xferFP) {
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 405668d4c4..0197eaaef2 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -381,7 +381,9 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceContext* dst,
             fp = fContext->createUPMToPMEffect(std::move(fp), useConfigConversionEffect);
         }
         fp = GrFragmentProcessor::SwizzleOutput(std::move(fp), tempDrawInfo.fSwizzle);
-        SkASSERT(fp);
+        if (!fp) {
+            return false;
+        }
 
         if (tempProxy->priv().hasPendingIO()) {
             this->flush(tempProxy.get());
@@ -508,7 +510,9 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src,
                 unpremul = false;
             }
             fp = GrFragmentProcessor::SwizzleOutput(std::move(fp), tempDrawInfo.fSwizzle);
-            SkASSERT(fp);
+            if (!fp) {
+                return false;
+            }
 
             GrPaint paint;
             paint.addColorFragmentProcessor(std::move(fp));
diff --git a/src/gpu/GrFragmentProcessor.cpp b/src/gpu/GrFragmentProcessor.cpp
index d69b34d94d..63dc3a37ae 100644
--- a/src/gpu/GrFragmentProcessor.cpp
+++ b/src/gpu/GrFragmentProcessor.cpp
@@ -121,15 +121,19 @@ namespace {
 
 class PremulInputFragmentProcessor : public GrFragmentProcessor {
 public:
+    static sk_sp<GrFragmentProcessor> Make() {
+        return sk_sp<GrFragmentProcessor>(new PremulInputFragmentProcessor);
+    }
+
+    const char* name() const override { return "PremultiplyInput"; }
+
+private:
     PremulInputFragmentProcessor()
             : INHERITED(kPreservesOpaqueInput_OptimizationFlag |
                         kConstantOutputForConstantInput_OptimizationFlag) {
         this->initClassID<PremulInputFragmentProcessor>();
     }
 
-    const char* name() const override { return "PremultiplyInput"; }
-
-private:
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override {
         class GLFP : public GrGLSLFragmentProcessor {
         public:
@@ -157,15 +161,19 @@ private:
 
 class UnpremulInputFragmentProcessor : public GrFragmentProcessor {
 public:
+    static sk_sp<GrFragmentProcessor> Make() {
+        return sk_sp<GrFragmentProcessor>(new UnpremulInputFragmentProcessor);
+    }
+
+    const char* name() const override { return "UnpremultiplyInput"; }
+
+private:
     UnpremulInputFragmentProcessor()
             : INHERITED(kPreservesOpaqueInput_OptimizationFlag |
                         kConstantOutputForConstantInput_OptimizationFlag) {
         this->initClassID<UnpremulInputFragmentProcessor>();
     }
 
-    const char* name() const override { return "UnpremultiplyInput"; }
-
-private:
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override {
         class GLFP : public GrGLSLFragmentProcessor {
         public:
@@ -198,7 +206,7 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::PremulInput(sk_sp<GrFragmentProc
     if (!fp) {
         return nullptr;
     }
-    sk_sp<GrFragmentProcessor> fpPipeline[] = { sk_make_sp<PremulInputFragmentProcessor>(), fp};
+    sk_sp<GrFragmentProcessor> fpPipeline[] = { PremulInputFragmentProcessor::Make(), fp};
     return GrFragmentProcessor::RunInSeries(fpPipeline, 2);
 }
 
@@ -206,7 +214,7 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::PremulOutput(sk_sp<GrFragmentPro
     if (!fp) {
         return nullptr;
     }
-    sk_sp<GrFragmentProcessor> fpPipeline[] = { fp, sk_make_sp<PremulInputFragmentProcessor>() };
+    sk_sp<GrFragmentProcessor> fpPipeline[] = { fp, PremulInputFragmentProcessor::Make() };
     return GrFragmentProcessor::RunInSeries(fpPipeline, 2);
 }
 
@@ -214,7 +222,7 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::UnpremulOutput(sk_sp<GrFragmentP
     if (!fp) {
         return nullptr;
     }
-    sk_sp<GrFragmentProcessor> fpPipeline[] = { fp, sk_make_sp<UnpremulInputFragmentProcessor>() };
+    sk_sp<GrFragmentProcessor> fpPipeline[] = { fp, UnpremulInputFragmentProcessor::Make() };
     return GrFragmentProcessor::RunInSeries(fpPipeline, 2);
 }
 
@@ -222,16 +230,20 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::SwizzleOutput(sk_sp<GrFragmentPr
                                                               const GrSwizzle& swizzle) {
     class SwizzleFragmentProcessor : public GrFragmentProcessor {
     public:
-        SwizzleFragmentProcessor(const GrSwizzle& swizzle)
-                : INHERITED(kAll_OptimizationFlags)
-                , fSwizzle(swizzle) {
-            this->initClassID<SwizzleFragmentProcessor>();
+        static sk_sp<GrFragmentProcessor> Make(const GrSwizzle& swizzle) {
+            return sk_sp<GrFragmentProcessor>(new SwizzleFragmentProcessor(swizzle));
         }
 
         const char* name() const override { return "Swizzle"; }
         const GrSwizzle& swizzle() const { return fSwizzle; }
 
     private:
+        SwizzleFragmentProcessor(const GrSwizzle& swizzle)
+                : INHERITED(kAll_OptimizationFlags)
+                , fSwizzle(swizzle) {
+            this->initClassID<SwizzleFragmentProcessor>();
+        }
+
         GrGLSLFragmentProcessor* onCreateGLSLInstance() const override {
             class GLFP : public GrGLSLFragmentProcessor {
             public:
@@ -271,7 +283,7 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::SwizzleOutput(sk_sp<GrFragmentPr
     if (GrSwizzle::RGBA() == swizzle) {
         return fp;
     }
-    sk_sp<GrFragmentProcessor> fpPipeline[] = { fp, sk_make_sp<SwizzleFragmentProcessor>(swizzle) };
+    sk_sp<GrFragmentProcessor> fpPipeline[] = { fp, SwizzleFragmentProcessor::Make(swizzle) };
     return GrFragmentProcessor::RunInSeries(fpPipeline, 2);
 }
 
@@ -280,15 +292,19 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::MakeInputPremulAndMulByOutput(
 
     class PremulFragmentProcessor : public GrFragmentProcessor {
     public:
+        static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor> processor) {
+            return sk_sp<GrFragmentProcessor>(new PremulFragmentProcessor(std::move(processor)));
+        }
+
+        const char* name() const override { return "Premultiply"; }
+
+    private:
         PremulFragmentProcessor(sk_sp<GrFragmentProcessor> processor)
                 : INHERITED(OptFlags(processor.get())) {
             this->initClassID<PremulFragmentProcessor>();
             this->registerChildProcessor(processor);
         }
 
-        const char* name() const override { return "Premultiply"; }
-
-    private:
         GrGLSLFragmentProcessor* onCreateGLSLInstance() const override {
             class GLFP : public GrGLSLFragmentProcessor {
             public:
@@ -332,7 +348,7 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::MakeInputPremulAndMulByOutput(
     if (!fp) {
         return nullptr;
     }
-    return sk_sp<GrFragmentProcessor>(new PremulFragmentProcessor(std::move(fp)));
+    return PremulFragmentProcessor::Make(std::move(fp));
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -341,10 +357,9 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::OverrideInput(sk_sp<GrFragmentPr
                                                               GrColor4f color) {
     class ReplaceInputFragmentProcessor : public GrFragmentProcessor {
     public:
-        ReplaceInputFragmentProcessor(sk_sp<GrFragmentProcessor> child, GrColor4f color)
-                : INHERITED(OptFlags(child.get(), color)), fColor(color) {
-            this->initClassID<ReplaceInputFragmentProcessor>();
-            this->registerChildProcessor(std::move(child));
+        static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor> child, GrColor4f color) {
+            return sk_sp<GrFragmentProcessor>(new ReplaceInputFragmentProcessor(std::move(child),
+                                                                                color));
         }
 
         const char* name() const override { return "Replace Color"; }
@@ -382,6 +397,12 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::OverrideInput(sk_sp<GrFragmentPr
         }
 
     private:
+        ReplaceInputFragmentProcessor(sk_sp<GrFragmentProcessor> child, GrColor4f color)
+                : INHERITED(OptFlags(child.get(), color)), fColor(color) {
+            this->initClassID<ReplaceInputFragmentProcessor>();
+            this->registerChildProcessor(std::move(child));
+        }
+
         static OptimizationFlags OptFlags(const GrFragmentProcessor* child, GrColor4f color) {
             OptimizationFlags childFlags = child->optimizationFlags();
             OptimizationFlags flags = kNone_OptimizationFlags;
@@ -410,20 +431,18 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::OverrideInput(sk_sp<GrFragmentPr
         typedef GrFragmentProcessor INHERITED;
     };
 
-    return sk_sp<GrFragmentProcessor>(new ReplaceInputFragmentProcessor(std::move(fp), color));
+    if (!fp) {
+        return nullptr;
+    }
+    return ReplaceInputFragmentProcessor::Make(std::move(fp), color);
 }
 
 sk_sp<GrFragmentProcessor> GrFragmentProcessor::RunInSeries(sk_sp<GrFragmentProcessor>* series,
                                                             int cnt) {
     class SeriesFragmentProcessor : public GrFragmentProcessor {
     public:
-        SeriesFragmentProcessor(sk_sp<GrFragmentProcessor>* children, int cnt)
-                : INHERITED(OptFlags(children, cnt)) {
-            SkASSERT(cnt > 1);
-            this->initClassID<SeriesFragmentProcessor>();
-            for (int i = 0; i < cnt; ++i) {
-                this->registerChildProcessor(std::move(children[i]));
-            }
+        static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor>* children, int cnt) {
+            return sk_sp<GrFragmentProcessor>(new SeriesFragmentProcessor(children, cnt));
         }
 
         const char* name() const override { return "Series"; }
@@ -448,6 +467,15 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::RunInSeries(sk_sp<GrFragmentProc
             return new GLFP;
         }
     private:
+        SeriesFragmentProcessor(sk_sp<GrFragmentProcessor>* children, int cnt)
+                : INHERITED(OptFlags(children, cnt)) {
+            SkASSERT(cnt > 1);
+            this->initClassID<SeriesFragmentProcessor>();
+            for (int i = 0; i < cnt; ++i) {
+                this->registerChildProcessor(std::move(children[i]));
+            }
+        }
+
         static OptimizationFlags OptFlags(sk_sp<GrFragmentProcessor>* children, int cnt) {
             OptimizationFlags flags = kAll_OptimizationFlags;
             for (int i = 0; i < cnt && flags != kNone_OptimizationFlags; ++i) {
@@ -496,7 +524,7 @@ sk_sp<GrFragmentProcessor> GrFragmentProcessor::RunInSeries(sk_sp<GrFragmentProc
         }
         series = replacementSeries.begin();
     }
-    return sk_sp<GrFragmentProcessor>(new SeriesFragmentProcessor(series, cnt));
+    return SeriesFragmentProcessor::Make(series, cnt);
 }
 
 //////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 08a3daea59..caa960d6fa 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -1423,6 +1423,9 @@ void SkGpuDevice::drawProducerNine(GrTextureProducer* producer,
                                           SkRect::MakeIWH(producer->width(), producer->height()),
                                           GrTextureProducer::kNo_FilterConstraint, true,
                                           &kMode, fRenderTargetContext->getColorSpace()));
+    if (!fp) {
+        return;
+    }
     GrPaint grPaint;
     if (!SkPaintToGrPaintWithTexture(this->context(), fRenderTargetContext.get(), paint,
                                      this->ctm(), std::move(fp), producer->isAlphaOnly(),
@@ -1478,6 +1481,9 @@ void SkGpuDevice::drawProducerLattice(GrTextureProducer* producer,
                                           SkRect::MakeIWH(producer->width(), producer->height()),
                                           GrTextureProducer::kNo_FilterConstraint, true,
                                           &kMode, fRenderTargetContext->getColorSpace()));
+    if (!fp) {
+        return;
+    }
     GrPaint grPaint;
     if (!SkPaintToGrPaintWithTexture(this->context(), fRenderTargetContext.get(), paint,
                                      this->ctm(), std::move(fp), producer->isAlphaOnly(),
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index 9adcb1a9ec..6db50941d7 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -456,7 +456,7 @@ static inline bool skpaint_to_grpaint_impl(GrContext* context,
 
             // The above may return null if compose results in a pass through of the prim color.
             if (shaderFP) {
-                grPaint->addColorFragmentProcessor(shaderFP);
+                grPaint->addColorFragmentProcessor(std::move(shaderFP));
             }
 
             // We can ignore origColor here - alpha is unchanged by gamma
diff --git a/src/gpu/effects/Gr1DKernelEffect.h b/src/gpu/effects/Gr1DKernelEffect.h
index bc05e45933..fbdfed6d4f 100644
--- a/src/gpu/effects/Gr1DKernelEffect.h
+++ b/src/gpu/effects/Gr1DKernelEffect.h
@@ -28,13 +28,6 @@ public:
         kY_Direction,
     };
 
-    Gr1DKernelEffect(OptimizationFlags optFlags,
-                     sk_sp<GrTextureProxy> proxy, Direction direction, int radius)
-        : INHERITED(optFlags, std::move(proxy), nullptr, SkMatrix::I())
-        , fDirection(direction)
-        , fRadius(radius) {
-    }
-
     ~Gr1DKernelEffect() override {}
 
     static int WidthFromRadius(int radius) { return 2 * radius + 1; }
@@ -50,6 +43,14 @@ public:
         return str;
     }
 
+protected:
+    Gr1DKernelEffect(OptimizationFlags optFlags,
+                     sk_sp<GrTextureProxy> proxy, Direction direction, int radius)
+        : INHERITED(optFlags, std::move(proxy), nullptr, SkMatrix::I())
+        , fDirection(direction)
+        , fRadius(radius) {
+    }
+
 private:
     Direction       fDirection;
     int             fRadius;
diff --git a/src/gpu/effects/GrSRGBEffect.cpp b/src/gpu/effects/GrSRGBEffect.cpp
index 7ca79c7452..c9a48b3d84 100644
--- a/src/gpu/effects/GrSRGBEffect.cpp
+++ b/src/gpu/effects/GrSRGBEffect.cpp
@@ -107,7 +107,7 @@ GR_DEFINE_FRAGMENT_PROCESSOR_TEST(GrSRGBEffect);
 #if GR_TEST_UTILS
 sk_sp<GrFragmentProcessor> GrSRGBEffect::TestCreate(GrProcessorTestData* d) {
     Mode testMode = static_cast<Mode>(d->fRandom->nextRangeU(0, 1));
-    return sk_sp<GrFragmentProcessor>(new GrSRGBEffect(testMode));
+    return GrSRGBEffect::Make(testMode);
 }
 #endif
 
@@ -122,6 +122,3 @@ GrGLSLFragmentProcessor* GrSRGBEffect::onCreateGLSLInstance() const {
     return new GrGLSRGBEffect();
 }
 
-sk_sp<GrFragmentProcessor> GrSRGBEffect::Make(Mode mode) {
-    return sk_sp<GrFragmentProcessor>(new GrSRGBEffect(mode));
-}
diff --git a/src/gpu/effects/GrSRGBEffect.h b/src/gpu/effects/GrSRGBEffect.h
index 17afa5e5bd..169b223221 100644
--- a/src/gpu/effects/GrSRGBEffect.h
+++ b/src/gpu/effects/GrSRGBEffect.h
@@ -20,7 +20,9 @@ public:
     /**
      * Creates an effect that applies the sRGB transfer function (or its inverse)
      */
-    static sk_sp<GrFragmentProcessor> Make(Mode mode);
+    static sk_sp<GrFragmentProcessor> Make(Mode mode) {
+        return sk_sp<GrFragmentProcessor>(new GrSRGBEffect(mode));
+    }
 
     const char* name() const override { return "sRGB"; }
 
diff --git a/src/gpu/effects/GrXfermodeFragmentProcessor.cpp b/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
index f034938852..dde383ed45 100644
--- a/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
+++ b/src/gpu/effects/GrXfermodeFragmentProcessor.cpp
@@ -29,14 +29,11 @@ static inline bool does_cpu_blend_impl_match_gpu(SkBlendMode mode) {
 
 class ComposeTwoFragmentProcessor : public GrFragmentProcessor {
 public:
-    ComposeTwoFragmentProcessor(sk_sp<GrFragmentProcessor> src, sk_sp<GrFragmentProcessor> dst,
-                                SkBlendMode mode)
-            : INHERITED(OptFlags(src.get(), dst.get(), mode)), fMode(mode) {
-        this->initClassID<ComposeTwoFragmentProcessor>();
-        SkDEBUGCODE(int shaderAChildIndex = )this->registerChildProcessor(std::move(src));
-        SkDEBUGCODE(int shaderBChildIndex = )this->registerChildProcessor(std::move(dst));
-        SkASSERT(0 == shaderAChildIndex);
-        SkASSERT(1 == shaderBChildIndex);
+    static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor> src,
+                                           sk_sp<GrFragmentProcessor> dst,
+                                           SkBlendMode mode) {
+        return sk_sp<GrFragmentProcessor>(new ComposeTwoFragmentProcessor(std::move(src),
+                                                                          std::move(dst), mode));
     }
 
     const char* name() const override { return "ComposeTwo"; }
@@ -60,6 +57,18 @@ public:
     SkBlendMode getMode() const { return fMode; }
 
 private:
+    ComposeTwoFragmentProcessor(sk_sp<GrFragmentProcessor> src,
+                                sk_sp<GrFragmentProcessor> dst,
+                                SkBlendMode mode)
+            : INHERITED(OptFlags(src.get(), dst.get(), mode))
+            , fMode(mode) {
+        this->initClassID<ComposeTwoFragmentProcessor>();
+        SkDEBUGCODE(int shaderAChildIndex = )this->registerChildProcessor(std::move(src));
+        SkDEBUGCODE(int shaderBChildIndex = )this->registerChildProcessor(std::move(dst));
+        SkASSERT(0 == shaderAChildIndex);
+        SkASSERT(1 == shaderBChildIndex);
+    }
+
     static OptimizationFlags OptFlags(const GrFragmentProcessor* src,
                                       const GrFragmentProcessor* dst, SkBlendMode mode) {
         OptimizationFlags flags;
@@ -235,8 +244,7 @@ sk_sp<GrFragmentProcessor> GrXfermodeFragmentProcessor::MakeFromTwoProcessors(
         case SkBlendMode::kDst:
             return dst;
         default:
-            return sk_sp<GrFragmentProcessor>(
-                new ComposeTwoFragmentProcessor(std::move(src), std::move(dst), mode));
+            return ComposeTwoFragmentProcessor::Make(std::move(src), std::move(dst), mode);
     }
 }
 
@@ -249,11 +257,13 @@ public:
         kSrc_Child,
     };
 
-    ComposeOneFragmentProcessor(sk_sp<GrFragmentProcessor> fp, SkBlendMode mode, Child child)
-            : INHERITED(OptFlags(fp.get(), mode, child)), fMode(mode), fChild(child) {
-        this->initClassID<ComposeOneFragmentProcessor>();
-        SkDEBUGCODE(int dstIndex =) this->registerChildProcessor(std::move(fp));
-        SkASSERT(0 == dstIndex);
+    static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor> fp, SkBlendMode mode,
+                                           Child child) {
+        if (!fp) {
+            return nullptr;
+        }
+        return sk_sp<GrFragmentProcessor>(new ComposeOneFragmentProcessor(std::move(fp),
+                                                                          mode, child));
     }
 
     const char* name() const override { return "ComposeOne"; }
@@ -396,6 +406,15 @@ private:
     }
 
 private:
+    ComposeOneFragmentProcessor(sk_sp<GrFragmentProcessor> fp, SkBlendMode mode, Child child)
+            : INHERITED(OptFlags(fp.get(), mode, child))
+            , fMode(mode)
+            , fChild(child) {
+        this->initClassID<ComposeOneFragmentProcessor>();
+        SkDEBUGCODE(int dstIndex =) this->registerChildProcessor(std::move(fp));
+        SkASSERT(0 == dstIndex);
+    }
+
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override;
 
     SkBlendMode fMode;
@@ -481,9 +500,8 @@ sk_sp<GrFragmentProcessor> GrXfermodeFragmentProcessor::MakeFromDstProcessor(
         case SkBlendMode::kSrc:
             return nullptr;
         default:
-            return sk_sp<GrFragmentProcessor>(
-                new ComposeOneFragmentProcessor(std::move(dst), mode,
-                                                ComposeOneFragmentProcessor::kDst_Child));
+            return ComposeOneFragmentProcessor::Make(std::move(dst), mode,
+                                                     ComposeOneFragmentProcessor::kDst_Child);
     }
 }
 
@@ -496,8 +514,7 @@ sk_sp<GrFragmentProcessor> GrXfermodeFragmentProcessor::MakeFromSrcProcessor(
         case SkBlendMode::kDst:
             return nullptr;
         default:
-            return sk_sp<GrFragmentProcessor>(
-                new ComposeOneFragmentProcessor(std::move(src), mode,
-                                                ComposeOneFragmentProcessor::kSrc_Child));
+            return ComposeOneFragmentProcessor::Make(std::move(src), mode,
+                                                     ComposeOneFragmentProcessor::kSrc_Child);
     }
 }
diff --git a/src/gpu/effects/GrYUVEffect.cpp b/src/gpu/effects/GrYUVEffect.cpp
index feb2670b02..7a18b02c4f 100644
--- a/src/gpu/effects/GrYUVEffect.cpp
+++ b/src/gpu/effects/GrYUVEffect.cpp
@@ -216,14 +216,10 @@ public:
         kV_OutputChannels
     };
 
-    RGBToYUVEffect(sk_sp<GrFragmentProcessor> rgbFP, SkYUVColorSpace colorSpace,
-                   OutputChannels output)
-            // This could advertise kConstantOutputForConstantInput, but doesn't seem useful.
-            : INHERITED(kPreservesOpaqueInput_OptimizationFlag)
-            , fColorSpace(colorSpace)
-            , fOutputChannels(output) {
-        this->initClassID<RGBToYUVEffect>();
-        this->registerChildProcessor(std::move(rgbFP));
+    static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor> rgbFP,
+                                           SkYUVColorSpace colorSpace,
+                                           OutputChannels output) {
+        return sk_sp<GrFragmentProcessor>(new RGBToYUVEffect(std::move(rgbFP), colorSpace, output));
     }
 
     const char* name() const override { return "RGBToYUV"; }
@@ -328,6 +324,16 @@ public:
     };
 
 private:
+    RGBToYUVEffect(sk_sp<GrFragmentProcessor> rgbFP, SkYUVColorSpace colorSpace,
+                   OutputChannels output)
+            // This could advertise kConstantOutputForConstantInput, but doesn't seem useful.
+            : INHERITED(kPreservesOpaqueInput_OptimizationFlag)
+            , fColorSpace(colorSpace)
+            , fOutputChannels(output) {
+        this->initClassID<RGBToYUVEffect>();
+        this->registerChildProcessor(std::move(rgbFP));
+    }
+
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override {
         return new GLSLProcessor;
     }
@@ -371,34 +377,29 @@ sk_sp<GrFragmentProcessor> GrYUVEffect::MakeYUVToRGB(sk_sp<GrTextureProxy> yProx
 sk_sp<GrFragmentProcessor>
 GrYUVEffect::MakeRGBToYUV(sk_sp<GrFragmentProcessor> rgbFP, SkYUVColorSpace colorSpace) {
     SkASSERT(rgbFP);
-    return sk_sp<GrFragmentProcessor>(
-        new RGBToYUVEffect(std::move(rgbFP), colorSpace, RGBToYUVEffect::kYUV_OutputChannels));
+    return RGBToYUVEffect::Make(std::move(rgbFP), colorSpace, RGBToYUVEffect::kYUV_OutputChannels);
 }
 
 sk_sp<GrFragmentProcessor>
 GrYUVEffect::MakeRGBToY(sk_sp<GrFragmentProcessor> rgbFP, SkYUVColorSpace colorSpace) {
     SkASSERT(rgbFP);
-    return sk_sp<GrFragmentProcessor>(
-        new RGBToYUVEffect(std::move(rgbFP), colorSpace, RGBToYUVEffect::kY_OutputChannels));
+    return RGBToYUVEffect::Make(std::move(rgbFP), colorSpace, RGBToYUVEffect::kY_OutputChannels);
 }
 
 sk_sp<GrFragmentProcessor>
 GrYUVEffect::MakeRGBToUV(sk_sp<GrFragmentProcessor> rgbFP, SkYUVColorSpace colorSpace) {
     SkASSERT(rgbFP);
-    return sk_sp<GrFragmentProcessor>(
-        new RGBToYUVEffect(std::move(rgbFP), colorSpace, RGBToYUVEffect::kUV_OutputChannels));
+    return RGBToYUVEffect::Make(std::move(rgbFP), colorSpace, RGBToYUVEffect::kUV_OutputChannels);
 }
 
 sk_sp<GrFragmentProcessor>
 GrYUVEffect::MakeRGBToU(sk_sp<GrFragmentProcessor> rgbFP, SkYUVColorSpace colorSpace) {
     SkASSERT(rgbFP);
-    return sk_sp<GrFragmentProcessor>(
-        new RGBToYUVEffect(std::move(rgbFP), colorSpace, RGBToYUVEffect::kU_OutputChannels));
+    return RGBToYUVEffect::Make(std::move(rgbFP), colorSpace, RGBToYUVEffect::kU_OutputChannels);
 }
 
 sk_sp<GrFragmentProcessor>
 GrYUVEffect::MakeRGBToV(sk_sp<GrFragmentProcessor> rgbFP, SkYUVColorSpace colorSpace) {
     SkASSERT(rgbFP);
-    return sk_sp<GrFragmentProcessor>(
-        new RGBToYUVEffect(std::move(rgbFP), colorSpace, RGBToYUVEffect::kV_OutputChannels));
+    return RGBToYUVEffect::Make(std::move(rgbFP), colorSpace, RGBToYUVEffect::kV_OutputChannels);
 }
diff --git a/src/shaders/SkLightingShader.cpp b/src/shaders/SkLightingShader.cpp
index cdfa528e1e..b19f9df97f 100644
--- a/src/shaders/SkLightingShader.cpp
+++ b/src/shaders/SkLightingShader.cpp
@@ -114,21 +114,9 @@ private:
 // premul'd.
 class LightingFP : public GrFragmentProcessor {
 public:
-    LightingFP(sk_sp<GrFragmentProcessor> normalFP, sk_sp<SkLights> lights)
-            : INHERITED(kPreservesOpaqueInput_OptimizationFlag) {
-        // fuse all ambient lights into a single one
-        fAmbientColor = lights->ambientLightColor();
-        for (int i = 0; i < lights->numLights(); ++i) {
-            if (SkLights::Light::kDirectional_LightType == lights->light(i).type()) {
-                fDirectionalLights.push_back(lights->light(i));
-                // TODO get the handle to the shadow map if there is one
-            } else {
-                SkDEBUGFAIL("Unimplemented Light Type passed to LightingFP");
-            }
-        }
-
-        this->registerChildProcessor(std::move(normalFP));
-        this->initClassID<LightingFP>();
+    static sk_sp<GrFragmentProcessor> Make(sk_sp<GrFragmentProcessor> normalFP,
+                                           sk_sp<SkLights> lights) {
+        return sk_sp<GrFragmentProcessor>(new LightingFP(std::move(normalFP), std::move(lights)));
     }
 
     class GLSLLightingFP : public GrGLSLFragmentProcessor {
@@ -247,6 +235,23 @@ public:
     const SkColor3f& ambientColor() const { return fAmbientColor; }
 
 private:
+    LightingFP(sk_sp<GrFragmentProcessor> normalFP, sk_sp<SkLights> lights)
+            : INHERITED(kPreservesOpaqueInput_OptimizationFlag) {
+        // fuse all ambient lights into a single one
+        fAmbientColor = lights->ambientLightColor();
+        for (int i = 0; i < lights->numLights(); ++i) {
+            if (SkLights::Light::kDirectional_LightType == lights->light(i).type()) {
+                fDirectionalLights.push_back(lights->light(i));
+                // TODO get the handle to the shadow map if there is one
+            } else {
+                SkDEBUGFAIL("Unimplemented Light Type passed to LightingFP");
+            }
+        }
+
+        this->registerChildProcessor(std::move(normalFP));
+        this->initClassID<LightingFP>();
+    }
+
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override { return new GLSLLightingFP; }
 
     bool onIsEqual(const GrFragmentProcessor& proc) const override {
@@ -272,9 +277,9 @@ sk_sp<GrFragmentProcessor> SkLightingShaderImpl::asFragmentProcessor(const AsFPA
     if (fDiffuseShader) {
         sk_sp<GrFragmentProcessor> fpPipeline[] = {
             as_SB(fDiffuseShader)->asFragmentProcessor(args),
-            sk_make_sp<LightingFP>(std::move(normalFP), fLights)
+            LightingFP::Make(std::move(normalFP), fLights)
         };
-        if(!fpPipeline[0]) {
+        if (!fpPipeline[0] || !fpPipeline[1]) {
             return nullptr;
         }
 
@@ -284,8 +289,7 @@ sk_sp<GrFragmentProcessor> SkLightingShaderImpl::asFragmentProcessor(const AsFPA
     } else {
         // FP is wrapped because paint comes in unpremul'd to fragment shader, but LightingFP
         // expects premul'd color.
-        return GrFragmentProcessor::PremulInput(sk_make_sp<LightingFP>(std::move(normalFP),
-                                                                       fLights));
+        return GrFragmentProcessor::PremulInput(LightingFP::Make(std::move(normalFP), fLights));
     }
 }
 
diff --git a/tests/GLProgramsTest.cpp b/tests/GLProgramsTest.cpp
index cfa5561564..4a89d0a9bf 100644
--- a/tests/GLProgramsTest.cpp
+++ b/tests/GLProgramsTest.cpp
@@ -217,7 +217,9 @@ static void set_random_color_coverage_stages(GrPaint* paint,
     const float procTreeProbability = 0.5f;
     if (d->fRandom->nextF() < procTreeProbability) {
         sk_sp<GrFragmentProcessor> fp(create_random_proc_tree(d, 2, maxTreeLevels));
-        paint->addColorFragmentProcessor(std::move(fp));
+        if (fp) {
+            paint->addColorFragmentProcessor(std::move(fp));
+        }
     } else {
         int numProcs = d->fRandom->nextULessThan(maxStages + 1);
         int numColorProcs = d->fRandom->nextULessThan(numProcs + 1);