Workaround stateful tinting bug on <= API 23

Since the framework doesn't guarantee a drawable
invalidation for state changes, we need to workaround
it in DrawableCompat by wrapping the drawable and forcing
one.

BUG: 25174572

Change-Id: Ie793ae2056bd72954d3fa8df4db0fb72bf17de41

7 files changed, 100 insertions, 55 deletions

diff --git a/v4/api21/android/support/v4/graphics/drawable/DrawableCompatLollipop.java b/v4/api21/android/support/v4/graphics/drawable/DrawableCompatLollipop.java
index 93990b47fb..2b1873f4dd 100644
--- a/v4/api21/android/support/v4/graphics/drawable/DrawableCompatLollipop.java
+++ b/v4/api21/android/support/v4/graphics/drawable/DrawableCompatLollipop.java
@@ -37,46 +37,25 @@ class DrawableCompatLollipop {
     }
 
     public static void setTint(Drawable drawable, int tint) {
-        if (drawable instanceof DrawableWrapperLollipop) {
-            // GradientDrawable on Lollipop does not support tinting, so we'll use our compatible
-            // functionality instead
-            DrawableCompatBase.setTint(drawable, tint);
-        } else {
-            // Else, we'll use the framework API
-            drawable.setTint(tint);
-        }
+        drawable.setTint(tint);
     }
 
     public static void setTintList(Drawable drawable, ColorStateList tint) {
-        if (drawable instanceof DrawableWrapperLollipop) {
-            // GradientDrawable on Lollipop does not support tinting, so we'll use our compatible
-            // functionality instead
-            DrawableCompatBase.setTintList(drawable, tint);
-        } else {
-            // Else, we'll use the framework API
-            drawable.setTintList(tint);
-        }
+        drawable.setTintList(tint);
     }
 
     public static void setTintMode(Drawable drawable, PorterDuff.Mode tintMode) {
-        if (drawable instanceof DrawableWrapperLollipop) {
-            // GradientDrawable on Lollipop does not support tinting, so we'll use our compatible
-            // functionality instead
-            DrawableCompatBase.setTintMode(drawable, tintMode);
-        } else {
-            // Else, we'll use the framework API
-            drawable.setTintMode(tintMode);
-        }
+        drawable.setTintMode(tintMode);
     }
 
     public static Drawable wrapForTinting(Drawable drawable) {
-        if (drawable instanceof GradientDrawable || drawable instanceof DrawableContainer) {
-            // GradientDrawable on Lollipop does not support tinting, so we'll use our compatible
-            // functionality instead. We also do the same for DrawableContainers since they may
-            // contain GradientDrawable instances.
-            return new DrawableWrapperLollipop(drawable);
-        }
-        return drawable;
+        return new DrawableWrapperLollipop(drawable, shouldForceCompatTinting(drawable));
     }
 
+    private static boolean shouldForceCompatTinting(Drawable drawable) {
+        // GradientDrawable on Lollipop does not support tinting, so we'll use our compatible
+        // functionality instead. We also do the same for DrawableContainers since they may
+        // contain GradientDrawable instances.
+        return drawable instanceof GradientDrawable || drawable instanceof DrawableContainer;
+    }
 }
diff --git a/v4/api21/android/support/v4/graphics/drawable/DrawableWrapperLollipop.java b/v4/api21/android/support/v4/graphics/drawable/DrawableWrapperLollipop.java
index 1f15040056..9533afd80f 100644
--- a/v4/api21/android/support/v4/graphics/drawable/DrawableWrapperLollipop.java
+++ b/v4/api21/android/support/v4/graphics/drawable/DrawableWrapperLollipop.java
@@ -16,15 +16,24 @@
 
 package android.support.v4.graphics.drawable;
 
+import android.content.res.ColorStateList;
 import android.content.res.Resources;
 import android.graphics.Outline;
