diff options
author | erfanian <erfanian@google.com> | 2017-06-30 15:34:18 -0700 |
---|---|---|
committer | Eric Erfanian <erfanian@google.com> | 2017-07-10 10:43:34 -0700 |
commit | ad26445126c237c226d0b65fc1dad1b5ca41ef77 (patch) | |
tree | 5e93ffef035236e0ad1db87aae62498a289dbd68 /java/com/android/contacts/common | |
parent | 61de7a383e67619b1c5fab26f81aef451438c9b8 (diff) |
Fix concurrency issue in constructor.
Before:
sColors served as the branch to initialize all member variables.
Subsequent calls to the constructor after sColors had been initialized,
would result in the use of static members that were not yet initialized.
Now:
We guard each reference with an explicit null check during construct.
Members that require synchronized initialization were put into a small synchronized
method.
This was chosen instead of AtomicReferences, and instead of double checked locking,
because -- although verbose -- we can tolerate a small number of overwrites to
each member variable (they are idempotent within the Application scope), thus avoiding the need for any one thread to wait/acquire a lock, as well as avoiding the need to import the Atomic library (which added no incremental benefit).
It is assumed that the JVM will not garbage collect overwritten references to
member variables that are still in use across instances of LetterTileDrawable.
This shouldn't matter because the references are volatile, anyway.
Initialization-on-demand was not available due to the use of non-static
resources on construct.
Bug: 63143138
Test: No. Old unit tests.
PiperOrigin-RevId: 160696979
Change-Id: Ie17a29e48f91cb3df07d81d29b6428b70fb4408a
Diffstat (limited to 'java/com/android/contacts/common')
-rw-r--r-- | java/com/android/contacts/common/lettertiles/LetterTileDrawable.java | 83 |
1 files changed, 57 insertions, 26 deletions
diff --git a/java/com/android/contacts/common/lettertiles/LetterTileDrawable.java b/java/com/android/contacts/common/lettertiles/LetterTileDrawable.java index 5c401fe38..5b7dcbe84 100644 --- a/java/com/android/contacts/common/lettertiles/LetterTileDrawable.java +++ b/java/com/android/contacts/common/lettertiles/LetterTileDrawable.java @@ -87,23 +87,23 @@ public class LetterTileDrawable extends Drawable { /** Default icon scale for vector drawable. */ private static final float VECTOR_ICON_SCALE = 0.7f; - /** Reusable components to avoid new allocations */ - private static final Paint sPaint = new Paint(); + private static Paint sPaint; private static final Rect sRect = new Rect(); private static final char[] sFirstChar = new char[1]; - /** Letter tile */ - private static TypedArray sColors; - - private static int sSpamColor; - private static int sDefaultColor; - private static int sTileFontColor; - private static float sLetterToTileRatio; - private static Drawable sDefaultPersonAvatar; - private static Drawable sDefaultBusinessAvatar; - private static Drawable sDefaultVoicemailAvatar; - private static Drawable sDefaultSpamAvatar; - private static Drawable sDefaultConferenceAvatar; + + private static volatile Integer sSpamColor; + private static volatile Integer sDefaultColor; + private static volatile Integer sTileFontColor; + private static volatile Float sLetterToTileRatio; + + private static volatile TypedArray sColors; + + private static volatile Drawable sDefaultPersonAvatar; + private static volatile Drawable sDefaultBusinessAvatar; + private static volatile Drawable sDefaultVoicemailAvatar; + private static volatile Drawable sDefaultSpamAvatar; + private static volatile Drawable sDefaultConferenceAvatar; private final Paint mPaint; @ContactType private int mContactType = TYPE_DEFAULT; @@ -117,35 +117,66 @@ public class LetterTileDrawable extends Drawable { private String mDisplayName; public LetterTileDrawable(final Resources res) { - if (sColors == null) { - sColors = res.obtainTypedArray(R.array.letter_tile_colors); + // Guard static members with null checks to insulate against concurrent calls to the + // constructor. + if (sSpamColor == null) { sSpamColor = res.getColor(R.color.spam_contact_background); + } + if (sDefaultColor == null) { sDefaultColor = res.getColor(R.color.letter_tile_default_color); + } + if (sTileFontColor == null) { sTileFontColor = res.getColor(R.color.letter_tile_font_color); + } + if (sTileFontColor == null) { sLetterToTileRatio = res.getFraction(R.dimen.letter_to_tile_ratio, 1, 1); + } + + if (sColors == null) { + sColors = res.obtainTypedArray(R.array.letter_tile_colors); + } + if (sDefaultPersonAvatar == null) { sDefaultPersonAvatar = res.getDrawable(R.drawable.product_logo_avatar_anonymous_white_color_120, null); - Assert.isNotNull(sDefaultPersonAvatar, "sDefaultPersonAvatar is null"); + } + if (sDefaultBusinessAvatar == null) { sDefaultBusinessAvatar = res.getDrawable(R.drawable.quantum_ic_business_vd_theme_24, null); - Assert.isNotNull(sDefaultBusinessAvatar, "sDefaultBusinessAvatar is null"); + } + if (sDefaultVoicemailAvatar == null) { sDefaultVoicemailAvatar = res.getDrawable(R.drawable.quantum_ic_voicemail_vd_theme_24, null); - Assert.isNotNull(sDefaultVoicemailAvatar, "sDefaultVoicemailAvatar is null"); + } + if (sDefaultSpamAvatar == null) { sDefaultSpamAvatar = res.getDrawable(R.drawable.quantum_ic_report_vd_theme_24, null); - Assert.isNotNull(sDefaultSpamAvatar, "sDefaultSpamAvatar is null"); + } + if (sDefaultConferenceAvatar == null) { sDefaultConferenceAvatar = res.getDrawable(R.drawable.quantum_ic_group_vd_theme_24, null); - Assert.isNotNull(sDefaultConferenceAvatar, "sDefaultConferenceAvatar is null"); - - sPaint.setTypeface( - Typeface.create(res.getString(R.string.letter_tile_letter_font_family), Typeface.NORMAL)); - sPaint.setTextAlign(Align.CENTER); - sPaint.setAntiAlias(true); } + + // Calls in this method are potentially not idempotent, and need to be set after + // the member is initilaized. + this.initializeDefaultPaintOptions(res); + + // These values should be reset on every call to the constructor. mPaint = new Paint(); mPaint.setFilterBitmap(true); mPaint.setDither(true); + mColor = sDefaultColor; } + private static void initializeDefaultPaintOptions(final Resources res) { + if (sPaint == null) { + synchronized (LetterTileDrawable.class) { + sPaint = new Paint(); + sPaint.setTypeface( + Typeface.create( + res.getString(R.string.letter_tile_letter_font_family), Typeface.NORMAL)); + sPaint.setTextAlign(Align.CENTER); + sPaint.setAntiAlias(true); + } + } + } + private Rect getScaledBounds(float scale, float offset) { // The drawable should be drawn in the middle of the canvas without changing its width to // height ratio. |