From cdc81e8affe2f2e71aba54cbb9d13820f0c31858 Mon Sep 17 00:00:00 2001
From: Tony Mak <tonymak@google.com>
Date: Thu, 22 Aug 2019 15:14:28 +0100
Subject: [PATCH] Hold a strong reference to the callback in
 TextClassifierService

Issue:
TextClassifierService failed to invoke the callback to send the result back
to client occasionally because  the callback object may be GCed.
And thus smart selection failed occasionally, as the client doesn't get
a response back when it hits this issue. It won't fallback to local
textclassifier due to the timeout specified in TextView.

Cause:
We thought that ITextClassifierCallback is a "cross process" reference, and
so we only store a weak reference of it to avoid leak.
And it turns out that it is wrong. As soon as the weak ref gets GCed in
the service, that counts as dropping the callback. The service doesn't
know about any strong references the client has.

Bug: 138865849

Test: Try smart selection over 30 times, make sure smart action is shown
for every single time.

Change-Id: Ia9218cf67e8d67697a0fdff22c7918a55efc39ca
---
 .../textclassifier/TextClassifierService.java  | 18 ++++--------------
 .../widget/SelectionActionModeHelper.java      |  1 +
 2 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/core/java/android/service/textclassifier/TextClassifierService.java b/core/java/android/service/textclassifier/TextClassifierService.java
index 9108c3365f770..5143f1820c2dd 100644
--- a/core/java/android/service/textclassifier/TextClassifierService.java
+++ b/core/java/android/service/textclassifier/TextClassifierService.java
@@ -51,7 +51,6 @@ import android.view.textclassifier.TextSelection;
 
 import com.android.internal.util.Preconditions;
 
-import java.lang.ref.WeakReference;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
@@ -431,23 +430,18 @@ public abstract class TextClassifierService extends Service {
      * Forwards the callback result to a wrapped binder callback.
      */
     private static final class ProxyCallback<T extends Parcelable> implements Callback<T> {
-        private WeakReference<ITextClassifierCallback> mTextClassifierCallback;
+        private ITextClassifierCallback mTextClassifierCallback;
 
         private ProxyCallback(ITextClassifierCallback textClassifierCallback) {
-            mTextClassifierCallback =
-                    new WeakReference<>(Preconditions.checkNotNull(textClassifierCallback));
+            mTextClassifierCallback = Preconditions.checkNotNull(textClassifierCallback);
         }
 
         @Override
         public void onSuccess(T result) {
-            ITextClassifierCallback callback = mTextClassifierCallback.get();
-            if (callback == null) {
-                return;
-            }
             try {
                 Bundle bundle = new Bundle(1);
                 bundle.putParcelable(KEY_RESULT, result);
-                callback.onSuccess(bundle);
+                mTextClassifierCallback.onSuccess(bundle);
             } catch (RemoteException e) {
                 Slog.d(LOG_TAG, "Error calling callback");
             }
@@ -455,13 +449,9 @@ public abstract class TextClassifierService extends Service {
 
         @Override
         public void onFailure(CharSequence error) {
-            ITextClassifierCallback callback = mTextClassifierCallback.get();
-            if (callback == null) {
-                return;
-            }
             try {
                 Slog.w(LOG_TAG, "Request fail: " + error);
-                callback.onFailure();
+                mTextClassifierCallback.onFailure();
             } catch (RemoteException e) {
                 Slog.d(LOG_TAG, "Error calling callback");
             }
diff --git a/core/java/android/widget/SelectionActionModeHelper.java b/core/java/android/widget/SelectionActionModeHelper.java
index 51ca805240905..d0f80936f477f 100644
--- a/core/java/android/widget/SelectionActionModeHelper.java
+++ b/core/java/android/widget/SelectionActionModeHelper.java
@@ -999,6 +999,7 @@ public final class SelectionActionModeHelper {
         }
 
         private void onTimeOut() {
+            Log.d(LOG_TAG, "Timeout in TextClassificationAsyncTask");
             if (getStatus() == Status.RUNNING) {
                 onPostExecute(mTimeOutResultSupplier.get());
             }