From 5f49c3023a512efbef8bc9515d310c7a72be4af2 Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Thu, 6 Sep 2012 16:33:31 -0700 Subject: [PATCH] The drawables cache strikes again Bug #7117785 Draawables created from the ConstantState cache found in Resources must be mutated before they can be safely modified by apps. Failure to do so results in all drawables sharing the same constant state to be affected by the modification. In the case of the bugreport above, the status bar code plays tricks with a background drawable and modifies its color to implement a fade in/out effect. This drawable comes from a cached resource (color 0x0) and the modifications made by the status bar apply to other clients of this drawable, most notably the recents panel. This change fixes several things: - Simplifies colors caching by removing the assetCookie from the key. This should result in better reuse of cached drawables - Makes View.setBackgroundColor() honor the mutate() contract - Ensure StateListDrawable properly mutates its children before modifying them - Optimize Bitmap/ColorDrawable to mark them mutated when they are not created from an existing ConstantSate. The same optimization should be applied to other drawables in the future Change-Id: I54adb5d5b914c7d8930bf9b46f7e3f9dcbf4bcab --- core/java/android/content/res/Resources.java | 4 +++- core/java/android/view/View.java | 2 +- .../graphics/drawable/BitmapDrawable.java | 9 +++++++++ .../graphics/drawable/ColorDrawable.java | 19 +++++++++++++++++++ .../graphics/drawable/DrawableContainer.java | 15 ++++++++------- .../graphics/drawable/GradientDrawable.java | 3 ++- .../graphics/drawable/NinePatchDrawable.java | 7 ++++++- 7 files changed, 48 insertions(+), 11 deletions(-) diff --git a/core/java/android/content/res/Resources.java b/core/java/android/content/res/Resources.java index 42a6bdceaee19..b316f230a944d 100755 --- a/core/java/android/content/res/Resources.java +++ b/core/java/android/content/res/Resources.java @@ -1897,12 +1897,14 @@ public class Resources { } } - final long key = (((long) value.assetCookie) << 32) | value.data; boolean isColorDrawable = false; if (value.type >= TypedValue.TYPE_FIRST_COLOR_INT && value.type <= TypedValue.TYPE_LAST_COLOR_INT) { isColorDrawable = true; } + final long key = isColorDrawable ? value.data : + (((long) value.assetCookie) << 32) | value.data; + Drawable dr = getCachedDrawable(isColorDrawable ? mColorDrawableCache : mDrawableCache, key); if (dr != null) { diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index b6f07fac55132..4d7e58ce0a25f 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -14107,7 +14107,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback, @RemotableViewMethod public void setBackgroundColor(int color) { if (mBackground instanceof ColorDrawable) { - ((ColorDrawable) mBackground).setColor(color); + ((ColorDrawable) mBackground.mutate()).setColor(color); computeOpaqueFlags(); } else { setBackground(new ColorDrawable(color)); diff --git a/graphics/java/android/graphics/drawable/BitmapDrawable.java b/graphics/java/android/graphics/drawable/BitmapDrawable.java index e82ccd4988b1b..7a4a1ca92ad17 100644 --- a/graphics/java/android/graphics/drawable/BitmapDrawable.java +++ b/graphics/java/android/graphics/drawable/BitmapDrawable.java @@ -80,6 +80,7 @@ public class BitmapDrawable extends Drawable { @Deprecated public BitmapDrawable() { mBitmapState = new BitmapState((Bitmap) null); + mMutated = true; } /** @@ -90,6 +91,7 @@ public class BitmapDrawable extends Drawable { public BitmapDrawable(Resources res) { mBitmapState = new BitmapState((Bitmap) null); mBitmapState.mTargetDensity = mTargetDensity; + mMutated = true; } /** @@ -100,6 +102,7 @@ public class BitmapDrawable extends Drawable { @Deprecated public BitmapDrawable(Bitmap bitmap) { this(new BitmapState(bitmap), null); + mMutated = true; } /** @@ -109,6 +112,7 @@ public class BitmapDrawable extends Drawable { public BitmapDrawable(Resources res, Bitmap bitmap) { this(new BitmapState(bitmap), res); mBitmapState.mTargetDensity = mTargetDensity; + mMutated = true; } /** @@ -122,6 +126,7 @@ public class BitmapDrawable extends Drawable { if (mBitmap == null) { android.util.Log.w("BitmapDrawable", "BitmapDrawable cannot decode " + filepath); } + mMutated = true; } /** @@ -134,6 +139,7 @@ public class BitmapDrawable extends Drawable { if (mBitmap == null) { android.util.Log.w("BitmapDrawable", "BitmapDrawable cannot decode " + filepath); } + mMutated = true; } /** @@ -147,6 +153,7 @@ public class BitmapDrawable extends Drawable { if (mBitmap == null) { android.util.Log.w("BitmapDrawable", "BitmapDrawable cannot decode " + is); } + mMutated = true; } /** @@ -159,6 +166,7 @@ public class BitmapDrawable extends Drawable { if (mBitmap == null) { android.util.Log.w("BitmapDrawable", "BitmapDrawable cannot decode " + is); } + mMutated = true; } /** @@ -552,6 +560,7 @@ public class BitmapDrawable extends Drawable { } else { mTargetDensity = state.mTargetDensity; } + mMutated = false; setBitmap(state != null ? state.mBitmap : null); } } diff --git a/graphics/java/android/graphics/drawable/ColorDrawable.java b/graphics/java/android/graphics/drawable/ColorDrawable.java index 88c91559dcbbf..f8e3944ca6cb1 100644 --- a/graphics/java/android/graphics/drawable/ColorDrawable.java +++ b/graphics/java/android/graphics/drawable/ColorDrawable.java @@ -36,12 +36,14 @@ import java.io.IOException; public class ColorDrawable extends Drawable { private ColorState mState; private final Paint mPaint = new Paint(); + private boolean mMutated; /** * Creates a new black ColorDrawable. */ public ColorDrawable() { this(null); + mMutated = true; } /** @@ -52,6 +54,7 @@ public class ColorDrawable extends Drawable { public ColorDrawable(int color) { this(null); setColor(color); + mMutated = true; } private ColorDrawable(ColorState state) { @@ -63,6 +66,21 @@ public class ColorDrawable extends Drawable { return super.getChangingConfigurations() | mState.mChangingConfigurations; } + /** + * A mutable BitmapDrawable still shares its Bitmap with any other Drawable + * that comes from the same resource. + * + * @return This drawable. + */ + @Override + public Drawable mutate() { + if (!mMutated && super.mutate() == this) { + mState = new ColorState(mState); + mMutated = true; + } + return this; + } + @Override public void draw(Canvas canvas) { if ((mState.mUseColor >>> 24) != 0) { @@ -165,6 +183,7 @@ public class ColorDrawable extends Drawable { if (state != null) { mBaseColor = state.mBaseColor; mUseColor = state.mUseColor; + mChangingConfigurations = state.mChangingConfigurations; } } diff --git a/graphics/java/android/graphics/drawable/DrawableContainer.java b/graphics/java/android/graphics/drawable/DrawableContainer.java index 15b2c0b7586b6..486390c4e932a 100644 --- a/graphics/java/android/graphics/drawable/DrawableContainer.java +++ b/graphics/java/android/graphics/drawable/DrawableContainer.java @@ -105,7 +105,7 @@ public class DrawableContainer extends Drawable implements Drawable.Callback { mAlpha = alpha; if (mCurrDrawable != null) { if (mEnterAnimationEnd == 0) { - mCurrDrawable.setAlpha(alpha); + mCurrDrawable.mutate().setAlpha(alpha); } else { animate(false); } @@ -118,7 +118,7 @@ public class DrawableContainer extends Drawable implements Drawable.Callback { if (mDrawableContainerState.mDither != dither) { mDrawableContainerState.mDither = dither; if (mCurrDrawable != null) { - mCurrDrawable.setDither(mDrawableContainerState.mDither); + mCurrDrawable.mutate().setDither(mDrawableContainerState.mDither); } } } @@ -128,7 +128,7 @@ public class DrawableContainer extends Drawable implements Drawable.Callback { if (mColorFilter != cf) { mColorFilter = cf; if (mCurrDrawable != null) { - mCurrDrawable.setColorFilter(cf); + mCurrDrawable.mutate().setColorFilter(cf); } } } @@ -176,7 +176,7 @@ public class DrawableContainer extends Drawable implements Drawable.Callback { } if (mCurrDrawable != null) { mCurrDrawable.jumpToCurrentState(); - mCurrDrawable.setAlpha(mAlpha); + mCurrDrawable.mutate().setAlpha(mAlpha); } if (mExitAnimationEnd != 0) { mExitAnimationEnd = 0; @@ -312,6 +312,7 @@ public class DrawableContainer extends Drawable implements Drawable.Callback { mCurrDrawable = d; mCurIndex = idx; if (d != null) { + d.mutate(); if (mDrawableContainerState.mEnterFadeDuration > 0) { mEnterAnimationEnd = now + mDrawableContainerState.mEnterFadeDuration; } else { @@ -355,13 +356,13 @@ public class DrawableContainer extends Drawable implements Drawable.Callback { if (mCurrDrawable != null) { if (mEnterAnimationEnd != 0) { if (mEnterAnimationEnd <= now) { - mCurrDrawable.setAlpha(mAlpha); + mCurrDrawable.mutate().setAlpha(mAlpha); mEnterAnimationEnd = 0; } else { int animAlpha = (int)((mEnterAnimationEnd-now)*255) / mDrawableContainerState.mEnterFadeDuration; if (DEBUG) android.util.Log.i(TAG, toString() + " cur alpha " + animAlpha); - mCurrDrawable.setAlpha(((255-animAlpha)*mAlpha)/255); + mCurrDrawable.mutate().setAlpha(((255-animAlpha)*mAlpha)/255); animating = true; } } @@ -378,7 +379,7 @@ public class DrawableContainer extends Drawable implements Drawable.Callback { int animAlpha = (int)((mExitAnimationEnd-now)*255) / mDrawableContainerState.mExitFadeDuration; if (DEBUG) android.util.Log.i(TAG, toString() + " last alpha " + animAlpha); - mLastDrawable.setAlpha((animAlpha*mAlpha)/255); + mLastDrawable.mutate().setAlpha((animAlpha*mAlpha)/255); animating = true; } } diff --git a/graphics/java/android/graphics/drawable/GradientDrawable.java b/graphics/java/android/graphics/drawable/GradientDrawable.java index 5b50bebd64ace..21344f4557dd2 100644 --- a/graphics/java/android/graphics/drawable/GradientDrawable.java +++ b/graphics/java/android/graphics/drawable/GradientDrawable.java @@ -124,7 +124,7 @@ public class GradientDrawable extends Drawable { private Paint mLayerPaint; // internal, used if we use saveLayer() private boolean mRectIsDirty; // internal state - private boolean mMutated; + private boolean mMutated = true; private Path mRingPath; private boolean mPathIsDirty = true; @@ -1202,6 +1202,7 @@ public class GradientDrawable extends Drawable { mGradientState = state; initializeWithState(state); mRectIsDirty = true; + mMutated = false; } private void initializeWithState(GradientState state) { diff --git a/graphics/java/android/graphics/drawable/NinePatchDrawable.java b/graphics/java/android/graphics/drawable/NinePatchDrawable.java index 62aea7102d9a5..7a434968b2531 100644 --- a/graphics/java/android/graphics/drawable/NinePatchDrawable.java +++ b/graphics/java/android/graphics/drawable/NinePatchDrawable.java @@ -77,6 +77,7 @@ public class NinePatchDrawable extends Drawable { @Deprecated public NinePatchDrawable(Bitmap bitmap, byte[] chunk, Rect padding, String srcName) { this(new NinePatchState(new NinePatch(bitmap, chunk, srcName), padding), null); + mMutated = true; } /** @@ -87,6 +88,7 @@ public class NinePatchDrawable extends Drawable { Rect padding, String srcName) { this(new NinePatchState(new NinePatch(bitmap, chunk, srcName), padding), res); mNinePatchState.mTargetDensity = mTargetDensity; + mMutated = true; } /** @@ -99,6 +101,7 @@ public class NinePatchDrawable extends Drawable { Rect padding, Rect layoutInsets, String srcName) { this(new NinePatchState(new NinePatch(bitmap, chunk, srcName), padding, layoutInsets), res); mNinePatchState.mTargetDensity = mTargetDensity; + mMutated = true; } /** @@ -109,6 +112,7 @@ public class NinePatchDrawable extends Drawable { @Deprecated public NinePatchDrawable(NinePatch patch) { this(new NinePatchState(patch, new Rect()), null); + mMutated = true; } /** @@ -118,6 +122,7 @@ public class NinePatchDrawable extends Drawable { public NinePatchDrawable(Resources res, NinePatch patch) { this(new NinePatchState(patch, new Rect()), res); mNinePatchState.mTargetDensity = mTargetDensity; + mMutated = true; } private void setNinePatchState(NinePatchState state, Resources res) { @@ -181,7 +186,7 @@ public class NinePatchDrawable extends Drawable { } } - private Insets scaleFromDensity(Insets insets, int sdensity, int tdensity) { + private static Insets scaleFromDensity(Insets insets, int sdensity, int tdensity) { int left = Bitmap.scaleFromDensity(insets.left, sdensity, tdensity); int top = Bitmap.scaleFromDensity(insets.top, sdensity, tdensity); int right = Bitmap.scaleFromDensity(insets.right, sdensity, tdensity);