From b343cb4433d75df42305065c0e60ad949405d924 Mon Sep 17 00:00:00 2001 From: Adam He Date: Tue, 6 Oct 2020 15:56:15 -0700 Subject: [PATCH] Misc small changes in Session.java. * Move helper methods to static methods in Helper.java * Add documentation on ambiguous methods. * Rename method to end with *Locked for consistency. Bug: 162357598 Test: atest CtsAutoFillServiceTestCases Change-Id: I1b2f788a74fa16f566520f5e83c2df21ffbca2dc --- .../AutofillInlineSessionController.java | 5 +- .../autofill/AutofillManagerServiceImpl.java | 4 +- .../com/android/server/autofill/Helper.java | 44 +++++++++ .../com/android/server/autofill/Session.java | 92 ++++++------------- 4 files changed, 77 insertions(+), 68 deletions(-) diff --git a/services/autofill/java/com/android/server/autofill/AutofillInlineSessionController.java b/services/autofill/java/com/android/server/autofill/AutofillInlineSessionController.java index c25dd37bc7d9a..dd5e58a94d154 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillInlineSessionController.java +++ b/services/autofill/java/com/android/server/autofill/AutofillInlineSessionController.java @@ -35,8 +35,8 @@ import java.util.function.Consumer; * Controls the interaction with the IME for the {@link AutofillInlineSuggestionsRequestSession}s. * *