+import android.graphics.PorterDuff;
 import android.graphics.Rect;
 import android.graphics.drawable.Drawable;
 
 class DrawableWrapperLollipop extends DrawableWrapperKitKat {
 
+    private final boolean mUseCompatTinting;
+
     DrawableWrapperLollipop(Drawable drawable) {
+        this(drawable, false);
+    }
+
+    DrawableWrapperLollipop(Drawable drawable, boolean useCompatTinting) {
         super(drawable);
+        mUseCompatTinting = useCompatTinting;
     }
 
     @Override
@@ -56,4 +65,47 @@ class DrawableWrapperLollipop extends DrawableWrapperKitKat {
     public Rect getDirtyBounds() {
         return mDrawable.getDirtyBounds();
     }
+
+    @Override
+    public void setTintList(ColorStateList tint) {
+        if (mUseCompatTinting) {
+            setCompatTintList(tint);
+        } else {
+            mDrawable.setTintList(tint);
+        }
+    }
+
+    @Override
+    public void setTint(int tintColor) {
+        if (mUseCompatTinting) {
+            setCompatTint(tintColor);
+        } else {
+            mDrawable.setTint(tintColor);
+        }
+    }
+
+    @Override
+    public void setTintMode(PorterDuff.Mode tintMode) {
+        if (mUseCompatTinting) {
+            setCompatTintMode(tintMode);
+        } else {
+            mDrawable.setTintMode(tintMode);
+        }
+    }
+
+    @Override
+    public boolean setState(int[] stateSet) {
+        if (super.setState(stateSet)) {
+            // Manually invalidate because the framework doesn't currently force an invalidation
+            // on a state change
+            invalidateSelf();
+            return true;
+        }
+        return false;
+    }
+
+    @Override
+    protected boolean isCompatTintEnabled() {
+        return mUseCompatTinting;
+    }
 }
diff --git a/v4/api22/android/support/v4/graphics/drawable/DrawableCompatApi22.java b/v4/api22/android/support/v4/graphics/drawable/DrawableCompatApi22.java
index bfd2bea122..50f265704a 100644
--- a/v4/api22/android/support/v4/graphics/drawable/DrawableCompatApi22.java
+++ b/v4/api22/android/support/v4/graphics/drawable/DrawableCompatApi22.java
@@ -24,8 +24,8 @@ import android.graphics.drawable.Drawable;
 class DrawableCompatApi22 {
 
     public static Drawable wrapForTinting(Drawable drawable) {
-        // We don't need to wrap anything in Lollipop-MR1
-        return drawable;
+        // We need to wrap to force an invalidation on any state change
+        return new DrawableWrapperLollipop(drawable);
     }
 
 }
diff --git a/v4/donut/android/support/v4/graphics/drawable/DrawableCompatBase.java b/v4/donut/android/support/v4/graphics/drawable/DrawableCompatBase.java
index 4809618527..fe0163dd61 100644
--- a/v4/donut/android/support/v4/graphics/drawable/DrawableCompatBase.java
+++ b/v4/donut/android/support/v4/graphics/drawable/DrawableCompatBase.java
@@ -27,19 +27,19 @@ class DrawableCompatBase {
 
     public static void setTint(Drawable drawable, int tint) {
         if (drawable instanceof DrawableWrapper) {
-            ((DrawableWrapper) drawable).setTint(tint);
+            ((DrawableWrapper) drawable).setCompatTint(tint);
         }
     }
 
     public static void setTintList(Drawable drawable, ColorStateList tint) {
         if (drawable instanceof DrawableWrapper) {
-            ((DrawableWrapper) drawable).setTintList(tint);
+            ((DrawableWrapper) drawable).setCompatTintList(tint);
         }
     }
 
     public static void setTintMode(Drawable drawable, PorterDuff.Mode tintMode) {
         if (drawable instanceof DrawableWrapper) {
-            ((DrawableWrapper) drawable).setTintMode(tintMode);
+            ((DrawableWrapper) drawable).setCompatTintMode(tintMode);
         }
     }
 
diff --git a/v4/donut/android/support/v4/graphics/drawable/DrawableWrapper.java b/v4/donut/android/support/v4/graphics/drawable/DrawableWrapper.java
index 1073f344c6..edbe5ad9d0 100644
--- a/v4/donut/android/support/v4/graphics/drawable/DrawableWrapper.java
+++ b/v4/donut/android/support/v4/graphics/drawable/DrawableWrapper.java
@@ -28,11 +28,11 @@ import android.graphics.drawable.Drawable;
  */
 public interface DrawableWrapper {
 
-    void setTint(int tint);
+    void setCompatTint(int tint);
 
-    void setTintList(ColorStateList tint);
+    void setCompatTintList(ColorStateList tint);
 
-    void setTintMode(PorterDuff.Mode tintMode);
+    void setCompatTintMode(PorterDuff.Mode tintMode);
 
     Drawable getWrappedDrawable();
 
diff --git a/v4/donut/android/support/v4/graphics/drawable/DrawableWrapperDonut.java b/v4/donut/android/support/v4/graphics/drawable/DrawableWrapperDonut.java
index 9293520455..36eb41dd7f 100644
--- a/v4/donut/android/support/v4/graphics/drawable/DrawableWrapperDonut.java
+++ b/v4/donut/android/support/v4/graphics/drawable/DrawableWrapperDonut.java
@@ -89,7 +89,8 @@ class DrawableWrapperDonut extends Drawable implements Drawable.Callback, Drawab
 
     @Override
     public boolean isStateful() {
-        return (mTintList != null && mTintList.isStateful()) || mDrawable.isStateful();
+        final ColorStateList tintList = isCompatTintEnabled() ? mTintList : null;
+        return (tintList != null && tintList.isStateful()) || mDrawable.isStateful();
     }
 
     @Override
@@ -188,30 +189,38 @@ class DrawableWrapperDonut extends Drawable implements Drawable.Callback, Drawab
     }
 
     @Override
-    public void setTint(int tint) {
-        setTintList(ColorStateList.valueOf(tint));
+    public void setCompatTint(int tint) {
+        setCompatTintList(ColorStateList.valueOf(tint));
     }
 
     @Override
-    public void setTintList(ColorStateList tint) {
-        mTintList = tint;
-        updateTint(getState());
+    public void setCompatTintList(ColorStateList tint) {
+        if (mTintList != tint) {
+            mTintList = tint;
+            updateTint(getState());
+        }
     }
 
     @Override
-    public void setTintMode(PorterDuff.Mode tintMode) {
-        mTintMode = tintMode;
-        updateTint(getState());
+    public void setCompatTintMode(PorterDuff.Mode tintMode) {
+        if (mTintMode != tintMode) {
+            mTintMode = tintMode;
+            updateTint(getState());
+        }
     }
 
     private boolean updateTint(int[] state) {
+        if (!isCompatTintEnabled()) {
+            // If compat tinting is not enabled, fail fast
+            return false;
+        }
+
         if (mTintList != null && mTintMode != null) {
             final int color = mTintList.getColorForState(state, mTintList.getDefaultColor());
-            final PorterDuff.Mode mode = mTintMode;
-            if (!mColorFilterSet || color != mCurrentColor || mode != mCurrentMode) {
-                setColorFilter(color, mode);
+            if (!mColorFilterSet || color != mCurrentColor || mTintMode != mCurrentMode) {
+                setColorFilter(color, mTintMode);
                 mCurrentColor = color;
-                mCurrentMode = mode;
+                mCurrentMode = mTintMode;
                 mColorFilterSet = true;
                 return true;
             }
@@ -245,4 +254,9 @@ class DrawableWrapperDonut extends Drawable implements Drawable.Callback, Drawab
         // Invalidate ourselves
         invalidateSelf();
     }
+
+    protected boolean isCompatTintEnabled() {
+        // It's enabled by default on Donut
+        return true;
+    }
 }
diff --git a/v7/appcompat/src/android/support/v7/widget/AppCompatDrawableManager.java b/v7/appcompat/src/android/support/v7/widget/AppCompatDrawableManager.java
index ea70a8876f..eb04b01b60 100644
--- a/v7/appcompat/src/android/support/v7/widget/AppCompatDrawableManager.java
+++ b/v7/appcompat/src/android/support/v7/widget/AppCompatDrawableManager.java
@@ -621,9 +621,9 @@ public final class AppCompatDrawableManager {
             drawable.clearColorFilter();
         }
 
-        if (Build.VERSION.SDK_INT <= 10) {
-            // On Gingerbread, GradientDrawable does not invalidate itself when it's
-            // ColorFilter has changed, so we need to force an invalidation
+        if (Build.VERSION.SDK_INT <= 23) {
+            // Pre-v23 there is no guarantee that a state change will invoke an invalidation,
+            // so we force it ourselves
             drawable.invalidateSelf();
         }
     }