Don't cache created Translators in TranslationManager.

Bug: 208599087
Test: atest CtsTranslationTestCases
Change-Id: I0c2f606ef8acc7001beafdd02b48e37e9734182e
This commit is contained in:
Mark Punzalan
2022-02-23 00:48:38 +00:00
parent 12dd1d6747
commit e859f4655e

View File

@@ -33,9 +33,9 @@ import android.os.RemoteException;
import android.os.SynchronousResultReceiver;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.IntArray;
import android.util.Log;
import android.util.Pair;
import android.util.SparseArray;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.SyncResultReceiver;
@@ -102,12 +102,7 @@ public final class TranslationManager {
@NonNull
@GuardedBy("mLock")
private final SparseArray<Translator> mTranslators = new SparseArray<>();
@NonNull
@GuardedBy("mLock")
private final ArrayMap<TranslationContext, Integer> mTranslatorIds =
new ArrayMap<>();
private final IntArray mTranslatorIds = new IntArray();
@NonNull
private final Handler mHandler;
@@ -127,6 +122,12 @@ public final class TranslationManager {
/**
* Creates an on-device Translator for natural language translation.
*
* <p>In Android 12, this method provided the same cached Translator object when given the
* same TranslationContext object. Do not use a Translator destroyed elsewhere as this will
* cause an exception on Android 12.
*
* <p>In later versions, this method never returns a cached Translator.
*
* @param translationContext {@link TranslationContext} containing the specs for creating the
* Translator.
* @param executor Executor to run callback operations
@@ -140,45 +141,25 @@ public final class TranslationManager {
Objects.requireNonNull(callback, "callback cannot be null");
synchronized (mLock) {
// TODO(b/176464808): Disallow multiple Translator now, it will throw
// IllegalStateException. Need to discuss if we can allow multiple Translators.
if (mTranslatorIds.containsKey(translationContext)) {
executor.execute(() -> callback.accept(
mTranslators.get(mTranslatorIds.get(translationContext))));
return;
}
int translatorId;
do {
translatorId = Math.abs(ID_GENERATOR.nextInt());
} while (translatorId == 0 || mTranslators.indexOfKey(translatorId) >= 0);
} while (translatorId == 0 || mTranslatorIds.indexOf(translatorId) >= 0);
final int tId = translatorId;
new Translator(mContext, translationContext, translatorId, this, mHandler, mService,
new Consumer<Translator>() {
@Override
public void accept(Translator translator) {
if (translator == null) {
final long token = Binder.clearCallingIdentity();
try {
executor.execute(() -> callback.accept(null));
} finally {
Binder.restoreCallingIdentity(token);
}
return;
}
synchronized (mLock) {
mTranslators.put(tId, translator);
mTranslatorIds.put(translationContext, tId);
}
final long token = Binder.clearCallingIdentity();
try {
executor.execute(() -> callback.accept(translator));
} finally {
Binder.restoreCallingIdentity(token);
}
new Translator(mContext, translationContext, tId, this, mHandler, mService,
translator -> {
if (translator == null) {
Binder.withCleanCallingIdentity(
() -> executor.execute(() -> callback.accept(null)));
return;
}
synchronized (mLock) {
mTranslatorIds.add(tId);
}
Binder.withCleanCallingIdentity(
() -> executor.execute(() -> callback.accept(translator)));
});
}
}
@@ -201,16 +182,10 @@ public final class TranslationManager {
Objects.requireNonNull(translationContext, "translationContext cannot be null");
synchronized (mLock) {
// TODO(b/176464808): Disallow multiple Translator now, it will throw
// IllegalStateException. Need to discuss if we can allow multiple Translators.
if (mTranslatorIds.containsKey(translationContext)) {
return mTranslators.get(mTranslatorIds.get(translationContext));
}
int translatorId;
do {
translatorId = Math.abs(ID_GENERATOR.nextInt());
} while (translatorId == 0 || mTranslators.indexOfKey(translatorId) >= 0);
} while (translatorId == 0 || mTranslatorIds.indexOf(translatorId) >= 0);
final Translator newTranslator = new Translator(mContext, translationContext,
translatorId, this, mHandler, mService);
@@ -220,8 +195,7 @@ public final class TranslationManager {
if (!newTranslator.isSessionCreated()) {
return null;
}
mTranslators.put(translatorId, newTranslator);
mTranslatorIds.put(translationContext, translatorId);
mTranslatorIds.add(translatorId);
return newTranslator;
} catch (Translator.ServiceBinderReceiver.TimeoutException e) {
// TODO(b/176464808): maybe make SyncResultReceiver.TimeoutException constructor
@@ -460,12 +434,9 @@ public final class TranslationManager {
void removeTranslator(int id) {
synchronized (mLock) {
mTranslators.remove(id);
for (int i = 0; i < mTranslatorIds.size(); i++) {
if (mTranslatorIds.valueAt(i) == id) {
mTranslatorIds.removeAt(i);
break;
}
int index = mTranslatorIds.indexOf(id);
if (index >= 0) {
mTranslatorIds.remove(index);
}
}
}