Make Typeface.create thread safe.
Typeface.create has not been thread-safe. This CL makes it thread safe.
At the same time, reorganize cache and lock in Typeface.
Bug: 64606109
Test: bit FrameworksCoreTests:android.graphics.TypefaceTest
Test: bit CtsGraphicsTestCases:android.graphics.cts.TypefaceTest
Change-Id: Id18e9b8bfab508a26fc0c83af6b1ca97e132b732
diff --git a/core/tests/coretests/src/android/graphics/TypefaceTest.java b/core/tests/coretests/src/android/graphics/TypefaceTest.java
index c8ce884..b0c7976 100644
--- a/core/tests/coretests/src/android/graphics/TypefaceTest.java
+++ b/core/tests/coretests/src/android/graphics/TypefaceTest.java
@@ -16,15 +16,29 @@
package android.graphics;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import android.content.Context;
+import android.content.res.AssetManager;
+import android.content.res.Resources;
import android.graphics.Paint;
import android.graphics.Typeface;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.runner.AndroidJUnit4;
+import android.test.suitebuilder.annotation.LargeTest;
import android.test.suitebuilder.annotation.MediumTest;
import android.test.suitebuilder.annotation.SmallTest;
-import junit.framework.TestCase;
+import com.android.frameworks.coretests.R;
+import org.junit.Test;
+import org.junit.runner.RunWith;
-public class TypefaceTest extends TestCase {
+import java.util.Random;
+
+@RunWith(AndroidJUnit4.class)
+public class TypefaceTest {
// create array of all std faces
private final Typeface[] mFaces = new Typeface[] {
@@ -36,8 +50,9 @@
Typeface.create(Typeface.SERIF, 3),
Typeface.create(Typeface.MONOSPACE, 0)
};
-
+
@SmallTest
+ @Test
public void testBasic() throws Exception {
assertTrue("basic", Typeface.DEFAULT != null);
assertTrue("basic", Typeface.DEFAULT_BOLD != null);
@@ -45,8 +60,9 @@
assertTrue("basic", Typeface.SERIF != null);
assertTrue("basic", Typeface.MONOSPACE != null);
}
-
+
@SmallTest
+ @Test
public void testUnique() throws Exception {
final int n = mFaces.length;
for (int i = 0; i < n; i++) {
@@ -57,6 +73,7 @@
}
@SmallTest
+ @Test
public void testStyles() throws Exception {
assertTrue("style", mFaces[0].getStyle() == Typeface.NORMAL);
assertTrue("style", mFaces[1].getStyle() == Typeface.BOLD);
@@ -68,6 +85,7 @@
}
@MediumTest
+ @Test
public void testUniformY() throws Exception {
Paint p = new Paint();
final int n = mFaces.length;
@@ -89,4 +107,69 @@
}
}
+ @LargeTest
+ @Test
+ public void testMultithreadCacheStressTest() {
+ final Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
+ final Resources res = context.getResources();
+ final AssetManager assets = res.getAssets();
+ final Typeface[] baseTypefaces = {
+ null,
+ Typeface.SANS_SERIF,
+ Typeface.SERIF,
+ Typeface.MONOSPACE,
+ res.getFont(R.font.samplefont),
+ res.getFont(R.font.samplefont2),
+ res.getFont(R.font.samplefont3),
+ res.getFont(R.font.samplefont4),
+ res.getFont(R.font.samplexmlfont),
+ Typeface.createFromAsset(assets, "fonts/a3em.ttf"),
+ Typeface.createFromAsset(assets, "fonts/b3em.ttf"),
+ Typeface.createFromAsset(assets, "fonts/c3em.ttf"),
+ Typeface.createFromAsset(assets, "fonts/all2em.ttf"),
+ Typeface.createFromAsset(assets, "fonts/hasGlyphTestFont.ttf"),
+ Typeface.createFromAsset(assets, "fonts/samplefont1.ttf"),
+ Typeface.createFromAsset(assets, "fonts/no_coverage.ttf"),
+ };
+
+ final int loopCount = 10000;
+
+ final Runnable threadedCreater = () -> {
+ final Random random = new Random();
+ for (int i = 0; i < loopCount; ++i) {
+ final Typeface base = baseTypefaces[random.nextInt(baseTypefaces.length)];
+ if (random.nextBoolean()) {
+ final int style = random.nextInt(3);
+ final Typeface result = Typeface.create(base, style);
+ assertEquals(style, result.getStyle());
+ } else {
+ final int weight = 100 * (random.nextInt(10) + 1); // [100, 1000]
+ final boolean italic = random.nextBoolean();
+ final Typeface result = Typeface.create(base, weight, italic);
+ assertEquals(italic, result.isItalic());
+ assertEquals(weight, result.getWeight());
+ }
+ }
+ };
+
+ final int threadCount = 4;
+ final Thread[] threads = new Thread[threadCount];
+ for (int i = 0; i < threadCount; ++i) {
+ threads[i] = new Thread(threadedCreater);
+ }
+
+ for (int i = 0; i < threadCount; ++i) {
+ threads[i].start();
+ }
+
+ for (int i = 0; i < threadCount; ++i) {
+ try {
+ threads[i].join();
+ } catch (InterruptedException e) {
+ // ignore
+ }
+ }
+
+ }
+
}
diff --git a/graphics/java/android/graphics/Typeface.java b/graphics/java/android/graphics/Typeface.java
index 7496fa6..cfc389f 100644
--- a/graphics/java/android/graphics/Typeface.java
+++ b/graphics/java/android/graphics/Typeface.java
@@ -97,19 +97,33 @@
public static final Typeface MONOSPACE;
static Typeface[] sDefaults;
- private static final LongSparseArray<SparseArray<Typeface>> sTypefaceCache =
+
+ /**
+ * Cache for Typeface objects for style variant. Currently max size is 3.
+ */
+ @GuardedBy("sStyledCacheLock")
+ private static final LongSparseArray<SparseArray<Typeface>> sStyledTypefaceCache =
new LongSparseArray<>(3);
+ private static final Object sStyledCacheLock = new Object();
+
+ /**
+ * Cache for Typeface objects for weight variant. Currently max size is 3.
+ */
+ @GuardedBy("sWeightCacheLock")
+ private static final LongSparseArray<SparseArray<Typeface>> sWeightTypefaceCache =
+ new LongSparseArray<>(3);
+ private static final Object sWeightCacheLock = new Object();
/**
* Cache for Typeface objects dynamically loaded from assets. Currently max size is 16.
*/
- @GuardedBy("sLock")
+ @GuardedBy("sDynamicCacheLock")
private static final LruCache<String, Typeface> sDynamicTypefaceCache = new LruCache<>(16);
+ private static final Object sDynamicCacheLock = new Object();
static Typeface sDefaultTypeface;
static final Map<String, Typeface> sSystemFontMap;
static final Map<String, FontFamily[]> sSystemFallbackMap;
- private static final Object sLock = new Object();
/**
* @hide
@@ -121,6 +135,7 @@
public static final int BOLD = 1;
public static final int ITALIC = 2;
public static final int BOLD_ITALIC = 3;
+ /** @hide */ public static final int STYLE_MASK = 0x03;
private int mStyle = 0;
private int mWeight = 0;
@@ -143,6 +158,13 @@
nativeSetDefault(t.native_instance);
}
+ // TODO: Make this public API. (b/64852739)
+ /** @hide */
+ @VisibleForTesting
+ public int getWeight() {
+ return mWeight;
+ }
+
/** Returns the typeface's intrinsic style attributes */
public int getStyle() {
return mStyle;
@@ -164,7 +186,7 @@
*/
@Nullable
public static Typeface createFromResources(AssetManager mgr, String path, int cookie) {
- synchronized (sDynamicTypefaceCache) {
+ synchronized (sDynamicCacheLock) {
final String key = Builder.createAssetUid(
mgr, path, 0 /* ttcIndex */, null /* axes */,
RESOLVE_BY_FONT_TABLE /* weight */, RESOLVE_BY_FONT_TABLE /* italic */,
@@ -241,7 +263,7 @@
FontFamily[] familyChain = { fontFamily };
typeface = createFromFamiliesWithDefault(familyChain, DEFAULT_FAMILY,
RESOLVE_BY_FONT_TABLE, RESOLVE_BY_FONT_TABLE);
- synchronized (sDynamicTypefaceCache) {
+ synchronized (sDynamicCacheLock) {
final String key = Builder.createAssetUid(mgr, path, 0 /* ttcIndex */,
null /* axes */, RESOLVE_BY_FONT_TABLE /* weight */,
RESOLVE_BY_FONT_TABLE /* italic */, DEFAULT_FAMILY);
@@ -255,7 +277,7 @@
* @hide
*/
public static Typeface findFromCache(AssetManager mgr, String path) {
- synchronized (sDynamicTypefaceCache) {
+ synchronized (sDynamicCacheLock) {
final String key = Builder.createAssetUid(mgr, path, 0 /* ttcIndex */, null /* axes */,
RESOLVE_BY_FONT_TABLE /* weight */, RESOLVE_BY_FONT_TABLE /* italic */,
DEFAULT_FAMILY);
@@ -525,12 +547,6 @@
return builder.toString();
}
- private static final Object sLock = new Object();
- // TODO: Unify with Typeface.sTypefaceCache.
- @GuardedBy("sLock")
- private static final LongSparseArray<SparseArray<Typeface>> sTypefaceCache =
- new LongSparseArray<>(3);
-
private Typeface resolveFallbackTypeface() {
if (mFallbackFamilyName == null) {
return null;
@@ -581,7 +597,7 @@
final String key = createAssetUid(
mAssetManager, mPath, mTtcIndex, mAxes, mWeight, mItalic,
mFallbackFamilyName);
- synchronized (sLock) {
+ synchronized (sDynamicCacheLock) {
Typeface typeface = sDynamicTypefaceCache.get(key);
if (typeface != null) return typeface;
final FontFamily fontFamily = new FontFamily();
@@ -666,6 +682,11 @@
* style from the same family of an existing typeface object. If family is
* null, this selects from the default font's family.
*
+ * <p>
+ * This method is not thread safe on API 27 or before.
+ * This method is thread safe on API 28 or after.
+ * </p>
+ *
* @param family An existing {@link Typeface} object. In case of {@code null}, the default
* typeface is used instead.
* @param style The style (normal, bold, italic) of the typeface.
@@ -673,36 +694,37 @@
* @return The best matching typeface.
*/
public static Typeface create(Typeface family, int style) {
- if (style < 0 || style > 3) {
- style = 0;
+ if ((style & ~STYLE_MASK) != 0) {
+ style = NORMAL;
}
- long ni = 0;
- if (family != null) {
- // Return early if we're asked for the same face/style
- if (family.mStyle == style) {
- return family;
- }
+ if (family == null) {
+ family = sDefaultTypeface;
+ }
- ni = family.native_instance;
+ // Return early if we're asked for the same face/style
+ if (family.mStyle == style) {
+ return family;
}
+ final long ni = family.native_instance;
+
Typeface typeface;
- SparseArray<Typeface> styles = sTypefaceCache.get(ni);
+ synchronized (sStyledCacheLock) {
+ SparseArray<Typeface> styles = sStyledTypefaceCache.get(ni);
- if (styles != null) {
- typeface = styles.get(style);
- if (typeface != null) {
- return typeface;
+ if (styles == null) {
+ styles = new SparseArray<Typeface>(4);
+ sStyledTypefaceCache.put(ni, styles);
+ } else {
+ typeface = styles.get(style);
+ if (typeface != null) {
+ return typeface;
+ }
}
- }
- typeface = new Typeface(nativeCreateFromTypeface(ni, style));
- if (styles == null) {
- styles = new SparseArray<Typeface>(4);
- sTypefaceCache.put(ni, styles);
+ typeface = new Typeface(nativeCreateFromTypeface(ni, style));
+ styles.put(style, typeface);
}
- styles.put(style, typeface);
-
return typeface;
}
@@ -710,6 +732,10 @@
* Creates a typeface object that best matches the specified existing typeface and the specified
* weight and italic style
*
+ * <p>
+ * This method is thread safe.
+ * </p>
+ *
* @param family An existing {@link Typeface} object. In case of {@code null}, the default
* typeface is used instead.
* @param weight The desired weight to be drawn.
@@ -728,12 +754,15 @@
private static @NonNull Typeface createWeightStyle(@NonNull Typeface base,
@IntRange(from = 1, to = 1000) int weight, boolean italic) {
- final int key = weight << 1 | (italic ? 1 : 0);
+ final int key = (weight << 1) | (italic ? 1 : 0);
Typeface typeface;
- synchronized(sLock) {
- SparseArray<Typeface> innerCache = sTypefaceCache.get(base.native_instance);
- if (innerCache != null) {
+ synchronized(sWeightCacheLock) {
+ SparseArray<Typeface> innerCache = sWeightTypefaceCache.get(base.native_instance);
+ if (innerCache == null) {
+ innerCache = new SparseArray<>(4);
+ sWeightTypefaceCache.put(base.native_instance, innerCache);
+ } else {
typeface = innerCache.get(key);
if (typeface != null) {
return typeface;
@@ -743,11 +772,6 @@
typeface = new Typeface(
nativeCreateFromTypefaceWithExactStyle(
base.native_instance, weight, italic));
-
- if (innerCache == null) {
- innerCache = new SparseArray<>(4); // [regular, bold] x [upright, italic]
- sTypefaceCache.put(base.native_instance, innerCache);
- }
innerCache.put(key, typeface);
}
return typeface;
@@ -780,7 +804,7 @@
if (path == null) {
throw new NullPointerException(); // for backward compatibility
}
- synchronized (sLock) {
+ synchronized (sDynamicCacheLock) {
Typeface typeface = new Builder(mgr, path).build();
if (typeface != null) return typeface;