From 844cc85ddfb5e1d829a0c09e5628ed05b11c9859 Mon Sep 17 00:00:00 2001
From: Joanne Chung <joannechung@google.com>
Date: Thu, 1 Jul 2021 18:55:50 +0800
Subject: [PATCH] Fix translation reference gone problem.

1. We save Translator WeakReference in ServiceBinderReceiver.
It has a chance the reference is gone even the session is still
alive. Save Translator instead of WeakReference.
2. Fix not access TranslationManager mTranslators, mTranslatorIds in
the lock.
3. Fix protential NPE of resultData access in ServiceBinderReceiver.

Bug: 192205945
Test: manual. The function works.
Test: atest CtsTranslationTestCases

Change-Id: I158901db4bbcd76203c705ab2a34f1e0b37c7565
---
 .../view/translation/TranslationManager.java    |  6 ++++--
 .../android/view/translation/Translator.java    | 17 ++++++-----------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/core/java/android/view/translation/TranslationManager.java b/core/java/android/view/translation/TranslationManager.java
index aec53206402b0..54c455c19621d 100644
--- a/core/java/android/view/translation/TranslationManager.java
+++ b/core/java/android/view/translation/TranslationManager.java
@@ -168,8 +168,10 @@ public final class TranslationManager {
                                 return;
                             }
 
-                            mTranslators.put(tId, translator);
-                            mTranslatorIds.put(translationContext, tId);
+                            synchronized (mLock) {
+                                mTranslators.put(tId, translator);
+                                mTranslatorIds.put(translationContext, tId);
+                            }
                             final long token = Binder.clearCallingIdentity();
                             try {
                                 executor.execute(() -> callback.accept(translator));
diff --git a/core/java/android/view/translation/Translator.java b/core/java/android/view/translation/Translator.java
index 8dbce1b4f53ea..edd0d16a5ccb4 100644
--- a/core/java/android/view/translation/Translator.java
+++ b/core/java/android/view/translation/Translator.java
@@ -102,19 +102,19 @@ public class Translator {
 
     static class ServiceBinderReceiver extends IResultReceiver.Stub {
         // TODO: refactor how translator is instantiated after removing deprecated createTranslator.
-        private final WeakReference<Translator> mTranslator;
+        private final Translator mTranslator;
         private final CountDownLatch mLatch = new CountDownLatch(1);
         private int mSessionId;
 
         private Consumer<Translator> mCallback;
 
         ServiceBinderReceiver(Translator translator, Consumer<Translator> callback) {
-            mTranslator = new WeakReference<>(translator);
+            mTranslator = translator;
             mCallback = callback;
         }
 
         ServiceBinderReceiver(Translator translator) {
-            mTranslator = new WeakReference<>(translator);
+            mTranslator = translator;
         }
 
         int getSessionStateResult() throws TimeoutException {
@@ -139,14 +139,9 @@ public class Translator {
                 }
                 return;
             }
-            mSessionId = resultData.getInt(EXTRA_SESSION_ID);
-            final Translator translator = mTranslator.get();
-            if (translator == null) {
-                Log.w(TAG, "received result after session is finished");
-                return;
-            }
             final IBinder binder;
             if (resultData != null) {
+                mSessionId = resultData.getInt(EXTRA_SESSION_ID);
                 binder = resultData.getBinder(EXTRA_SERVICE_BINDER);
                 if (binder == null) {
                     Log.wtf(TAG, "No " + EXTRA_SERVICE_BINDER + " extra result");
@@ -155,10 +150,10 @@ public class Translator {
             } else {
                 binder = null;
             }
-            translator.setServiceBinder(binder);
+            mTranslator.setServiceBinder(binder);
             mLatch.countDown();
             if (mCallback != null) {
-                mCallback.accept(translator);
+                mCallback.accept(mTranslator);
             }
         }