From 27e202208b42e78fefcece893a4bb5d8fe2eae58 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Thu, 18 May 2017 15:24:11 -0700 Subject: [PATCH] Hide Autofill UI when non-savable id is gone. AutofillManager keeps track of which views the AutofillServiec is interested to save, so when these views are gone, the session is finished. But when the AutofillService returns a dataset whose views it can not save, the FillUi for these views are not hiding when the views are gone. This CL fixes this issue by: - Keeping track which non-savable views should be tracked. - Pass the view (instead of it's id) when the UI on such views should be hid. This CL also optimized some AIDL and internal calls by avoiding the creating of unnecessary Lists. Test: manual verification with Snapchat Test: existing CtsAutoFillServiceTestCases pass Test: new tests on MultipleFragmentLoginTest pass Fixes: 38199452 Change-Id: I78fa357962dbc6667146d8e08cd6bacb63e0f337 --- core/java/android/app/Activity.java | 20 +++++ .../android/view/autofill/AutofillManager.java | 98 +++++++++++++++++----- .../view/autofill/IAutoFillManagerClient.aidl | 4 +- .../java/com/android/server/autofill/Helper.java | 14 ++++ .../java/com/android/server/autofill/Session.java | 37 +++++++- 5 files changed, 146 insertions(+), 27 deletions(-) diff --git a/core/java/android/app/Activity.java b/core/java/android/app/Activity.java index 65e0b90384bf..b439c1dc02c4 100644 --- a/core/java/android/app/Activity.java +++ b/core/java/android/app/Activity.java @@ -7449,6 +7449,7 @@ public class Activity extends ContextThemeWrapper } /** @hide */ + @Override @NonNull public View[] findViewsByAccessibilityIdTraversal(@NonNull int[] viewIds) { final View[] views = new View[viewIds.length]; final ArrayList roots = @@ -7472,6 +7473,25 @@ public class Activity extends ContextThemeWrapper /** @hide */ @Override + @Nullable public View findViewByAccessibilityIdTraversal(int viewId) { + final ArrayList roots = + WindowManagerGlobal.getInstance().getRootViews(getActivityToken()); + for (int rootNum = 0; rootNum < roots.size(); rootNum++) { + final View rootView = roots.get(rootNum).getView(); + + if (rootView != null) { + final View view = rootView.findViewByAccessibilityIdTraversal(viewId); + if (view != null) { + return view; + } + } + } + + return null; + } + + /** @hide */ + @Override @NonNull public boolean[] getViewVisibility(@NonNull int[] viewIds) { final boolean[] isVisible = new boolean[viewIds.length]; final View views[] = findViewsByAccessibilityIdTraversal(viewIds); diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java index 39ac39951d23..d8b316e70320 100644 --- a/core/java/android/view/autofill/AutofillManager.java +++ b/core/java/android/view/autofill/AutofillManager.java @@ -186,6 +186,10 @@ public final class AutofillManager { @GuardedBy("mLock") @Nullable private TrackedViews mTrackedViews; + /** Views that are only tracked because they are fillable and could be anchoring the UI. */ + @GuardedBy("mLock") + @Nullable private ArraySet mFillableIds; + /** @hide */ public interface AutofillClient { /** @@ -238,13 +242,22 @@ public final class AutofillManager { boolean isVisibleForAutofill(); /** - * Find views by traversing the hierarchies of the client. + * Finds views by traversing the hierarchies of the client. * * @param viewIds The accessibility ids of the views to find * - * @return And array containing the views, or {@code null} if not found + * @return And array containing the views (empty if no views found). */ @NonNull View[] findViewsByAccessibilityIdTraversal(@NonNull int[] viewIds); + + /** + * Finds a view by traversing the hierarchies of the client. + * + * @param viewId The accessibility id of the views to find + * + * @return The view, or {@code null} if not found + */ + @Nullable View findViewByAccessibilityIdTraversal(int viewId); } /** @@ -471,8 +484,17 @@ public final class AutofillManager { */ public void notifyViewVisibilityChange(@NonNull View view, boolean isVisible) { synchronized (mLock) { - if (mEnabled && mSessionId != NO_SESSION && mTrackedViews != null) { - mTrackedViews.notifyViewVisibilityChange(view, isVisible); + if (mEnabled && mSessionId != NO_SESSION) { + if (!isVisible && mFillableIds != null) { + final AutofillId id = view.getAutofillId(); + if (mFillableIds.contains(id)) { + if (sDebug) Log.d(TAG, "Hidding UI when view " + id + " became invisible"); + requestHideFillUi(id, view); + } + } + if (mTrackedViews != null) { + mTrackedViews.notifyViewVisibilityChange(view, isVisible); + } } } } @@ -1048,9 +1070,10 @@ public final class AutofillManager { * * @param trackedIds The views to be tracked * @param saveOnAllViewsInvisible Finish the session once all tracked views are invisible. + * @param fillableIds Views that might anchor FillUI. */ - private void setTrackedViews(int sessionId, List trackedIds, - boolean saveOnAllViewsInvisible) { + private void setTrackedViews(int sessionId, @Nullable AutofillId[] trackedIds, + boolean saveOnAllViewsInvisible, @Nullable AutofillId[] fillableIds) { synchronized (mLock) { if (mEnabled && mSessionId == sessionId) { if (saveOnAllViewsInvisible) { @@ -1058,15 +1081,32 @@ public final class AutofillManager { } else { mTrackedViews = null; } + if (fillableIds != null) { + if (mFillableIds == null) { + mFillableIds = new ArraySet<>(fillableIds.length); + } + for (AutofillId id : fillableIds) { + mFillableIds.add(id); + } + if (sVerbose) { + Log.v(TAG, "setTrackedViews(): fillableIds=" + fillableIds + + ", mFillableIds" + mFillableIds); + } + } } } } - private void requestHideFillUi(int sessionId, AutofillId id) { + private void requestHideFillUi(AutofillId id) { final View anchor = findView(id); + if (sVerbose) Log.v(TAG, "requestHideFillUi(" + id + "): anchor = " + anchor); if (anchor == null) { return; } + requestHideFillUi(id, anchor); + } + + private void requestHideFillUi(AutofillId id, View anchor) { AutofillCallback callback = null; synchronized (mLock) { @@ -1123,6 +1163,18 @@ public final class AutofillManager { * * @return The array of viewIds. */ + // TODO: move to Helper as static method + @NonNull private int[] getViewIds(@NonNull AutofillId[] autofillIds) { + final int numIds = autofillIds.length; + final int[] viewIds = new int[numIds]; + for (int i = 0; i < numIds; i++) { + viewIds[i] = autofillIds[i].getViewId(); + } + + return viewIds; + } + + // TODO: move to Helper as static method @NonNull private int[] getViewIds(@NonNull List autofillIds) { final int numIds = autofillIds.size(); final int[] viewIds = new int[numIds]; @@ -1147,7 +1199,7 @@ public final class AutofillManager { return null; } - return client.findViewsByAccessibilityIdTraversal(new int[]{autofillId.getViewId()})[0]; + return client.findViewByAccessibilityIdTraversal(autofillId.getViewId()); } /** @hide */ @@ -1173,6 +1225,7 @@ public final class AutofillManager { * * @return {@code true} iff set is not empty and value is in set */ + // TODO: move to Helper as static method private boolean isInSet(@Nullable ArraySet set, T value) { return set != null && set.contains(value); } @@ -1186,6 +1239,7 @@ public final class AutofillManager { * @return The set including the new value. If set was {@code null}, a set containing only * the new value. */ + // TODO: move to Helper as static method @NonNull private ArraySet addToSet(@Nullable ArraySet set, T valueToAdd) { if (set == null) { @@ -1206,6 +1260,7 @@ public final class AutofillManager { * @return The set without the removed value. {@code null} if set was null, or is empty * after removal. */ + // TODO: move to Helper as static method @Nullable private ArraySet removeFromSet(@Nullable ArraySet set, T valueToRemove) { if (set == null) { @@ -1226,11 +1281,8 @@ public final class AutofillManager { * * @param trackedIds The views to be tracked */ - TrackedViews(List trackedIds) { - mVisibleTrackedIds = null; - mInvisibleTrackedIds = null; - - AutofillClient client = getClientLocked(); + TrackedViews(@Nullable AutofillId[] trackedIds) { + final AutofillClient client = getClientLocked(); if (trackedIds != null && client != null) { final boolean[] isVisible; @@ -1238,12 +1290,12 @@ public final class AutofillManager { isVisible = client.getViewVisibility(getViewIds(trackedIds)); } else { // All false - isVisible = new boolean[trackedIds.size()]; + isVisible = new boolean[trackedIds.length]; } - final int numIds = trackedIds.size(); + final int numIds = trackedIds.length; for (int i = 0; i < numIds; i++) { - final AutofillId id = trackedIds.get(i); + final AutofillId id = trackedIds[i]; if (isVisible[i]) { mVisibleTrackedIds = addToSet(mVisibleTrackedIds, id); @@ -1294,6 +1346,9 @@ public final class AutofillManager { } if (mVisibleTrackedIds == null) { + if (sVerbose) { + Log.v(TAG, "No more visible ids. Invisibile = " + mInvisibleTrackedIds); + } finishSessionLocked(); } } @@ -1473,8 +1528,7 @@ public final class AutofillManager { public void requestHideFillUi(int sessionId, AutofillId id) { final AutofillManager afm = mAfm.get(); if (afm != null) { - afm.mContext.getMainThreadHandler().post( - () -> afm.requestHideFillUi(sessionId, id)); + afm.mContext.getMainThreadHandler().post(() -> afm.requestHideFillUi(id)); } } @@ -1501,12 +1555,12 @@ public final class AutofillManager { } @Override - public void setTrackedViews(int sessionId, List ids, - boolean saveOnAllViewsInvisible) { + public void setTrackedViews(int sessionId, AutofillId[] ids, + boolean saveOnAllViewsInvisible, AutofillId[] fillableIds) { final AutofillManager afm = mAfm.get(); if (afm != null) { - afm.mContext.getMainThreadHandler().post( - () -> afm.setTrackedViews(sessionId, ids, saveOnAllViewsInvisible) + afm.mContext.getMainThreadHandler().post(() -> + afm.setTrackedViews(sessionId, ids, saveOnAllViewsInvisible, fillableIds) ); } } diff --git a/core/java/android/view/autofill/IAutoFillManagerClient.aidl b/core/java/android/view/autofill/IAutoFillManagerClient.aidl index 1d66f7f7f463..d18b1816e09e 100644 --- a/core/java/android/view/autofill/IAutoFillManagerClient.aidl +++ b/core/java/android/view/autofill/IAutoFillManagerClient.aidl @@ -52,8 +52,8 @@ oneway interface IAutoFillManagerClient { * Sets the views to track. If saveOnAllViewsInvisible is set and all these view are invisible * the session is finished automatically. */ - void setTrackedViews(int sessionId, in List ids, - boolean saveOnAllViewsInvisible); + void setTrackedViews(int sessionId, in @nullable AutofillId[] savableIds, + boolean saveOnAllViewsInvisible, in @nullable AutofillId[] fillableIds); /** * Requests showing the fill UI. diff --git a/services/autofill/java/com/android/server/autofill/Helper.java b/services/autofill/java/com/android/server/autofill/Helper.java index 0281f73d5b40..86e32e041a96 100644 --- a/services/autofill/java/com/android/server/autofill/Helper.java +++ b/services/autofill/java/com/android/server/autofill/Helper.java @@ -16,7 +16,10 @@ package com.android.server.autofill; +import android.annotation.Nullable; import android.os.Bundle; +import android.util.ArraySet; +import android.view.autofill.AutofillId; import java.util.Arrays; import java.util.Objects; @@ -68,4 +71,15 @@ public final class Helper { append(builder, bundle); return builder.toString(); } + + @Nullable + static AutofillId[] toArray(@Nullable ArraySet set) { + if (set == null) return null; + + final AutofillId[] array = new AutofillId[set.size()]; + for (int i = 0; i < set.size(); i++) { + array[i] = set.valueAt(i); + } + return array; + } } diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index 0122301b8bad..aa80075b0ca9 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -28,6 +28,7 @@ import static android.view.autofill.AutofillManager.ACTION_VIEW_EXITED; import static com.android.server.autofill.Helper.sDebug; import static com.android.server.autofill.Helper.sPartitionMaxCount; import static com.android.server.autofill.Helper.sVerbose; +import static com.android.server.autofill.Helper.toArray; import static com.android.server.autofill.ViewState.STATE_AUTOFILLED; import static com.android.server.autofill.ViewState.STATE_RESTARTED_SESSION; @@ -57,6 +58,7 @@ import android.service.autofill.FillResponse; import android.service.autofill.SaveInfo; import android.service.autofill.SaveRequest; import android.util.ArrayMap; +import android.util.ArraySet; import android.util.Slog; import android.util.SparseArray; import android.view.autofill.AutofillId; @@ -1139,15 +1141,20 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState // Only track the views of the last response as only those are reported back to the // service, see #showSaveLocked - ArrayList trackedViews = new ArrayList<>(); + final FillResponse response = mResponses.valueAt(getLastResponseIndex()); + + ArraySet trackedViews = null; boolean saveOnAllViewsInvisible = false; - SaveInfo saveInfo = mResponses.valueAt(getLastResponseIndex()).getSaveInfo(); + final SaveInfo saveInfo = response.getSaveInfo(); if (saveInfo != null) { saveOnAllViewsInvisible = (saveInfo.getFlags() & SaveInfo.FLAG_SAVE_ON_ALL_VIEWS_INVISIBLE) != 0; // We only need to track views if we want to save once they become invisible. if (saveOnAllViewsInvisible) { + if (trackedViews == null) { + trackedViews = new ArraySet<>(); + } if (saveInfo.getRequiredIds() != null) { Collections.addAll(trackedViews, saveInfo.getRequiredIds()); } @@ -1158,8 +1165,32 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } } + // Must also track that are part of datasets, otherwise the FillUI won't be hidden when + // they go away (if they're not savable). + + final ArrayList datasets = response.getDatasets(); + ArraySet fillableIds = null; + if (datasets != null) { + for (int i = 0; i < datasets.size(); i++) { + final Dataset dataset = datasets.get(i); + final ArrayList fieldIds = dataset.getFieldIds(); + if (fieldIds == null) continue; + + for (int j = 0; j < fieldIds.size(); j++) { + final AutofillId id = fieldIds.get(j); + if (trackedViews == null || !trackedViews.contains(id)) { + fillableIds = ArrayUtils.add(fillableIds, id); + } + } + } + } + try { - mClient.setTrackedViews(id, trackedViews, saveOnAllViewsInvisible); + if (sVerbose) { + Slog.v(TAG, "updateTrackedIdsLocked(): " + trackedViews + " => " + fillableIds); + } + mClient.setTrackedViews(id, toArray(trackedViews), saveOnAllViewsInvisible, + toArray(fillableIds)); } catch (RemoteException e) { Slog.w(TAG, "Cannot set tracked ids", e); } -- 2.11.0