Alter SkXfermode's asFragmentProcessor & asXPFactory contracts

src/core/SkXfermode.cpp

Index: src/core/SkXfermode.cpp
diff --git a/src/core/SkXfermode.cpp b/src/core/SkXfermode.cpp
index 86e3ed5dcf8e579b9e3742b9026c551758a90731..1b2993ad75110076765dd64251f2c8f883336fb4 100644
--- a/src/core/SkXfermode.cpp
+++ b/src/core/SkXfermode.cpp
@@ -650,13 +650,18 @@ bool SkXfermode::asMode(Mode* mode) const {
     return false;
 }
 
-bool SkXfermode::asFragmentProcessor(const GrFragmentProcessor**,
-                                     const GrFragmentProcessor*) const {
-    return false;
+const GrFragmentProcessor* SkXfermode::asFragmentProcessor(const GrFragmentProcessor*) const {
+    // This should never be called.
+    // TODO: make pure virtual in SkXfermode once Android update lands
+    SkASSERT(0);
+    return nullptr;
 }
 
-bool SkXfermode::asXPFactory(GrXPFactory**) const {
-    return false;
+GrXPFactory* SkXfermode::asXPFactory() const {
+    // This should never be called.
+    // TODO: make pure virtual in SkXfermode once Android update lands
+    SkASSERT(0);
+    return nullptr;
 }
 
 SkPMColor SkXfermode::xferColor(SkPMColor src, SkPMColor dst) const{
@@ -902,33 +907,20 @@ void SkProcCoeffXfermode::xferA8(SkAlpha* SK_RESTRICT dst,
 #include "effects/GrPorterDuffXferProcessor.h"
 #include "effects/GrXfermodeFragmentProcessor.h"
 
-bool SkProcCoeffXfermode::asFragmentProcessor(const GrFragmentProcessor** fp,
-                                              const GrFragmentProcessor* dst) const {
-    if (fp) {
-        SkASSERT(dst);
-        *fp = GrXfermodeFragmentProcessor::CreateFromDstProcessor(dst, fMode);
-        SkASSERT(*fp || kSrc_Mode == fMode);
-    }
-    return true;
+const GrFragmentProcessor* SkProcCoeffXfermode::asFragmentProcessor(
+                                                            const GrFragmentProcessor* dst) const {
+    SkASSERT(dst);
+    return GrXfermodeFragmentProcessor::CreateFromDstProcessor(dst, fMode);
 }
 
-bool SkProcCoeffXfermode::asXPFactory(GrXPFactory** xp) const {
+GrXPFactory* SkProcCoeffXfermode::asXPFactory() const {
     if (CANNOT_USE_COEFF != fSrcCoeff) {
-        if (xp) {
-            *xp = GrPorterDuffXPFactory::Create(fMode);
-            SkASSERT(*xp);
-        }
-        return true;
+        SkASSERT(fMode <= kLastCoeffMode);

[egdaniel, 2016/02/08 15:32:11]

I feel like this assert assumes knowledge of exactly what GrPorterDuffXPFactory
will or will not accept. Would it not be safer to do something like:

"return SkAssertResult(GrPDXPF::Create(fMode));"

[robertphillips, 2016/02/08 15:54:53]

Done.

+        return GrPorterDuffXPFactory::Create(fMode);
     }
 
-    if (GrCustomXfermode::IsSupportedMode(fMode)) {
-        if (xp) {
-            *xp = GrCustomXfermode::CreateXPFactory(fMode);
-            SkASSERT(*xp);
-        }
-        return true;
-    }
-    return false;
+    SkASSERT(GrCustomXfermode::IsSupportedMode(fMode));
+    return GrCustomXfermode::CreateXPFactory(fMode);
 }
 #endif