Skip to content

Commit 5f49c30

Browse files
author
Romain Guy
committed
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
1 parent 369bb97 commit 5f49c30

File tree

7 files changed

+48
-11
lines changed

7 files changed

+48
-11
lines changed

‎core/java/android/content/res/Resources.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1897,12 +1897,14 @@ private boolean verifyPreloadConfig(TypedValue value, String name) {
18971897
}
18981898
}
18991899

1900-
final long key = (((long) value.assetCookie) << 32) | value.data;
19011900
boolean isColorDrawable = false;
19021901
if (value.type >= TypedValue.TYPE_FIRST_COLOR_INT &&
19031902
value.type <= TypedValue.TYPE_LAST_COLOR_INT) {
19041903
isColorDrawable = true;
19051904
}
1905+
final long key = isColorDrawable ? value.data :
1906+
(((long) value.assetCookie) << 32) | value.data;
1907+
19061908
Drawable dr = getCachedDrawable(isColorDrawable ? mColorDrawableCache : mDrawableCache, key);
19071909

19081910
if (dr != null) {

‎core/java/android/view/View.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14107,7 +14107,7 @@ public void jumpDrawablesToCurrentState() {
1410714107
@RemotableViewMethod
1410814108
public void setBackgroundColor(int color) {
1410914109
if (mBackground instanceof ColorDrawable) {
14110-
((ColorDrawable) mBackground).setColor(color);
14110+
((ColorDrawable) mBackground.mutate()).setColor(color);
1411114111
computeOpaqueFlags();
1411214112
} else {
1411314113
setBackground(new ColorDrawable(color));

‎graphics/java/android/graphics/drawable/BitmapDrawable.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ public class BitmapDrawable extends Drawable {
8080
@Deprecated
8181
public BitmapDrawable() {
8282
mBitmapState = new BitmapState((Bitmap) null);
83+
mMutated = true;
8384
}
8485

8586
/**
@@ -90,6 +91,7 @@ public BitmapDrawable() {
9091
public BitmapDrawable(Resources res) {
9192
mBitmapState = new BitmapState((Bitmap) null);
9293
mBitmapState.mTargetDensity = mTargetDensity;
94+
mMutated = true;
9395
}
9496

9597
/**
@@ -100,6 +102,7 @@ public BitmapDrawable(Resources res) {
100102
@Deprecated
101103
public BitmapDrawable(Bitmap bitmap) {
102104
this(new BitmapState(bitmap), null);
105+
mMutated = true;
103106
}
104107

105108
/**
@@ -109,6 +112,7 @@ public BitmapDrawable(Bitmap bitmap) {
109112
public BitmapDrawable(Resources res, Bitmap bitmap) {
110113
this(new BitmapState(bitmap), res);
111114
mBitmapState.mTargetDensity = mTargetDensity;
115+
mMutated = true;
112116
}
113117

114118
/**
@@ -122,6 +126,7 @@ public BitmapDrawable(String filepath) {
122126
if (mBitmap == null) {
123127
android.util.Log.w("BitmapDrawable", "BitmapDrawable cannot decode " + filepath);
124128
}
129+
mMutated = true;
125130
}
126131

127132
/**
@@ -134,6 +139,7 @@ public BitmapDrawable(Resources res, String filepath) {
134139
if (mBitmap == null) {
135140
android.util.Log.w("BitmapDrawable", "BitmapDrawable cannot decode " + filepath);
136141
}
142+
mMutated = true;
137143
}
138144

139145
/**
@@ -147,6 +153,7 @@ public BitmapDrawable(java.io.InputStream is) {
147153
if (mBitmap == null) {
148154
android.util.Log.w("BitmapDrawable", "BitmapDrawable cannot decode " + is);
149155
}
156+
mMutated = true;
150157
}
151158

152159
/**
@@ -159,6 +166,7 @@ public BitmapDrawable(Resources res, java.io.InputStream is) {
159166
if (mBitmap == null) {
160167
android.util.Log.w("BitmapDrawable", "BitmapDrawable cannot decode " + is);
161168
}
169+
mMutated = true;
162170
}
163171

164172
/**
@@ -552,6 +560,7 @@ private BitmapDrawable(BitmapState state, Resources res) {
552560
} else {
553561
mTargetDensity = state.mTargetDensity;
554562
}
563+
mMutated = false;
555564
setBitmap(state != null ? state.mBitmap : null);
556565
}
557566
}

‎graphics/java/android/graphics/drawable/ColorDrawable.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,14 @@
3636
public class ColorDrawable extends Drawable {
3737
private ColorState mState;
3838
private final Paint mPaint = new Paint();
39+
private boolean mMutated;
3940

4041
/**
4142
* Creates a new black ColorDrawable.
4243
*/
4344
public ColorDrawable() {
4445
this(null);
46+
mMutated = true;
4547
}
4648

4749
/**
@@ -52,6 +54,7 @@ public ColorDrawable() {
5254
public ColorDrawable(int color) {
5355
this(null);
5456
setColor(color);
57+
mMutated = true;
5558
}
5659

5760
private ColorDrawable(ColorState state) {
@@ -63,6 +66,21 @@ public int getChangingConfigurations() {
6366
return super.getChangingConfigurations() | mState.mChangingConfigurations;
6467
}
6568

69+
/**
70+
* A mutable BitmapDrawable still shares its Bitmap with any other Drawable
71+
* that comes from the same resource.
72+
*
73+
* @return This drawable.
74+
*/
75+
@Override
76+
public Drawable mutate() {
77+
if (!mMutated && super.mutate() == this) {
78+
mState = new ColorState(mState);
79+
mMutated = true;
80+
}
81+
return this;
82+
}
83+
6684
@Override
6785
public void draw(Canvas canvas) {
6886
if ((mState.mUseColor >>> 24) != 0) {
@@ -165,6 +183,7 @@ final static class ColorState extends ConstantState {
165183
if (state != null) {
166184
mBaseColor = state.mBaseColor;
167185
mUseColor = state.mUseColor;
186+
mChangingConfigurations = state.mChangingConfigurations;
168187
}
169188
}
170189

‎graphics/java/android/graphics/drawable/DrawableContainer.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public void setAlpha(int alpha) {
105105
mAlpha = alpha;
106106
if (mCurrDrawable != null) {
107107
if (mEnterAnimationEnd == 0) {
108-
mCurrDrawable.setAlpha(alpha);
108+
mCurrDrawable.mutate().setAlpha(alpha);
109109
} else {
110110
animate(false);
111111
}
@@ -118,7 +118,7 @@ public void setDither(boolean dither) {
118118
if (mDrawableContainerState.mDither != dither) {
119119
mDrawableContainerState.mDither = dither;
120120
if (mCurrDrawable != null) {
121-
mCurrDrawable.setDither(mDrawableContainerState.mDither);
121+
mCurrDrawable.mutate().setDither(mDrawableContainerState.mDither);
122122
}
123123
}
124124
}
@@ -128,7 +128,7 @@ public void setColorFilter(ColorFilter cf) {
128128
if (mColorFilter != cf) {
129129
mColorFilter = cf;
130130
if (mCurrDrawable != null) {
131-
mCurrDrawable.setColorFilter(cf);
131+
mCurrDrawable.mutate().setColorFilter(cf);
132132
}
133133
}
134134
}
@@ -176,7 +176,7 @@ public void jumpToCurrentState() {
176176
}
177177
if (mCurrDrawable != null) {
178178
mCurrDrawable.jumpToCurrentState();
179-
mCurrDrawable.setAlpha(mAlpha);
179+
mCurrDrawable.mutate().setAlpha(mAlpha);
180180
}
181181
if (mExitAnimationEnd != 0) {
182182
mExitAnimationEnd = 0;
@@ -312,6 +312,7 @@ public boolean selectDrawable(int idx) {
312312
mCurrDrawable = d;
313313
mCurIndex = idx;
314314
if (d != null) {
315+
d.mutate();
315316
if (mDrawableContainerState.mEnterFadeDuration > 0) {
316317
mEnterAnimationEnd = now + mDrawableContainerState.mEnterFadeDuration;
317318
} else {
@@ -355,13 +356,13 @@ void animate(boolean schedule) {
355356
if (mCurrDrawable != null) {
356357
if (mEnterAnimationEnd != 0) {
357358
if (mEnterAnimationEnd <= now) {
358-
mCurrDrawable.setAlpha(mAlpha);
359+
mCurrDrawable.mutate().setAlpha(mAlpha);
359360
mEnterAnimationEnd = 0;
360361
} else {
361362
int animAlpha = (int)((mEnterAnimationEnd-now)*255)
362363
/ mDrawableContainerState.mEnterFadeDuration;
363364
if (DEBUG) android.util.Log.i(TAG, toString() + " cur alpha " + animAlpha);
364-
mCurrDrawable.setAlpha(((255-animAlpha)*mAlpha)/255);
365+
mCurrDrawable.mutate().setAlpha(((255-animAlpha)*mAlpha)/255);
365366
animating = true;
366367
}
367368
}
@@ -378,7 +379,7 @@ void animate(boolean schedule) {
378379
int animAlpha = (int)((mExitAnimationEnd-now)*255)
379380
/ mDrawableContainerState.mExitFadeDuration;
380381
if (DEBUG) android.util.Log.i(TAG, toString() + " last alpha " + animAlpha);
381-
mLastDrawable.setAlpha((animAlpha*mAlpha)/255);
382+
mLastDrawable.mutate().setAlpha((animAlpha*mAlpha)/255);
382383
animating = true;
383384
}
384385
}

‎graphics/java/android/graphics/drawable/GradientDrawable.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ public class GradientDrawable extends Drawable {
124124

125125
private Paint mLayerPaint; // internal, used if we use saveLayer()
126126
private boolean mRectIsDirty; // internal state
127-
private boolean mMutated;
127+
private boolean mMutated = true;
128128
private Path mRingPath;
129129
private boolean mPathIsDirty = true;
130130

@@ -1202,6 +1202,7 @@ private GradientDrawable(GradientState state) {
12021202
mGradientState = state;
12031203
initializeWithState(state);
12041204
mRectIsDirty = true;
1205+
mMutated = false;
12051206
}
12061207

12071208
private void initializeWithState(GradientState state) {

‎graphics/java/android/graphics/drawable/NinePatchDrawable.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ public class NinePatchDrawable extends Drawable {
7777
@Deprecated
7878
public NinePatchDrawable(Bitmap bitmap, byte[] chunk, Rect padding, String srcName) {
7979
this(new NinePatchState(new NinePatch(bitmap, chunk, srcName), padding), null);
80+
mMutated = true;
8081
}
8182

8283
/**
@@ -87,6 +88,7 @@ public NinePatchDrawable(Resources res, Bitmap bitmap, byte[] chunk,
8788
Rect padding, String srcName) {
8889
this(new NinePatchState(new NinePatch(bitmap, chunk, srcName), padding), res);
8990
mNinePatchState.mTargetDensity = mTargetDensity;
91+
mMutated = true;
9092
}
9193

9294
/**
@@ -99,6 +101,7 @@ public NinePatchDrawable(Resources res, Bitmap bitmap, byte[] chunk,
99101
Rect padding, Rect layoutInsets, String srcName) {
100102
this(new NinePatchState(new NinePatch(bitmap, chunk, srcName), padding, layoutInsets), res);
101103
mNinePatchState.mTargetDensity = mTargetDensity;
104+
mMutated = true;
102105
}
103106

104107
/**
@@ -109,6 +112,7 @@ public NinePatchDrawable(Resources res, Bitmap bitmap, byte[] chunk,
109112
@Deprecated
110113
public NinePatchDrawable(NinePatch patch) {
111114
this(new NinePatchState(patch, new Rect()), null);
115+
mMutated = true;
112116
}
113117

114118
/**
@@ -118,6 +122,7 @@ public NinePatchDrawable(NinePatch patch) {
118122
public NinePatchDrawable(Resources res, NinePatch patch) {
119123
this(new NinePatchState(patch, new Rect()), res);
120124
mNinePatchState.mTargetDensity = mTargetDensity;
125+
mMutated = true;
121126
}
122127

123128
private void setNinePatchState(NinePatchState state, Resources res) {
@@ -181,7 +186,7 @@ public void setTargetDensity(int density) {
181186
}
182187
}
183188

184-
private Insets scaleFromDensity(Insets insets, int sdensity, int tdensity) {
189+
private static Insets scaleFromDensity(Insets insets, int sdensity, int tdensity) {
185190
int left = Bitmap.scaleFromDensity(insets.left, sdensity, tdensity);
186191
int top = Bitmap.scaleFromDensity(insets.top, sdensity, tdensity);
187192
int right = Bitmap.scaleFromDensity(insets.right, sdensity, tdensity);

0 commit comments

Comments
 (0)