From 49f08edf7d08101dfc00920f2abb1dab2280cd7e Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Mon, 26 Mar 2018 16:18:45 -0700 Subject: [PATCH] Recover dataset picker when view fail to autofill. When a dataset is selected, the framework tries to autofill all views belonging to it. But if one (or more view) failed to autofill, we should let the user recover by tapping the view again. This scenario typically happens when views are recycled. Test: atest MutableAutofillIdTest#testViewGoneDuringAutofillCanStillBeFilled Test: atest CtsAutoFillServiceTestCases # manually retrying flaky failures Fixes: 76149637 Change-Id: I7a6352c68b4a7d5e4cb80a7346c66efd831f21c8 --- .../view/autofill/AutofillManager.java | 27 ++++++++++++++++++- .../view/autofill/IAutoFillManager.aidl | 3 +++ .../autofill/AutofillManagerService.java | 12 +++++++++ .../autofill/AutofillManagerServiceImpl.java | 13 +++++++++ .../com/android/server/autofill/Session.java | 26 ++++++++++++++++++ .../android/server/autofill/ViewState.java | 2 ++ 6 files changed, 82 insertions(+), 1 deletion(-) diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java index 01fd09031d1e4..88300dbda1c44 100644 --- a/core/java/android/view/autofill/AutofillManager.java +++ b/core/java/android/view/autofill/AutofillManager.java @@ -1819,13 +1819,22 @@ public final class AutofillManager { final View[] views = client.autofillClientFindViewsByAutofillIdTraversal( Helper.toArray(ids)); + ArrayList failedIds = null; + for (int i = 0; i < itemCount; i++) { final AutofillId id = ids.get(i); final AutofillValue value = values.get(i); final int viewId = id.getViewId(); final View view = views[i]; if (view == null) { - Log.w(TAG, "autofill(): no View with id " + viewId); + // Most likely view has been removed after the initial request was sent to the + // the service; this is fine, but we need to update the view status in the + // server side so it can be triggered again. + Log.d(TAG, "autofill(): no View with id " + id); + if (failedIds == null) { + failedIds = new ArrayList<>(); + } + failedIds.add(id); continue; } if (id.isVirtual()) { @@ -1859,12 +1868,28 @@ public final class AutofillManager { } } + if (failedIds != null) { + if (sVerbose) { + Log.v(TAG, "autofill(): total failed views: " + failedIds); + } + try { + mService.setAutofillFailure(mSessionId, failedIds, mContext.getUserId()); + } catch (RemoteException e) { + // In theory, we could ignore this error since it's not a big deal, but + // in reality, we rather crash the app anyways, as the failure could be + // a consequence of something going wrong on the server side... + e.rethrowFromSystemServer(); + } + } + if (virtualValues != null) { for (int i = 0; i < virtualValues.size(); i++) { final View parent = virtualValues.keyAt(i); final SparseArray childrenValues = virtualValues.valueAt(i); parent.autofill(childrenValues); numApplied += childrenValues.size(); + // TODO: we should provide a callback so the parent can call failures; something + // like notifyAutofillFailed(View view, int[] childrenIds); } } diff --git a/core/java/android/view/autofill/IAutoFillManager.aidl b/core/java/android/view/autofill/IAutoFillManager.aidl index 56f79abf6c19e..176df7327b16d 100644 --- a/core/java/android/view/autofill/IAutoFillManager.aidl +++ b/core/java/android/view/autofill/IAutoFillManager.aidl @@ -16,6 +16,8 @@ package android.view.autofill; +import java.util.List; + import android.content.ComponentName; import android.graphics.Rect; import android.os.Bundle; @@ -47,6 +49,7 @@ interface IAutoFillManager { in AutofillId autoFillId, in Rect bounds, in AutofillValue value, int userId, boolean hasCallback, int flags, in ComponentName componentName, int sessionId, int action, boolean compatMode); + void setAutofillFailure(int sessionId, in List ids, int userId); void finishSession(int sessionId, int userId); void cancelSession(int sessionId, int userId); void setAuthenticationResult(in Bundle data, int sessionId, int authenticationId, int userId); diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java index 009e723d61ce5..51143085d3667 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerService.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerService.java @@ -1035,6 +1035,18 @@ public final class AutofillManagerService extends SystemService { return sessionId; } + @Override + public void setAutofillFailure(int sessionId, @NonNull List ids, int userId) { + synchronized (mLock) { + final AutofillManagerServiceImpl service = peekServiceForUserLocked(userId); + if (service != null) { + service.setAutofillFailureLocked(sessionId, getCallingUid(), ids); + } else if (sVerbose) { + Slog.v(TAG, "setAutofillFailure(): no service for " + userId); + } + } + } + @Override public void finishSession(int sessionId, int userId) { synchronized (mLock) { diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java index ef0d7e6b5727a..0bb29a7d1967e 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java @@ -403,6 +403,19 @@ final class AutofillManagerServiceImpl { } } + @GuardedBy("mLock") + void setAutofillFailureLocked(int sessionId, int uid, @NonNull List ids) { + if (!isEnabledLocked()) { + return; + } + final Session session = mSessions.get(sessionId); + if (session == null || uid != session.uid) { + Slog.v(TAG, "setAutofillFailure(): no session for " + sessionId + "(" + uid + ")"); + return; + } + session.setAutofillFailureLocked(ids); + } + @GuardedBy("mLock") void finishSessionLocked(int sessionId, int uid) { if (!isEnabledLocked()) { diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index bcb0bc413b536..706fb1a724911 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -1827,6 +1827,11 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } viewState.setState(ViewState.STATE_STARTED_PARTITION); requestNewFillResponseLocked(flags); + } else { + if (sVerbose) { + Slog.v(TAG, "Not starting new partition for view " + id + ": " + + viewState.getStateAsString()); + } } } @@ -2194,6 +2199,27 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } } + /** + * Sets the state of views that failed to autofill. + */ + @GuardedBy("mLock") + void setAutofillFailureLocked(@NonNull List ids) { + for (int i = 0; i < ids.size(); i++) { + final AutofillId id = ids.get(i); + final ViewState viewState = mViewStates.get(id); + if (viewState == null) { + Slog.w(TAG, "setAutofillFailure(): no view for id " + id); + continue; + } + viewState.resetState(ViewState.STATE_AUTOFILLED); + final int state = viewState.getState(); + viewState.setState(state | ViewState.STATE_AUTOFILL_FAILED); + if (sVerbose) { + Slog.v(TAG, "Changed state of " + id + " to " + viewState.getStateAsString()); + } + } + } + @GuardedBy("mLock") private void replaceResponseLocked(@NonNull FillResponse oldResponse, @NonNull FillResponse newResponse, @Nullable Bundle newClientState) { diff --git a/services/autofill/java/com/android/server/autofill/ViewState.java b/services/autofill/java/com/android/server/autofill/ViewState.java index 97ab97bc8d82b..9210de239b416 100644 --- a/services/autofill/java/com/android/server/autofill/ViewState.java +++ b/services/autofill/java/com/android/server/autofill/ViewState.java @@ -69,6 +69,8 @@ final class ViewState { public static final int STATE_RESTARTED_SESSION = 0x100; /** View is the URL bar of a package on compat mode. */ public static final int STATE_URL_BAR = 0x200; + /** View was asked to autofil but failed to do so. */ + public static final int STATE_AUTOFILL_FAILED = 0x400; public final AutofillId id;