From ff092aae1a6e78182f504382dafb8bd37a78353a Mon Sep 17 00:00:00 2001 From: Adam He Date: Wed, 3 Mar 2021 16:39:55 -0800 Subject: [PATCH] Implement TranslationCallback with support for partial responses. Translator#translate() is also updated to be asynchronous with callback instead. Bug: 178651352 Test: atest CtsTranslationTestCases CTS-Coverage-Bug: 178651352 Change-Id: Ibe1f97994fe6ee593a3a3bbb452c075b49a7eb5a --- core/api/current.txt | 4 +- .../translation/ITranslationCallback.aidl | 2 +- .../OnTranslationResultCallbackWrapper.java | 6 +- .../translation/TranslationService.java | 25 +----- .../ITranslationDirectManager.aidl | 3 +- .../view/translation/TranslationResponse.java | 65 +++++++++++++-- .../android/view/translation/Translator.java | 81 +++++++++++-------- .../translation/UiTranslationController.java | 2 +- 8 files changed, 118 insertions(+), 70 deletions(-) diff --git a/core/api/current.txt b/core/api/current.txt index 18c049fe21411..e137f7d5dc58f 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -52357,6 +52357,7 @@ package android.view.translation { method @NonNull public android.util.SparseArray getTranslationResponseValues(); method public int getTranslationStatus(); method @NonNull public android.util.SparseArray getViewTranslationResponses(); + method public boolean isFinalResponse(); method public void writeToParcel(@NonNull android.os.Parcel, int); field @NonNull public static final android.os.Parcelable.Creator CREATOR; field public static final int TRANSLATION_STATUS_CONTEXT_UNSUPPORTED = 2; // 0x2 @@ -52367,6 +52368,7 @@ package android.view.translation { public static final class TranslationResponse.Builder { ctor public TranslationResponse.Builder(int); method @NonNull public android.view.translation.TranslationResponse build(); + method @NonNull public android.view.translation.TranslationResponse.Builder setFinalResponse(boolean); method @NonNull public android.view.translation.TranslationResponse.Builder setTranslationResponseValue(int, @NonNull android.view.translation.TranslationResponseValue); method @NonNull public android.view.translation.TranslationResponse.Builder setTranslationResponseValues(@NonNull android.util.SparseArray); method @NonNull public android.view.translation.TranslationResponse.Builder setTranslationStatus(int); @@ -52408,7 +52410,7 @@ package android.view.translation { public class Translator { method public void destroy(); method public boolean isDestroyed(); - method @Nullable @WorkerThread public android.view.translation.TranslationResponse translate(@NonNull android.view.translation.TranslationRequest); + method @Nullable public void translate(@NonNull android.view.translation.TranslationRequest, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer); } public final class UiTranslationManager { diff --git a/core/java/android/service/translation/ITranslationCallback.aidl b/core/java/android/service/translation/ITranslationCallback.aidl index 333cb577f790b..893c9d0ff661f 100644 --- a/core/java/android/service/translation/ITranslationCallback.aidl +++ b/core/java/android/service/translation/ITranslationCallback.aidl @@ -24,6 +24,6 @@ import android.view.translation.TranslationResponse; * @hide */ oneway interface ITranslationCallback { - void onTranslationComplete(in TranslationResponse translationResponse); + void onTranslationResponse(in TranslationResponse translationResponse); void onError(); } diff --git a/core/java/android/service/translation/OnTranslationResultCallbackWrapper.java b/core/java/android/service/translation/OnTranslationResultCallbackWrapper.java index 345c69c0935d1..10a2aa2379272 100644 --- a/core/java/android/service/translation/OnTranslationResultCallbackWrapper.java +++ b/core/java/android/service/translation/OnTranslationResultCallbackWrapper.java @@ -51,12 +51,12 @@ final class OnTranslationResultCallbackWrapper implements @Override public void onTranslationSuccess(@Nullable TranslationResponse response) { assertNotCalled(); - if (mCalled.getAndSet(true)) { - throw new IllegalStateException("Already called"); + if (mCalled.getAndSet(response.isFinalResponse())) { + throw new IllegalStateException("Already called with complete response"); } try { - mCallback.onTranslationComplete(response); + mCallback.onTranslationResponse(response); } catch (RemoteException e) { if (e instanceof DeadObjectException) { Log.w(TAG, "Process is dead, ignore."); diff --git a/core/java/android/service/translation/TranslationService.java b/core/java/android/service/translation/TranslationService.java index d14a87f5e66f8..c5f1f04caaaba 100644 --- a/core/java/android/service/translation/TranslationService.java +++ b/core/java/android/service/translation/TranslationService.java @@ -42,7 +42,6 @@ import android.view.translation.TranslationResponse; import android.view.translation.TranslationSpec; import com.android.internal.os.IResultReceiver; -import com.android.internal.util.SyncResultReceiver; /** * Service for translating text. @@ -139,30 +138,10 @@ public abstract class TranslationService extends Service { @Override public void onTranslationRequest(TranslationRequest request, int sessionId, - ITranslationCallback callback, IResultReceiver receiver) + ITranslationCallback callback) throws RemoteException { - // TODO(b/176464808): Currently, the API is used for both sync and async case. - // It may work now, but maybe two methods is more cleaner. To think how to - // define the APIs for these two cases. - final ITranslationCallback cb = callback != null - ? callback - : new ITranslationCallback.Stub() { - @Override - public void onTranslationComplete( - TranslationResponse translationResponse) - throws RemoteException { - receiver.send(0, - SyncResultReceiver.bundleFor(translationResponse)); - } - - @Override - public void onError() throws RemoteException { - //TODO: implement default error callback - } - }; - // TODO(b/176464808): make it a private member of client final OnTranslationResultCallback translationResultCallback = - new OnTranslationResultCallbackWrapper(cb); + new OnTranslationResultCallbackWrapper(callback); mHandler.sendMessage(obtainMessage(TranslationService::onTranslationRequest, TranslationService.this, request, sessionId, mCancellationSignal, translationResultCallback)); diff --git a/core/java/android/view/translation/ITranslationDirectManager.aidl b/core/java/android/view/translation/ITranslationDirectManager.aidl index 46475b778d4f1..525a7790b8372 100644 --- a/core/java/android/view/translation/ITranslationDirectManager.aidl +++ b/core/java/android/view/translation/ITranslationDirectManager.aidl @@ -18,7 +18,6 @@ package android.view.translation; import android.view.translation.TranslationRequest; import android.service.translation.ITranslationCallback; -import com.android.internal.os.IResultReceiver; /** * Interface between an app (TranslationManager / Translator) and the remote TranslationService @@ -28,6 +27,6 @@ import com.android.internal.os.IResultReceiver; */ oneway interface ITranslationDirectManager { void onTranslationRequest(in TranslationRequest request, int sessionId, - in ITranslationCallback callback, in IResultReceiver receiver); + in ITranslationCallback callback); void onFinishTranslationSession(int sessionId); } diff --git a/core/java/android/view/translation/TranslationResponse.java b/core/java/android/view/translation/TranslationResponse.java index 03731e11f0ed6..35040f10361b5 100644 --- a/core/java/android/view/translation/TranslationResponse.java +++ b/core/java/android/view/translation/TranslationResponse.java @@ -67,6 +67,14 @@ public final class TranslationResponse implements Parcelable { @NonNull private final SparseArray mViewTranslationResponses; + /** + * Whether this response contains complete translated values, or is the final response in a + * series of partial responses. + * + *

