Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a88f5b74 authored by Mark Punzalan's avatar Mark Punzalan Committed by Android (Google) Code Review
Browse files

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

parents 0469e1ae e859f465
Loading
Loading
Loading
Loading
+26 −55
Original line number Diff line number Diff line
@@ -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) {
            new Translator(mContext, translationContext, tId, this, mHandler, mService,
                    translator -> {
                        if (translator == null) {
                                final long token = Binder.clearCallingIdentity();
                                try {
                                    executor.execute(() -> callback.accept(null));
                                } finally {
                                    Binder.restoreCallingIdentity(token);
                                }
                            Binder.withCleanCallingIdentity(
                                    () -> executor.execute(() -> callback.accept(null)));
                            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);
                            }
                            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);
            }
        }
    }