The class maintains the inline suggestion session with the autofill service. There is at most - * one active inline suggestion session at any given corresponding to one focused view. - * New sessions are created only when {@link #onCreateInlineSuggestionsRequestLocked} is called.

+ * one active inline suggestion session at any given corresponding to one focused view. New + * sessions are created only when {@link #onCreateInlineSuggestionsRequestLocked} is called.

* *

The class manages the interaction between the {@link com.android.server.autofill.Session} and * the inline suggestion session whenever inline suggestions can be provided. All calls to the @@ -83,7 +83,6 @@ final class AutofillInlineSessionController { @GuardedBy("mLock") void onCreateInlineSuggestionsRequestLocked(@NonNull AutofillId autofillId, @NonNull Consumer requestConsumer, @NonNull Bundle uiExtras) { - // TODO(b/151123764): rename the method to better reflect what it does. if (mSession != null) { // Destroy the existing session. mSession.destroySessionLocked(); diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java index d59c955de8898..67e34b1d411bf 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java @@ -1044,7 +1044,7 @@ final class AutofillManagerServiceImpl pw.println(compatPkgs); } pw.print(prefix); pw.print("Inline Suggestions Enabled: "); - pw.println(isInlineSuggestionsEnabled()); + pw.println(isInlineSuggestionsEnabledLocked()); pw.print(prefix); pw.print("Last prune: "); pw.println(mLastPrune); mDisabledInfoCache.dump(mUserId, prefix, pw); @@ -1157,7 +1157,7 @@ final class AutofillManagerServiceImpl } @GuardedBy("mLock") - boolean isInlineSuggestionsEnabled() { + boolean isInlineSuggestionsEnabledLocked() { if (mInfo != null) { return mInfo.isInlineSuggestionsEnabled(); } diff --git a/services/autofill/java/com/android/server/autofill/Helper.java b/services/autofill/java/com/android/server/autofill/Helper.java index 80b0375a229dd..e35c0ee4e59b2 100644 --- a/services/autofill/java/com/android/server/autofill/Helper.java +++ b/services/autofill/java/com/android/server/autofill/Helper.java @@ -24,6 +24,9 @@ import android.app.assist.AssistStructure.WindowNode; import android.content.ComponentName; import android.metrics.LogMaker; import android.service.autofill.Dataset; +import android.service.autofill.InternalSanitizer; +import android.service.autofill.SaveInfo; +import android.text.TextUtils; import android.util.ArrayMap; import android.util.ArraySet; import android.util.Slog; @@ -38,6 +41,7 @@ import com.android.internal.util.ArrayUtils; import java.io.PrintWriter; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Arrays; public final class Helper { @@ -234,6 +238,46 @@ public final class Helper { } } + @Nullable + static ArrayMap createSanitizers(@Nullable SaveInfo saveInfo) { + if (saveInfo == null) return null; + + final InternalSanitizer[] sanitizerKeys = saveInfo.getSanitizerKeys(); + if (sanitizerKeys == null) return null; + + final int size = sanitizerKeys.length; + final ArrayMap sanitizers = new ArrayMap<>(size); + if (sDebug) Slog.d(TAG, "Service provided " + size + " sanitizers"); + final AutofillId[][] sanitizerValues = saveInfo.getSanitizerValues(); + for (int i = 0; i < size; i++) { + final InternalSanitizer sanitizer = sanitizerKeys[i]; + final AutofillId[] ids = sanitizerValues[i]; + if (sDebug) { + Slog.d(TAG, "sanitizer #" + i + " (" + sanitizer + ") for ids " + + Arrays.toString(ids)); + } + for (AutofillId id : ids) { + sanitizers.put(id, sanitizer); + } + } + return sanitizers; + } + + /** + * Returns true if {@code s1} contains all characters of {@code s2}, in order. + */ + static boolean containsCharsInOrder(String s1, String s2) { + int prevIndex = -1; + for (char ch : s2.toCharArray()) { + int index = TextUtils.indexOf(s1, ch, prevIndex + 1); + if (index == -1) { + return false; + } + prevIndex = index; + } + return true; + } + private interface ViewNodeFilter { boolean matches(ViewNode node); } diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index 4d2e4f6ee486c..36755061bf189 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -30,6 +30,8 @@ import static android.view.autofill.AutofillManager.FLAG_SMART_SUGGESTION_SYSTEM import static android.view.autofill.AutofillManager.getSmartSuggestionModeToString; import static com.android.internal.util.function.pooled.PooledLambda.obtainMessage; +import static com.android.server.autofill.Helper.containsCharsInOrder; +import static com.android.server.autofill.Helper.createSanitizers; import static com.android.server.autofill.Helper.getNumericValue; import static com.android.server.autofill.Helper.sDebug; import static com.android.server.autofill.Helper.sVerbose; @@ -543,6 +545,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState return ids; } + /** + * Returns the String value of an {@link AutofillValue} by {@link AutofillId id} if it is of + * type {@code AUTOFILL_TYPE_TEXT} or {@code AUTOFILL_TYPE_LIST}. + */ @Override @Nullable public String findByAutofillId(@NonNull AutofillId id) { @@ -704,7 +710,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState * Autofill provider). */ private boolean isInlineSuggestionsEnabledByAutofillProviderLocked() { - return mService.isInlineSuggestionsEnabled(); + return mService.isInlineSuggestionsEnabledLocked(); } private boolean isViewFocusedLocked(int flags) { @@ -765,7 +771,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState // triggers a new partition and we end up with many duplicate partitions. This is // enhanced as the focus change can be much faster than the taking of the assist structure. // Hence remove the currently queued request and replace it with the one queued after the - // structure is taken. This causes only one fill request per bust of focus changes. + // structure is taken. This causes only one fill request per burst of focus changes. cancelCurrentRequestLocked(); // Only ask IME to create inline suggestions request if Autofill provider supports it and @@ -778,7 +784,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState && isViewFocusedLocked(flags)) { Consumer inlineSuggestionsRequestConsumer = mAssistReceiver.newAutofillRequestLocked(viewState, - /*isInlineRequest=*/ true); + /* isInlineRequest= */ true); if (inlineSuggestionsRequestConsumer != null) { final AutofillId focusedId = mCurrentViewId; remoteRenderService.getInlineSuggestionsRendererInfo( @@ -792,8 +798,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState viewState.setState(ViewState.STATE_PENDING_CREATE_INLINE_REQUEST); } } else { - mAssistReceiver.newAutofillRequestLocked(viewState, - /*isInlineRequest=*/ false); + mAssistReceiver.newAutofillRequestLocked(viewState, /* isInlineRequest= */ false); } // Now request the assist structure data. @@ -1170,7 +1175,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState return null; } - // FillServiceCallbacks + // VultureCallback + @Override + public void onServiceDied(@NonNull RemoteFillService service) { + Slog.w(TAG, "removing session because service died"); + forceRemoveSelfLocked(); + } + + // AutoFillUiCallback @Override public void authenticate(int requestId, int datasetIndex, IntentSender intent, Bundle extras, boolean authenticateInline) { @@ -1200,13 +1212,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState this, authenticationId, intent, fillInIntent, authenticateInline)); } - // VultureCallback - @Override - public void onServiceDied(@NonNull RemoteFillService service) { - Slog.w(TAG, "removing session because service died"); - forceRemoveSelfLocked(); - } - // AutoFillUiCallback @Override public void fill(int requestId, int datasetIndex, Dataset dataset) { @@ -1280,6 +1285,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } } + // AutoFillUiCallback @Override public void dispatchUnhandledKey(AutofillId id, KeyEvent keyEvent) { synchronized (mLock) { @@ -2239,31 +2245,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState mService.logSaveShown(id, mClientState); } - @Nullable - private ArrayMap createSanitizers(@Nullable SaveInfo saveInfo) { - if (saveInfo == null) return null; - - final InternalSanitizer[] sanitizerKeys = saveInfo.getSanitizerKeys(); - if (sanitizerKeys == null) return null; - - final int size = sanitizerKeys.length ; - final ArrayMap sanitizers = new ArrayMap<>(size); - if (sDebug) Slog.d(TAG, "Service provided " + size + " sanitizers"); - final AutofillId[][] sanitizerValues = saveInfo.getSanitizerValues(); - for (int i = 0; i < size; i++) { - final InternalSanitizer sanitizer = sanitizerKeys[i]; - final AutofillId[] ids = sanitizerValues[i]; - if (sDebug) { - Slog.d(TAG, "sanitizer #" + i + " (" + sanitizer + ") for ids " - + Arrays.toString(ids)); - } - for (AutofillId id : ids) { - sanitizers.put(id, sanitizer); - } - } - return sanitizers; - } - @Nullable private AutofillValue getSanitizedValue( @Nullable ArrayMap sanitizers, @@ -2328,12 +2309,12 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState */ @GuardedBy("mLock") @Nullable - private CharSequence[] getAutofillOptionsFromContextsLocked(AutofillId id) { + private CharSequence[] getAutofillOptionsFromContextsLocked(@NonNull AutofillId autofillId) { final int numContexts = mContexts.size(); - for (int i = numContexts - 1; i >= 0; i--) { final FillContext context = mContexts.get(i); - final ViewNode node = Helper.findViewNodeByAutofillId(context.getStructure(), id); + final ViewNode node = Helper.findViewNodeByAutofillId(context.getStructure(), + autofillId); if (node != null && node.getAutofillOptions() != null) { return node.getAutofillOptions(); } @@ -2439,7 +2420,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState // TODO(b/113281366): rather than merge it here, it might be better to simply reuse the old // session instead of creating a new one. But we need to consider what would happen on corner // cases such as "Main Activity M -> activity A with username -> activity B with password" - // If user follows the normal workflow, than session A would be merged with session B as + // If user follows the normal workflow, then session A would be merged with session B as // expected. But if when on Activity A the user taps back or somehow launches another activity, // session A could be merged with the wrong session. /** @@ -2532,11 +2513,11 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } requestNewFillResponseLocked(viewState, ViewState.STATE_STARTED_PARTITION, flags); return true; - } else { - if (sVerbose) { - Slog.v(TAG, "Not starting new partition for view " + id + ": " - + viewState.getStateAsString()); - } + } + + if (sVerbose) { + Slog.v(TAG, "Not starting new partition for view " + id + ": " + + viewState.getStateAsString()); } return false; } @@ -2897,21 +2878,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } } - /** - * Returns true if {@code s1} contains all characters of {@code s2}, in order. - */ - private static boolean containsCharsInOrder(String s1, String s2) { - int prevIndex = -1; - for (char ch : s2.toCharArray()) { - int index = TextUtils.indexOf(s1, ch, prevIndex + 1); - if (index == -1) { - return false; - } - prevIndex = index; - } - return true; - } - @Override public void onFillReady(@NonNull FillResponse response, @NonNull AutofillId filledId, @Nullable AutofillValue value) { @@ -3380,7 +3346,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState mInlineSessionController.getInlineSuggestionsRequestLocked().orElse(null)); } if (mAugmentedAutofillDestroyer == null) { - mAugmentedAutofillDestroyer = () -> remoteService.onDestroyAutofillWindowsRequest(); + mAugmentedAutofillDestroyer = remoteService::onDestroyAutofillWindowsRequest; } return mAugmentedAutofillDestroyer; }