diff --git a/core/java/android/view/textclassifier/SystemTextClassifier.java b/core/java/android/view/textclassifier/SystemTextClassifier.java index 8eac1c1520c08..2c844eb3eb2ec 100644 --- a/core/java/android/view/textclassifier/SystemTextClassifier.java +++ b/core/java/android/view/textclassifier/SystemTextClassifier.java @@ -89,7 +89,7 @@ public final class SystemTextClassifier implements TextClassifier { try { request.setSystemTextClassifierMetadata(mSystemTcMetadata); final BlockingCallback callback = - new BlockingCallback<>("textselection"); + new BlockingCallback<>("textselection", mSettings); mManagerService.onSuggestSelection(mSessionId, request, callback); final TextSelection selection = callback.get(); if (selection != null) { @@ -112,7 +112,7 @@ public final class SystemTextClassifier implements TextClassifier { try { request.setSystemTextClassifierMetadata(mSystemTcMetadata); final BlockingCallback callback = - new BlockingCallback<>("textclassification"); + new BlockingCallback<>("textclassification", mSettings); mManagerService.onClassifyText(mSessionId, request, callback); final TextClassification classification = callback.get(); if (classification != null) { @@ -142,7 +142,7 @@ public final class SystemTextClassifier implements TextClassifier { try { request.setSystemTextClassifierMetadata(mSystemTcMetadata); final BlockingCallback callback = - new BlockingCallback<>("textlinks"); + new BlockingCallback<>("textlinks", mSettings); mManagerService.onGenerateLinks(mSessionId, request, callback); final TextLinks links = callback.get(); if (links != null) { @@ -193,7 +193,7 @@ public final class SystemTextClassifier implements TextClassifier { try { request.setSystemTextClassifierMetadata(mSystemTcMetadata); final BlockingCallback callback = - new BlockingCallback<>("textlanguage"); + new BlockingCallback<>("textlanguage", mSettings); mManagerService.onDetectLanguage(mSessionId, request, callback); final TextLanguage textLanguage = callback.get(); if (textLanguage != null) { @@ -213,7 +213,7 @@ public final class SystemTextClassifier implements TextClassifier { try { request.setSystemTextClassifierMetadata(mSystemTcMetadata); final BlockingCallback callback = - new BlockingCallback<>("conversation-actions"); + new BlockingCallback<>("conversation-actions", mSettings); mManagerService.onSuggestConversationActions(mSessionId, request, callback); final ConversationActions conversationActions = callback.get(); if (conversationActions != null) { @@ -279,8 +279,8 @@ public final class SystemTextClassifier implements TextClassifier { extends ITextClassifierCallback.Stub { private final ResponseReceiver mReceiver; - BlockingCallback(String name) { - mReceiver = new ResponseReceiver<>(name); + BlockingCallback(String name, TextClassificationConstants settings) { + mReceiver = new ResponseReceiver<>(name, settings); } @Override @@ -303,10 +303,12 @@ public final class SystemTextClassifier implements TextClassifier { private final CountDownLatch mLatch = new CountDownLatch(1); private final String mName; + private final TextClassificationConstants mSettings; private T mResponse; - private ResponseReceiver(String name) { + private ResponseReceiver(String name, TextClassificationConstants settings) { mName = name; + mSettings = settings; } public void onSuccess(T response) { @@ -327,7 +329,9 @@ public final class SystemTextClassifier implements TextClassifier { // NOTE that TextClassifier calls should preferably always be called on a worker thread. if (Looper.myLooper() != Looper.getMainLooper()) { try { - boolean success = mLatch.await(2, TimeUnit.SECONDS); + boolean success = mLatch.await( + mSettings.getSystemTextClassifierApiTimeoutInSecond(), + TimeUnit.SECONDS); if (!success) { Log.w(LOG_TAG, "Timeout in ResponseReceiver.get(): " + mName); } diff --git a/core/java/android/view/textclassifier/TextClassificationConstants.java b/core/java/android/view/textclassifier/TextClassificationConstants.java index adb6fea91799e..2975afc084e6f 100644 --- a/core/java/android/view/textclassifier/TextClassificationConstants.java +++ b/core/java/android/view/textclassifier/TextClassificationConstants.java @@ -82,6 +82,14 @@ public final class TextClassificationConstants { static final String TEXT_CLASSIFIER_SERVICE_PACKAGE_OVERRIDE = "textclassifier_service_package_override"; + /** + * The timeout value in seconds used by {@link SystemTextClassifier} for each TextClassifier + * API calls. + */ + @VisibleForTesting + static final String SYSTEM_TEXT_CLASSIFIER_API_TIMEOUT_IN_SECOND = + "system_textclassifier_api_timeout_in_second"; + private static final String DEFAULT_TEXT_CLASSIFIER_SERVICE_PACKAGE_OVERRIDE = null; private static final boolean LOCAL_TEXT_CLASSIFIER_ENABLED_DEFAULT = true; private static final boolean SYSTEM_TEXT_CLASSIFIER_ENABLED_DEFAULT = true; @@ -91,6 +99,7 @@ public final class TextClassificationConstants { private static final boolean SMART_LINKIFY_ENABLED_DEFAULT = true; private static final boolean SMART_SELECT_ANIMATION_ENABLED_DEFAULT = true; private static final int GENERATE_LINKS_MAX_TEXT_LENGTH_DEFAULT = 100 * 1000; + private static final long SYSTEM_TEXT_CLASSIFIER_API_TIMEOUT_IN_SECOND_DEFAULT = 60; @Nullable public String getTextClassifierServicePackageOverride() { @@ -140,27 +149,27 @@ public final class TextClassificationConstants { GENERATE_LINKS_MAX_TEXT_LENGTH, GENERATE_LINKS_MAX_TEXT_LENGTH_DEFAULT); } + public long getSystemTextClassifierApiTimeoutInSecond() { + return DeviceConfig.getLong(DeviceConfig.NAMESPACE_TEXTCLASSIFIER, + SYSTEM_TEXT_CLASSIFIER_API_TIMEOUT_IN_SECOND, + SYSTEM_TEXT_CLASSIFIER_API_TIMEOUT_IN_SECOND_DEFAULT); + } + void dump(IndentingPrintWriter pw) { pw.println("TextClassificationConstants:"); pw.increaseIndent(); - pw.printPair("generate_links_max_text_length", getGenerateLinksMaxTextLength()) - .println(); - pw.printPair("local_textclassifier_enabled", isLocalTextClassifierEnabled()) - .println(); - pw.printPair("model_dark_launch_enabled", isModelDarkLaunchEnabled()) - .println(); - pw.printPair("smart_linkify_enabled", isSmartLinkifyEnabled()) - .println(); - pw.printPair("smart_select_animation_enabled", isSmartSelectionAnimationEnabled()) - .println(); - pw.printPair("smart_selection_enabled", isSmartSelectionEnabled()) - .println(); - pw.printPair("smart_text_share_enabled", isSmartTextShareEnabled()) - .println(); - pw.printPair("system_textclassifier_enabled", isSystemTextClassifierEnabled()) - .println(); - pw.printPair("textclassifier_service_package_override", + pw.print(GENERATE_LINKS_MAX_TEXT_LENGTH, getGenerateLinksMaxTextLength()).println(); + pw.print(LOCAL_TEXT_CLASSIFIER_ENABLED, isLocalTextClassifierEnabled()).println(); + pw.print(MODEL_DARK_LAUNCH_ENABLED, isModelDarkLaunchEnabled()).println(); + pw.print(SMART_LINKIFY_ENABLED, isSmartLinkifyEnabled()).println(); + pw.print(SMART_SELECT_ANIMATION_ENABLED, isSmartSelectionAnimationEnabled()).println(); + pw.print(SMART_SELECTION_ENABLED, isSmartSelectionEnabled()).println(); + pw.print(SMART_TEXT_SHARE_ENABLED, isSmartTextShareEnabled()).println(); + pw.print(SYSTEM_TEXT_CLASSIFIER_ENABLED, isSystemTextClassifierEnabled()).println(); + pw.print(TEXT_CLASSIFIER_SERVICE_PACKAGE_OVERRIDE, getTextClassifierServicePackageOverride()).println(); + pw.print(SYSTEM_TEXT_CLASSIFIER_API_TIMEOUT_IN_SECOND, + getSystemTextClassifierApiTimeoutInSecond()).println(); pw.decreaseIndent(); } } \ No newline at end of file diff --git a/core/tests/coretests/src/android/view/textclassifier/TextClassificationConstantsTest.java b/core/tests/coretests/src/android/view/textclassifier/TextClassificationConstantsTest.java index 2f21b7f0c75a5..16e750a3c12d8 100644 --- a/core/tests/coretests/src/android/view/textclassifier/TextClassificationConstantsTest.java +++ b/core/tests/coretests/src/android/view/textclassifier/TextClassificationConstantsTest.java @@ -16,7 +16,7 @@ package android.view.textclassifier; -import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.common.truth.Truth.assertThat; import android.provider.DeviceConfig; @@ -26,73 +26,63 @@ import androidx.test.runner.AndroidJUnit4; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.function.Consumer; + @SmallTest @RunWith(AndroidJUnit4.class) public class TextClassificationConstantsTest { - private static final float EPSILON = 0.0001f; @Test - public void testLoadFromDeviceConfig_booleanValue() throws Exception { - // Saves config original value. - final String originalValue = DeviceConfig.getProperty(DeviceConfig.NAMESPACE_TEXTCLASSIFIER, - TextClassificationConstants.LOCAL_TEXT_CLASSIFIER_ENABLED); - - final TextClassificationConstants constants = new TextClassificationConstants(); - try { - // Sets and checks different value. - setDeviceConfig(TextClassificationConstants.LOCAL_TEXT_CLASSIFIER_ENABLED, "false"); - assertWithMessage(TextClassificationConstants.LOCAL_TEXT_CLASSIFIER_ENABLED) - .that(constants.isLocalTextClassifierEnabled()).isFalse(); - } finally { - // Restores config original value. - setDeviceConfig(TextClassificationConstants.LOCAL_TEXT_CLASSIFIER_ENABLED, - originalValue); - } + public void booleanSettings() { + assertSettings( + TextClassificationConstants.LOCAL_TEXT_CLASSIFIER_ENABLED, + "false", + settings -> assertThat(settings.isLocalTextClassifierEnabled()).isFalse()); } @Test - public void testLoadFromDeviceConfig_IntValue() throws Exception { - // Saves config original value. - final String originalValue = DeviceConfig.getProperty(DeviceConfig.NAMESPACE_TEXTCLASSIFIER, - TextClassificationConstants.GENERATE_LINKS_MAX_TEXT_LENGTH); - - final TextClassificationConstants constants = new TextClassificationConstants(); - try { - // Sets and checks different value. - setDeviceConfig(TextClassificationConstants.GENERATE_LINKS_MAX_TEXT_LENGTH, "8"); - assertWithMessage(TextClassificationConstants.GENERATE_LINKS_MAX_TEXT_LENGTH) - .that(constants.getGenerateLinksMaxTextLength()).isEqualTo(8); - } finally { - // Restores config original value. - setDeviceConfig(TextClassificationConstants.GENERATE_LINKS_MAX_TEXT_LENGTH, - originalValue); - } + public void intSettings() { + assertSettings( + TextClassificationConstants.GENERATE_LINKS_MAX_TEXT_LENGTH, + "128", + settings -> assertThat(settings.getGenerateLinksMaxTextLength()).isEqualTo(128)); } @Test - public void testLoadFromDeviceConfig_StringValue() throws Exception { - // Saves config original value. - final String originalValue = DeviceConfig.getProperty(DeviceConfig.NAMESPACE_TEXTCLASSIFIER, - TextClassificationConstants.TEXT_CLASSIFIER_SERVICE_PACKAGE_OVERRIDE); + public void stringSettings() { + assertSettings( + TextClassificationConstants.TEXT_CLASSIFIER_SERVICE_PACKAGE_OVERRIDE, + "com.example.textclassifier", + settings -> assertThat( + settings.getTextClassifierServicePackageOverride()) + .isEqualTo("com.example.textclassifier")); + } - final TextClassificationConstants constants = new TextClassificationConstants(); + @Test + public void longSettings() { + assertSettings( + TextClassificationConstants.SYSTEM_TEXT_CLASSIFIER_API_TIMEOUT_IN_SECOND, + "1", + settings -> assertThat( + settings.getSystemTextClassifierApiTimeoutInSecond()) + .isEqualTo(1)); + } + + private static void assertSettings( + String key, String value, Consumer settingsConsumer) { + final String originalValue = + DeviceConfig.getProperty(DeviceConfig.NAMESPACE_TEXTCLASSIFIER, key); + TextClassificationConstants settings = new TextClassificationConstants(); try { - // Sets and checks different value. - final String testTextClassifier = "com.example.textclassifier"; - setDeviceConfig(TextClassificationConstants.TEXT_CLASSIFIER_SERVICE_PACKAGE_OVERRIDE, - testTextClassifier); - assertWithMessage(TextClassificationConstants.TEXT_CLASSIFIER_SERVICE_PACKAGE_OVERRIDE) - .that(constants.getTextClassifierServicePackageOverride()).isEqualTo( - testTextClassifier); + setDeviceConfig(key, value); + settingsConsumer.accept(settings); } finally { - // Restores config original value. - setDeviceConfig(TextClassificationConstants.TEXT_CLASSIFIER_SERVICE_PACKAGE_OVERRIDE, - originalValue); + setDeviceConfig(key, originalValue); } } - private void setDeviceConfig(String key, String value) { - DeviceConfig.setProperty(DeviceConfig.NAMESPACE_TEXTCLASSIFIER, key, - value, /* makeDefault */ false); + private static void setDeviceConfig(String key, String value) { + DeviceConfig.setProperty( + DeviceConfig.NAMESPACE_TEXTCLASSIFIER, key, value, /* makeDefault */ false); } } diff --git a/services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java b/services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java index 363e86dea8b1e..93ba758d29d62 100644 --- a/services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java +++ b/services/core/java/com/android/server/textclassifier/TextClassificationManagerService.java @@ -66,7 +66,6 @@ import com.android.internal.util.FunctionalUtils.ThrowingRunnable; import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.Preconditions; import com.android.server.SystemService; -import com.android.server.SystemService.TargetUser; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -458,6 +457,9 @@ public final class TextClassificationManagerService extends ITextClassifierServi callback.onFailure(); } else if (serviceState.isBoundLocked()) { if (!serviceState.checkRequestAcceptedLocked(Binder.getCallingUid(), methodName)) { + Slog.w(LOG_TAG, String.format("UID %d is not allowed to see the %s request", + Binder.getCallingUid(), methodName)); + callback.onFailure(); return; } textClassifierServiceConsumer.accept(serviceState.mService); @@ -497,7 +499,7 @@ public final class TextClassificationManagerService extends ITextClassifierServi private final IBinder mBinder; @NonNull private final Runnable mRequest; - @Nullable + @NonNull private final Runnable mOnServiceFailure; @GuardedBy("mLock") @NonNull @@ -516,7 +518,7 @@ public final class TextClassificationManagerService extends ITextClassifierServi * @param uid the calling uid of the request. */ PendingRequest(@Nullable String name, - @NonNull ThrowingRunnable request, @Nullable ThrowingRunnable onServiceFailure, + @NonNull ThrowingRunnable request, @NonNull ThrowingRunnable onServiceFailure, @Nullable IBinder binder, @NonNull TextClassificationManagerService service, @NonNull ServiceState serviceState, int uid) { @@ -524,7 +526,8 @@ public final class TextClassificationManagerService extends ITextClassifierServi mRequest = logOnFailure(Objects.requireNonNull(request), "handling pending request"); mOnServiceFailure = - logOnFailure(onServiceFailure, "notifying callback of service failure"); + logOnFailure(Objects.requireNonNull(onServiceFailure), + "notifying callback of service failure"); mBinder = binder; mService = service; mServiceState = Objects.requireNonNull(serviceState); @@ -799,9 +802,7 @@ public final class TextClassificationManagerService extends ITextClassifierServi request -> { Slog.w(LOG_TAG, String.format("Pending request[%s] is dropped", request.mName)); - if (request.mOnServiceFailure != null) { - request.mOnServiceFailure.run(); - } + request.mOnServiceFailure.run(); }); @Nullable @GuardedBy("mLock") @@ -843,15 +844,16 @@ public final class TextClassificationManagerService extends ITextClassifierServi while ((request = mPendingRequests.poll()) != null) { if (isBoundLocked()) { if (!checkRequestAcceptedLocked(request.mUid, request.mName)) { - return; - } - request.mRequest.run(); - } else { - if (request.mOnServiceFailure != null) { - Slog.d(LOG_TAG, "Unable to bind TextClassifierService for PendingRequest " - + request.mName); + Slog.w(LOG_TAG, String.format("UID %d is not allowed to see the %s request", + request.mUid, request.mName)); request.mOnServiceFailure.run(); + } else { + request.mRequest.run(); } + } else { + Slog.d(LOG_TAG, "Unable to bind TextClassifierService for PendingRequest " + + request.mName); + request.mOnServiceFailure.run(); } if (request.mBinder != null) {