From c2430f3c4d6e3b996917f57c8afb0b00b5bef45b Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Mon, 1 May 2017 09:35:33 -0700 Subject: [PATCH] Removed deprecated setAuthentication() method that didn't take ids. Such method would cause the AutofillUi to show on all fields, now it only shows in the fields the service is interested on. This doesn't solve FillResponse auth on multiple partition, but that will come soon... Bug: 37424539 Test: removed hack from testFillResponseAuthJustOneField() Test: CtsAutoFillServiceTestCases pass Change-Id: Id97dddfb9fc1630cd6bac96b9bae9d4a2986dd6d --- .../service/autofill/FillResponse.java | 38 +++++++------- .../autofill/AutofillManagerService.java | 6 +-- .../com/android/server/autofill/Session.java | 51 ++++++++++++++----- .../android/server/autofill/ViewState.java | 16 ++++-- 4 files changed, 71 insertions(+), 40 deletions(-) diff --git a/core/java/android/service/autofill/FillResponse.java b/core/java/android/service/autofill/FillResponse.java index 42c0151298b0f..1914db9eb4387 100644 --- a/core/java/android/service/autofill/FillResponse.java +++ b/core/java/android/service/autofill/FillResponse.java @@ -30,6 +30,7 @@ import android.view.autofill.AutofillManager; import android.widget.RemoteViews; import java.util.ArrayList; +import java.util.Arrays; /** * Response for a {@link @@ -258,12 +259,17 @@ public final class FillResponse implements Parcelable { * @param ids id of Views that when focused will display the authentication UI affordance. * * @return This builder. + * @throw {@link IllegalArgumentException} if {@code ids} is {@code null} or empty, or if + * neither {@code authentication} nor {@code presentation} is non-{@code null}. + * * @see android.app.PendingIntent#getIntentSender() */ public @NonNull Builder setAuthentication(@NonNull AutofillId[] ids, @Nullable IntentSender authentication, @Nullable RemoteViews presentation) { throwIfDestroyed(); - // TODO(b/37424539): assert ids is not null nor empty once old version is removed + if (ids == null || ids.length == 0) { + throw new IllegalArgumentException("ids cannot be null or empry"); + } if (authentication == null ^ presentation == null) { throw new IllegalArgumentException("authentication and presentation" + " must be both non-null or null"); @@ -274,17 +280,6 @@ public final class FillResponse implements Parcelable { return this; } - /** - * TODO(b/37424539): will be removed once clients use the version that takes ids - * @hide - * @deprecated - */ - @Deprecated - public @NonNull Builder setAuthentication(@Nullable IntentSender authentication, - @Nullable RemoteViews presentation) { - return setAuthentication(null, authentication, presentation); - } - /** * Specifies views that should not trigger new * {@link AutofillService#onFillRequest(FillRequest, android.os.CancellationSignal, @@ -396,6 +391,7 @@ public final class FillResponse implements Parcelable { public String toString() { if (!sDebug) return super.toString(); + // TODO: create a dump() method instead return new StringBuilder( "FillResponse : [mRequestId=" + mRequestId) .append(", datasets=").append(mDatasets) @@ -403,10 +399,8 @@ public final class FillResponse implements Parcelable { .append(", clientState=").append(mClientState != null) .append(", hasPresentation=").append(mPresentation != null) .append(", hasAuthentication=").append(mAuthentication != null) - .append(", authenticationSize=").append(mAuthenticationIds != null - ? mAuthenticationIds.length : "N/A") - .append(", ignoredIdsSize=").append(mIgnoredIds != null - ? mIgnoredIds.length : "N/A") + .append(", authenticationIds=").append(Arrays.toString(mAuthenticationIds)) + .append(", ignoredIds=").append(Arrays.toString(mIgnoredIds)) .append("]") .toString(); } @@ -447,8 +441,16 @@ public final class FillResponse implements Parcelable { } builder.setSaveInfo(parcel.readParcelable(null)); builder.setClientState(parcel.readParcelable(null)); - builder.setAuthentication(parcel.readParcelableArray(null, AutofillId.class), - parcel.readParcelable(null), parcel.readParcelable(null)); + + // Sets authentication state. + final AutofillId[] authenticationIds = parcel.readParcelableArray(null, + AutofillId.class); + final IntentSender authentication = parcel.readParcelable(null); + final RemoteViews presentation = parcel.readParcelable(null); + if (authenticationIds != null) { + builder.setAuthentication(authenticationIds, authentication, presentation); + } + builder.setIgnoredIds(parcel.readParcelableArray(null, AutofillId.class)); final FillResponse response = builder.build(); diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java index 2358ec55f6861..b536ad969c95c 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java @@ -603,15 +603,12 @@ public final class AutofillManagerService extends SystemService { if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return; boolean oldDebug = sDebug; - boolean oldVerbose = sVerbose; try { synchronized (mLock) { oldDebug = sDebug; - oldVerbose = sVerbose; setDebugLocked(true); - setVerboseLocked(true); pw.print("Debug mode: "); pw.println(oldDebug); - pw.print("Verbose mode: "); pw.println(oldVerbose); + pw.print("Verbose mode: "); pw.println(sVerbose); pw.print("Disabled users: "); pw.println(mDisabledUsers); final int size = mServicesCache.size(); pw.print("Cached services: "); @@ -631,7 +628,6 @@ public final class AutofillManagerService extends SystemService { mRequestsHistory.reverseDump(fd, pw, args); } finally { setDebugLocked(oldDebug); - setVerboseLocked(oldVerbose); } } diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index c85ce436375ea..018fb6886cd98 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -922,6 +922,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } } } + + if (ArrayUtils.contains(response.getAuthenticationIds(), id)) { + return false; + } } return true; @@ -943,8 +947,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState Slog.v(TAG, "Creating viewState for " + id + " on " + getActionAsString(action)); } - viewState = new ViewState(this, id, value, this, ViewState.STATE_INITIAL); + boolean isIgnored = isIgnoredLocked(id); + viewState = new ViewState(this, id, value, this, + isIgnored ? ViewState.STATE_IGNORED : ViewState.STATE_INITIAL); mViewStates.put(id, viewState); + if (isIgnored) { + if (sDebug) Slog.d(TAG, "updateLocked(): ignoring view " + id); + return; + } } else { if (sVerbose) Slog.v(TAG, "Ignored " + getActionAsString(action) + " for " + id); return; @@ -983,16 +993,11 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState break; case ACTION_VIEW_ENTERED: if (shouldStartNewPartitionLocked(id)) { - // TODO(b/37424539): proper implementation - if (mResponseWaitingAuth != null) { - viewState.setState(ViewState.STATE_WAITING_RESPONSE_AUTH); - } else { - if (sDebug) { - Slog.d(TAG, "Starting partition for view id " + viewState.id); - } - viewState.setState(ViewState.STATE_STARTED_PARTITION); - requestNewFillResponseLocked(flags); + if (sDebug) { + Slog.d(TAG, "Starting partition for view id " + viewState.id); } + viewState.setState(ViewState.STATE_STARTED_PARTITION); + requestNewFillResponseLocked(flags); } // Remove the UI if the ViewState has changed. @@ -1015,6 +1020,18 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } } + /** + * Checks whether a view should be ignored. + */ + private boolean isIgnoredLocked(AutofillId id) { + if (mResponses == null || mResponses.size() == 0) { + return false; + } + // Always check the latest response only + final FillResponse response = mResponses.valueAt(mResponses.size() - 1); + return ArrayUtils.contains(response.getIgnoredIds(), id); + } + @Override public void onFillReady(FillResponse response, AutofillId filledId, @Nullable AutofillValue value) { @@ -1149,18 +1166,24 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState final SaveInfo saveInfo = response.getSaveInfo(); if (saveInfo != null) { final AutofillId[] requiredIds = saveInfo.getRequiredIds(); - for (int i = 0; i < requiredIds.length; i++) { - final AutofillId id = requiredIds[i]; + for (AutofillId id : requiredIds) { createOrUpdateViewStateLocked(id, state, null); } final AutofillId[] optionalIds = saveInfo.getOptionalIds(); if (optionalIds != null) { - for (int i = 0; i < optionalIds.length; i++) { - final AutofillId id = optionalIds[i]; + for (AutofillId id : optionalIds) { createOrUpdateViewStateLocked(id, state, null); } } } + + final AutofillId[] authIds = response.getAuthenticationIds(); + if (authIds != null) { + for (AutofillId id : authIds) { + createOrUpdateViewStateLocked(id, state, null); + } + } + } /** diff --git a/services/autofill/java/com/android/server/autofill/ViewState.java b/services/autofill/java/com/android/server/autofill/ViewState.java index 7ca943511dc06..d114e14523732 100644 --- a/services/autofill/java/com/android/server/autofill/ViewState.java +++ b/services/autofill/java/com/android/server/autofill/ViewState.java @@ -17,6 +17,7 @@ package com.android.server.autofill; import static com.android.server.autofill.Helper.sDebug; +import static com.android.server.autofill.Helper.sVerbose; import android.annotation.Nullable; import android.graphics.Rect; @@ -61,8 +62,8 @@ final class ViewState { public static final int STATE_STARTED_PARTITION = 0x20; /** User select a dataset in this view, but service must authenticate first. */ public static final int STATE_WAITING_DATASET_AUTH = 0x40; - // TODO(b/37424539): temporary workaround until partitioning supports auth - public static final int STATE_WAITING_RESPONSE_AUTH = 0x80; + /** Service does not care about this view. */ + public static final int STATE_IGNORED = 0x80; public final AutofillId id; @@ -199,7 +200,16 @@ final class ViewState { void dump(String prefix, PrintWriter pw) { pw.print(prefix); pw.print("id:" ); pw.println(this.id); pw.print(prefix); pw.print("state:" ); pw.println(getStateAsString()); - pw.print(prefix); pw.print("has response:" ); pw.println(mResponse != null); + pw.print(prefix); pw.print("response:"); + if (mResponse == null) { + pw.println("N/A"); + } else { + if (sVerbose) { + pw.println(mResponse); + } else { + pw.println(mResponse.getRequestId()); + } + } pw.print(prefix); pw.print("initialValue:" ); pw.println(mInitialValue); pw.print(prefix); pw.print("currentValue:" ); pw.println(mCurrentValue); pw.print(prefix); pw.print("autofilledValue:" ); pw.println(mAutofilledValue);