Merge "Don't cache created Translators in TranslationManager." into tm-dev

core/java/android/view/translation/TranslationManager.java

@@ -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<>();
+    private final IntArray mTranslatorIds = new IntArray();
-
-    @NonNull
-    @GuardedBy("mLock")
-    private final ArrayMap<TranslationContext, Integer> mTranslatorIds =
-            new ArrayMap<>();
 
     @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 Translator(mContext, translationContext, tId, this, mHandler, mService,
-                    new Consumer<Translator>() {
+                    translator -> {
-                        @Override
+                        if (translator == null) {
-                        public void accept(Translator translator) {
+                            Binder.withCleanCallingIdentity(
-                            if (translator == null) {
+                                    () -> executor.execute(() -> callback.accept(null)));
-                                final long token = Binder.clearCallingIdentity();
+                            return;
-                                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);
-                            }
                         }
+
+                        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.add(translatorId);
-                mTranslatorIds.put(translationContext, 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);
+            int index = mTranslatorIds.indexOf(id);
-            for (int i = 0; i < mTranslatorIds.size(); i++) {
+            if (index >= 0) {
-                if (mTranslatorIds.valueAt(i) == id) {
+                mTranslatorIds.remove(index);
-                    mTranslatorIds.removeAt(i);
-                    break;
-                }
             }
         }
     }