diff options
3 files changed, 73 insertions, 24 deletions
diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java index e15baaeef570..1d9eb715ee3b 100644 --- a/core/java/android/view/autofill/AutofillManager.java +++ b/core/java/android/view/autofill/AutofillManager.java @@ -3056,19 +3056,17 @@ public final class AutofillManager { } @GuardedBy("mLock") - private void handleFailedIdsLocked(ArrayList<AutofillId> failedIds) { - if (failedIds != null && !failedIds.isEmpty()) { - 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... - throw e.rethrowFromSystemServer(); - } + private void handleFailedIdsLocked(@NonNull ArrayList<AutofillId> failedIds) { + if (!failedIds.isEmpty() && 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... + throw e.rethrowFromSystemServer(); } } @@ -3090,7 +3088,7 @@ public final class AutofillManager { final View[] views = client.autofillClientFindViewsByAutofillIdTraversal( Helper.toArray(ids)); - ArrayList<AutofillId> failedIds = null; + ArrayList<AutofillId> failedIds = new ArrayList<>(); for (int i = 0; i < itemCount; i++) { final AutofillId id = ids.get(i); @@ -3101,9 +3099,6 @@ public final class AutofillManager { // 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; } diff --git a/services/autofill/java/com/android/server/autofill/PresentationStatsEventLogger.java b/services/autofill/java/com/android/server/autofill/PresentationStatsEventLogger.java index 3645c40aeda2..9c84b123a435 100644 --- a/services/autofill/java/com/android/server/autofill/PresentationStatsEventLogger.java +++ b/services/autofill/java/com/android/server/autofill/PresentationStatsEventLogger.java @@ -545,6 +545,25 @@ public final class PresentationStatsEventLogger { }); } + /** + * Set views_fillable_total_count as long as mEventInternal presents. + */ + public void maybeSetViewFillableCounts(int totalFillableCount) { + mEventInternal.ifPresent(event -> { + event.mViewFillableTotalCount = totalFillableCount; + }); + } + + /** + * Set views_filled_failure_count using failure count as long as mEventInternal + * presents. + */ + public void maybeSetViewFillFailureCounts(int failureCount) { + mEventInternal.ifPresent(event -> { + event.mViewFillFailureCount = failureCount; + }); + } + public void logAndEndEvent() { if (!mEventInternal.isPresent()) { Slog.w(TAG, "Shouldn't be logging AutofillPresentationEventReported again for same " @@ -587,7 +606,9 @@ public final class PresentationStatsEventLogger { + " mFieldClassificationRequestId=" + event.mFieldClassificationRequestId + " mAppPackageUid=" + mCallingAppUid + " mIsCredentialRequest=" + event.mIsCredentialRequest - + " mWebviewRequestedCredential=" + event.mWebviewRequestedCredential); + + " mWebviewRequestedCredential=" + event.mWebviewRequestedCredential + + " mViewFillableTotalCount=" + event.mViewFillableTotalCount + + " mViewFillFailureCount=" + event.mViewFillFailureCount); } // TODO(b/234185326): Distinguish empty responses from other no presentation reasons. @@ -628,7 +649,9 @@ public final class PresentationStatsEventLogger { event.mFieldClassificationRequestId, mCallingAppUid, event.mIsCredentialRequest, - event.mWebviewRequestedCredential); + event.mWebviewRequestedCredential, + event.mViewFillableTotalCount, + event.mViewFillFailureCount); mEventInternal = Optional.empty(); } @@ -664,6 +687,8 @@ public final class PresentationStatsEventLogger { int mFieldClassificationRequestId = -1; boolean mIsCredentialRequest = false; boolean mWebviewRequestedCredential = false; + int mViewFillableTotalCount = -1; + int mViewFillFailureCount = -1; PresentationStatsEventInternal() {} } diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index a55b8d008a68..d006cf6d703a 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -170,8 +170,8 @@ import android.util.Slog; import android.util.SparseArray; import android.util.TimeUtils; import android.view.KeyEvent; -import android.view.autofill.AutofillId; import android.view.autofill.AutofillFeatureFlags; +import android.view.autofill.AutofillId; import android.view.autofill.AutofillManager; import android.view.autofill.AutofillManager.AutofillCommitReason; import android.view.autofill.AutofillManager.SmartSuggestionMode; @@ -597,6 +597,17 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState private boolean mIgnoreViewStateResetToEmpty; + /* + * Id of the previous view that was entered. Once set, it would only be replaced by non-null + * view ids. + * When a user focuses on a field, autofill request is sent. When the keyboard pops up, or the + * autofill dialog shows up, this field loses focus. After selecting a suggestion, focus goes + * back to the same field. This field allows to ignore focus loss when autofill dialog comes up. + * TODO(b/319872477): Note that there maybe some cases where we incorrectly detect focus loss. + */ + @GuardedBy("mLock") + private @Nullable AutofillId mPreviousNonNullEnteredViewId; + void onSwitchInputMethodLocked() { // One caveat is that for the case where the focus is on a field for which regular autofill // returns null, and augmented autofill is triggered, and then the user switches the input @@ -4390,6 +4401,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState case ACTION_START_SESSION: // View is triggering autofill. mCurrentViewId = viewState.id; + mPreviousNonNullEnteredViewId = viewState.id; viewState.update(value, virtualBounds, flags); startNewEventForPresentationStatsEventLogger(); mPresentationStatsEventLogger.maybeSetIsNewRequest(true); @@ -4459,6 +4471,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState if (value != null) { viewState.setCurrentValue(value); } + // isSameViewEntered has some limitations, where it isn't considered same view when + // autofill suggestions pop up, user selects, and the focus lands back on the view. + // isSameViewAgain tries to overcome that situation. + final boolean isSameViewAgain = isSameViewEntered + || Objects.equals(mCurrentViewId, mPreviousNonNullEnteredViewId); + if (mCurrentViewId != null) { + mPreviousNonNullEnteredViewId = mCurrentViewId; + } boolean isCredmanRequested = (flags & FLAG_VIEW_REQUESTS_CREDMAN_SERVICE) != 0; if (shouldRequestSecondaryProvider(flags)) { if (requestNewFillResponseOnViewEnteredIfNecessaryLocked( @@ -4510,7 +4530,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState // With Fill Dialog, request starts prior to view getting entered. So, we can't end // the event at this moment, otherwise we will be wrongly attributing fill dialog // event as concluded. - if (!wasPreviouslyFillDialog) { + if (!wasPreviouslyFillDialog && !isSameViewAgain) { + // TODO(b/319872477): Re-consider this logic below mPresentationStatsEventLogger.maybeSetNoPresentationEventReason( NOT_SHOWN_REASON_VIEW_FOCUS_CHANGED); mPresentationStatsEventLogger.logAndEndEvent(); @@ -4588,10 +4609,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState mCurrentViewId = null; } - + // It's not necessary that there's no more presentation for this view. It could + // be that the user chose some suggestion, in which case, view exits. mPresentationStatsEventLogger.maybeSetNoPresentationEventReason( NOT_SHOWN_REASON_VIEW_FOCUS_CHANGED); - mPresentationStatsEventLogger.logAndEndEvent(); } break; default: @@ -5327,6 +5348,9 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState */ @GuardedBy("mLock") void setAutofillFailureLocked(@NonNull List<AutofillId> ids) { + if (sVerbose && !ids.isEmpty()) { + Slog.v(TAG, "Total views that failed to populate: " + ids.size()); + } for (int i = 0; i < ids.size(); i++) { final AutofillId id = ids.get(i); final ViewState viewState = mViewStates.get(id); @@ -5341,6 +5365,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState Slog.v(TAG, "Changed state of " + id + " to " + viewState.getStateAsString()); } } + mPresentationStatsEventLogger.maybeSetViewFillFailureCounts(ids.size()); + mPresentationStatsEventLogger.logAndEndEvent(); } @GuardedBy("mLock") @@ -6526,8 +6552,11 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState if (waitingDatasetAuth) { mUi.hideFillUi(this); } + if (sVerbose) { + Slog.v(TAG, "Total views to be autofilled: " + ids.size()); + } + mPresentationStatsEventLogger.maybeSetViewFillableCounts(ids.size()); if (sDebug) Slog.d(TAG, "autoFillApp(): the buck is on the app: " + dataset); - mClient.autofill(id, ids, values, hideHighlight); if (dataset.getId() != null) { if (mSelectedDatasetIds == null) { |