This is {@code true} by default.

+ */ + private final boolean mFinalResponse; + abstract static class BaseBuilder { /** @@ -122,6 +130,10 @@ public final class TranslationResponse implements Parcelable { return new SparseArray<>(); } + private static boolean defaultFinalResponse() { + return true; + } + @@ -166,7 +178,8 @@ public final class TranslationResponse implements Parcelable { /* package-private */ TranslationResponse( @TranslationStatus int translationStatus, @NonNull SparseArray translationResponseValues, - @NonNull SparseArray viewTranslationResponses) { + @NonNull SparseArray viewTranslationResponses, + boolean finalResponse) { this.mTranslationStatus = translationStatus; if (!(mTranslationStatus == TRANSLATION_STATUS_SUCCESS) @@ -185,6 +198,7 @@ public final class TranslationResponse implements Parcelable { this.mViewTranslationResponses = viewTranslationResponses; com.android.internal.util.AnnotationValidations.validate( NonNull.class, null, mViewTranslationResponses); + this.mFinalResponse = finalResponse; // onConstructed(); // You can define this method to get a callback } @@ -215,6 +229,17 @@ public final class TranslationResponse implements Parcelable { return mViewTranslationResponses; } + /** + * Whether this response contains complete translated values, or is the final response in a + * series of partial responses. + * + *

This is {@code true} by default.

+ */ + @DataClass.Generated.Member + public boolean isFinalResponse() { + return mFinalResponse; + } + @Override @DataClass.Generated.Member public String toString() { @@ -224,7 +249,8 @@ public final class TranslationResponse implements Parcelable { return "TranslationResponse { " + "translationStatus = " + translationStatusToString(mTranslationStatus) + ", " + "translationResponseValues = " + mTranslationResponseValues + ", " + - "viewTranslationResponses = " + mViewTranslationResponses + + "viewTranslationResponses = " + mViewTranslationResponses + ", " + + "finalResponse = " + mFinalResponse + " }"; } @@ -234,6 +260,9 @@ public final class TranslationResponse implements Parcelable { // You can override field parcelling by defining methods like: // void parcelFieldName(Parcel dest, int flags) { ... } + byte flg = 0; + if (mFinalResponse) flg |= 0x8; + dest.writeByte(flg); dest.writeInt(mTranslationStatus); dest.writeSparseArray(mTranslationResponseValues); dest.writeSparseArray(mViewTranslationResponses); @@ -250,6 +279,8 @@ public final class TranslationResponse implements Parcelable { // You can override field unparcelling by defining methods like: // static FieldType unparcelFieldName(Parcel in) { ... } + byte flg = in.readByte(); + boolean finalResponse = (flg & 0x8) != 0; int translationStatus = in.readInt(); SparseArray translationResponseValues = (SparseArray) in.readSparseArray(TranslationResponseValue.class.getClassLoader()); SparseArray viewTranslationResponses = (SparseArray) in.readSparseArray(ViewTranslationResponse.class.getClassLoader()); @@ -272,6 +303,7 @@ public final class TranslationResponse implements Parcelable { this.mViewTranslationResponses = viewTranslationResponses; com.android.internal.util.AnnotationValidations.validate( NonNull.class, null, mViewTranslationResponses); + this.mFinalResponse = finalResponse; // onConstructed(); // You can define this method to get a callback } @@ -300,6 +332,7 @@ public final class TranslationResponse implements Parcelable { private @TranslationStatus int mTranslationStatus; private @NonNull SparseArray mTranslationResponseValues; private @NonNull SparseArray mViewTranslationResponses; + private boolean mFinalResponse; private long mBuilderFieldsSet = 0L; @@ -360,10 +393,24 @@ public final class TranslationResponse implements Parcelable { return this; } + /** + * Whether this response contains complete translated values, or is the final response in a + * series of partial responses. + * + *

This is {@code true} by default.

+ */ + @DataClass.Generated.Member + public @NonNull Builder setFinalResponse(boolean value) { + checkNotUsed(); + mBuilderFieldsSet |= 0x8; + mFinalResponse = value; + return this; + } + /** Builds the instance. This builder should not be touched after calling this! */ public @NonNull TranslationResponse build() { checkNotUsed(); - mBuilderFieldsSet |= 0x8; // Mark builder used + mBuilderFieldsSet |= 0x10; // Mark builder used if ((mBuilderFieldsSet & 0x2) == 0) { mTranslationResponseValues = defaultTranslationResponseValues(); @@ -371,15 +418,19 @@ public final class TranslationResponse implements Parcelable { if ((mBuilderFieldsSet & 0x4) == 0) { mViewTranslationResponses = defaultViewTranslationResponses(); } + if ((mBuilderFieldsSet & 0x8) == 0) { + mFinalResponse = defaultFinalResponse(); + } TranslationResponse o = new TranslationResponse( mTranslationStatus, mTranslationResponseValues, - mViewTranslationResponses); + mViewTranslationResponses, + mFinalResponse); return o; } private void checkNotUsed() { - if ((mBuilderFieldsSet & 0x8) != 0) { + if ((mBuilderFieldsSet & 0x10) != 0) { throw new IllegalStateException( "This Builder should not be reused. Use a new Builder instance instead"); } @@ -387,10 +438,10 @@ public final class TranslationResponse implements Parcelable { } @DataClass.Generated( - time = 1614211889478L, + time = 1616189850232L, codegenVersion = "1.0.22", sourceFile = "frameworks/base/core/java/android/view/translation/TranslationResponse.java", - inputSignatures = "public static final int TRANSLATION_STATUS_SUCCESS\npublic static final int TRANSLATION_STATUS_UNKNOWN_ERROR\npublic static final int TRANSLATION_STATUS_CONTEXT_UNSUPPORTED\nprivate final @android.view.translation.TranslationResponse.TranslationStatus int mTranslationStatus\nprivate final @android.annotation.NonNull android.util.SparseArray mTranslationResponseValues\nprivate final @android.annotation.NonNull android.util.SparseArray mViewTranslationResponses\nprivate static android.util.SparseArray defaultTranslationResponseValues()\nprivate static android.util.SparseArray defaultViewTranslationResponses()\nclass TranslationResponse extends java.lang.Object implements [android.os.Parcelable]\npublic @android.annotation.NonNull @java.lang.SuppressWarnings android.view.translation.TranslationResponse.Builder setTranslationResponseValue(int,android.view.translation.TranslationResponseValue)\npublic @android.annotation.NonNull @java.lang.SuppressWarnings android.view.translation.TranslationResponse.Builder setViewTranslationResponse(int,android.view.translation.ViewTranslationResponse)\nclass BaseBuilder extends java.lang.Object implements []\n@com.android.internal.util.DataClass(genBuilder=true, genToString=true, genHiddenConstDefs=true)\npublic @android.annotation.NonNull @java.lang.SuppressWarnings android.view.translation.TranslationResponse.Builder setTranslationResponseValue(int,android.view.translation.TranslationResponseValue)\npublic @android.annotation.NonNull @java.lang.SuppressWarnings android.view.translation.TranslationResponse.Builder setViewTranslationResponse(int,android.view.translation.ViewTranslationResponse)\nclass BaseBuilder extends java.lang.Object implements []") + inputSignatures = "public static final int TRANSLATION_STATUS_SUCCESS\npublic static final int TRANSLATION_STATUS_UNKNOWN_ERROR\npublic static final int TRANSLATION_STATUS_CONTEXT_UNSUPPORTED\nprivate final @android.view.translation.TranslationResponse.TranslationStatus int mTranslationStatus\nprivate final @android.annotation.NonNull android.util.SparseArray mTranslationResponseValues\nprivate final @android.annotation.NonNull android.util.SparseArray mViewTranslationResponses\nprivate final boolean mFinalResponse\nprivate static android.util.SparseArray defaultTranslationResponseValues()\nprivate static android.util.SparseArray defaultViewTranslationResponses()\nprivate static boolean defaultFinalResponse()\nclass TranslationResponse extends java.lang.Object implements [android.os.Parcelable]\npublic @android.annotation.NonNull @java.lang.SuppressWarnings android.view.translation.TranslationResponse.Builder setTranslationResponseValue(int,android.view.translation.TranslationResponseValue)\npublic @android.annotation.NonNull @java.lang.SuppressWarnings android.view.translation.TranslationResponse.Builder setViewTranslationResponse(int,android.view.translation.ViewTranslationResponse)\nclass BaseBuilder extends java.lang.Object implements []\n@com.android.internal.util.DataClass(genBuilder=true, genToString=true, genHiddenConstDefs=true)\npublic @android.annotation.NonNull @java.lang.SuppressWarnings android.view.translation.TranslationResponse.Builder setTranslationResponseValue(int,android.view.translation.TranslationResponseValue)\npublic @android.annotation.NonNull @java.lang.SuppressWarnings android.view.translation.TranslationResponse.Builder setViewTranslationResponse(int,android.view.translation.ViewTranslationResponse)\nclass BaseBuilder extends java.lang.Object implements []") @Deprecated private void __metadata() {} diff --git a/core/java/android/view/translation/Translator.java b/core/java/android/view/translation/Translator.java index 3e1e6dbe7d8ec..0cc26e128a551 100644 --- a/core/java/android/view/translation/Translator.java +++ b/core/java/android/view/translation/Translator.java @@ -19,11 +19,12 @@ package android.view.translation; import static android.view.translation.TranslationManager.STATUS_SYNC_CALL_FAIL; import static android.view.translation.TranslationManager.SYNC_CALLS_TIMEOUT_MS; +import android.annotation.CallbackExecutor; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.SuppressLint; -import android.annotation.WorkerThread; import android.content.Context; +import android.os.Binder; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; @@ -33,12 +34,12 @@ import android.util.Log; import com.android.internal.annotations.GuardedBy; import com.android.internal.os.IResultReceiver; -import com.android.internal.util.SyncResultReceiver; import java.io.PrintWriter; import java.lang.ref.WeakReference; import java.util.Objects; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; @@ -230,18 +231,21 @@ public class Translator { * Requests a translation for the provided {@link TranslationRequest} using the Translator's * source spec and destination spec. * - *

NOTE: Call on a worker thread. - * * @param request {@link TranslationRequest} request to be translate. * * @return {@link TranslationRequest} containing translated request, * or null if translation could not be done. * @throws IllegalStateException if this TextClassification session was destroyed when calls */ + //TODO: Add cancellation signal @Nullable - @WorkerThread - public TranslationResponse translate(@NonNull TranslationRequest request) { + public void translate(@NonNull TranslationRequest request, + @NonNull @CallbackExecutor Executor executor, + @NonNull Consumer callback) { Objects.requireNonNull(request, "Translation request cannot be null"); + Objects.requireNonNull(executor, "Executor cannot be null"); + Objects.requireNonNull(callback, "Callback cannot be null"); + if (isDestroyed()) { // TODO(b/176464808): Disallow multiple Translator now, it will throw // IllegalStateException. Need to discuss if we can allow multiple Translators. @@ -249,21 +253,13 @@ public class Translator { "This translator has been destroyed"); } - TranslationResponse response = null; + final ITranslationCallback responseCallback = + new TranslationResponseCallbackImpl(callback, executor); try { - final SyncResultReceiver receiver = new SyncResultReceiver(SYNC_CALLS_TIMEOUT_MS); - mDirectServiceBinder.onTranslationRequest(request, mId, null, receiver); - - response = receiver.getParcelableResult(); + mDirectServiceBinder.onTranslationRequest(request, mId, responseCallback); } catch (RemoteException e) { Log.w(TAG, "RemoteException calling requestTranslate(): " + e); - } catch (SyncResultReceiver.TimeoutException e) { - Log.e(TAG, "Timed out calling requestTranslate: " + e); } - if (sDEBUG) { - Log.v(TAG, "Receive translation response: " + response); - } - return response; } /** @@ -299,15 +295,16 @@ public class Translator { // TODO: add methods for UI-toolkit case. /** @hide */ public void requestUiTranslate(@NonNull TranslationRequest request, - @NonNull Consumer responseCallback) { + @NonNull Executor executor, + @NonNull Consumer callback) { if (mDirectServiceBinder == null) { Log.wtf(TAG, "Translator created without proper initialization."); return; } - final ITranslationCallback callback = - new TranslationResponseCallbackImpl(responseCallback); + final ITranslationCallback translationCallback = + new TranslationResponseCallbackImpl(callback, executor); try { - mDirectServiceBinder.onTranslationRequest(request, mId, callback, null); + mDirectServiceBinder.onTranslationRequest(request, mId, translationCallback); } catch (RemoteException e) { Log.w(TAG, "RemoteException calling flushRequest"); } @@ -315,26 +312,46 @@ public class Translator { private static class TranslationResponseCallbackImpl extends ITranslationCallback.Stub { - private final WeakReference> mResponseCallback; + private final WeakReference> mCallback; + private final WeakReference mExecutor; - TranslationResponseCallbackImpl(Consumer responseCallback) { - mResponseCallback = new WeakReference<>(responseCallback); + TranslationResponseCallbackImpl(Consumer callback, Executor executor) { + mCallback = new WeakReference<>(callback); + mExecutor = new WeakReference<>(executor); } @Override - public void onTranslationComplete(TranslationResponse response) throws RemoteException { - provideTranslationResponse(response); + public void onTranslationResponse(TranslationResponse response) throws RemoteException { + final Consumer callback = mCallback.get(); + final Runnable runnable = + () -> callback.accept(response); + if (callback != null) { + final Executor executor = mExecutor.get(); + final long token = Binder.clearCallingIdentity(); + try { + executor.execute(runnable); + } finally { + restoreCallingIdentity(token); + } + } } @Override public void onError() throws RemoteException { - provideTranslationResponse(null); - } + final Consumer callback = mCallback.get(); + final Runnable runnable = () -> callback.accept( + new TranslationResponse.Builder( + TranslationResponse.TRANSLATION_STATUS_UNKNOWN_ERROR) + .build()); - private void provideTranslationResponse(TranslationResponse response) { - final Consumer responseCallback = mResponseCallback.get(); - if (responseCallback != null) { - responseCallback.accept(response); + if (callback != null) { + final Executor executor = mExecutor.get(); + final long token = Binder.clearCallingIdentity(); + try { + executor.execute(runnable); + } finally { + restoreCallingIdentity(token); + } } } } diff --git a/core/java/android/view/translation/UiTranslationController.java b/core/java/android/view/translation/UiTranslationController.java index 9f90b3bf1322e..7f934c02228e8 100644 --- a/core/java/android/view/translation/UiTranslationController.java +++ b/core/java/android/view/translation/UiTranslationController.java @@ -287,7 +287,7 @@ public class UiTranslationController { final TranslationRequest request = new TranslationRequest.Builder() .setViewTranslationRequests(requests) .build(); - translator.requestUiTranslate(request, this::onTranslationCompleted); + translator.requestUiTranslate(request, (r) -> r.run(), this::onTranslationCompleted); } /**