Merge "Autofill Fix Presentation Log Issues" into main

5 files changed, 299 insertions, 121 deletions

diff --git a/services/autofill/java/com/android/server/autofill/PresentationStatsEventLogger.java b/services/autofill/java/com/android/server/autofill/PresentationStatsEventLogger.java
index 930af5e7f056..5044e93b293d 100644
--- a/services/autofill/java/com/android/server/autofill/PresentationStatsEventLogger.java
+++ b/services/autofill/java/com/android/server/autofill/PresentationStatsEventLogger.java
@@ -71,6 +71,7 @@ import android.util.ArraySet;
 import android.util.Slog;
 import android.view.autofill.AutofillId;
 import android.view.autofill.AutofillManager;
+import android.view.autofill.AutofillValue;
 
 import com.android.internal.util.FrameworkStatsLog;
 
@@ -244,10 +245,18 @@ public final class PresentationStatsEventLogger {
             Slog.e(TAG, "Failed to start new event because already have active event.");
             return;
         }
+        Slog.d(TAG, "Started new PresentationStatsEvent");
         mEventInternal = Optional.of(new PresentationStatsEventInternal());
     }
 
     /**
+     * Test use only, returns a copy of the events object
+     */
+    Optional<PresentationStatsEventInternal> getInternalEvent() {
+        return mEventInternal;
+    }
+
+    /**
      * Set request_id
      */
     public void maybeSetRequestId(int requestId) {
@@ -339,10 +348,16 @@ public final class PresentationStatsEventLogger {
         });
     }
 
-    public void maybeSetCountShown(int datasets) {
+    /**
+     * This is called when a dataset is shown to the user. Will set the count shown,
+     * related timestamps and presentation reason.
+     */
+    public void logWhenDatasetShown(int datasets) {
         mEventInternal.ifPresent(
                 event -> {
+                    maybeSetSuggestionPresentedTimestampMs();
                     event.mCountShown = datasets;
+                    event.mNoPresentationReason = NOT_SHOWN_REASON_ANY_SHOWN;
                 });
     }
 
@@ -405,7 +420,12 @@ public final class PresentationStatsEventLogger {
 
     public void maybeSetDisplayPresentationType(@UiType int uiType) {
         mEventInternal.ifPresent(event -> {
-            event.mDisplayPresentationType = getDisplayPresentationType(uiType);
+            // There are cases in which another UI type will show up after selects a dataset
+            // such as with Inline after Fill Dialog. Set as the first presentation type only.
+            if (event.mDisplayPresentationType
+                    == AUTOFILL_PRESENTATION_EVENT_REPORTED__DISPLAY_PRESENTATION_TYPE__UNKNOWN_AUTOFILL_DISPLAY_PRESENTATION_TYPE) {
+                event.mDisplayPresentationType = getDisplayPresentationType(uiType);
+            }
         });
     }
 
@@ -430,9 +450,12 @@ public final class PresentationStatsEventLogger {
     }
 
     public void maybeSetSuggestionSentTimestampMs(int timestamp) {
-        mEventInternal.ifPresent(event -> {
-            event.mSuggestionSentTimestampMs = timestamp;
-        });
+        mEventInternal.ifPresent(
+                event -> {
+                    if (event.mSuggestionSentTimestampMs == DEFAULT_VALUE_INT) {
+                        event.mSuggestionSentTimestampMs = timestamp;
+                    }
+                });
     }
 
     public void maybeSetSuggestionSentTimestampMs() {
@@ -481,8 +504,6 @@ public final class PresentationStatsEventLogger {
 
     public void maybeSetInlinePresentationAndSuggestionHostUid(Context context, int userId) {
         mEventInternal.ifPresent(event -> {
-            event.mDisplayPresentationType =
-                    AUTOFILL_PRESENTATION_EVENT_REPORTED__DISPLAY_PRESENTATION_TYPE__INLINE;
             String imeString = Settings.Secure.getStringForUser(context.getContentResolver(),
                     Settings.Secure.DEFAULT_INPUT_METHOD, userId);
             if (TextUtils.isEmpty(imeString)) {
@@ -602,40 +623,56 @@ public final class PresentationStatsEventLogger {
     }
 
     /**
+     * Sets the field length whenever the text changes. Will keep track of the first
+     * and last modification lengths.
+     */
+    public void updateTextFieldLength(AutofillValue value) {
+        mEventInternal.ifPresent(event -> {
+            if (value == null || !value.isText()) {
+                return;
+            }
+
+            int length = value.getTextValue().length();
+
+            if (event.mFieldFirstLength == DEFAULT_VALUE_INT) {
+                event.mFieldFirstLength = length;
+            }
+            event.mFieldLastLength = length;
+        });
+    }
+
+    /**
      * Set various timestamps whenever the ViewState is modified
      *
      * <p>If the ViewState contains ViewState.STATE_AUTOFILLED, sets field_autofilled_timestamp_ms
      * else, set field_first_modified_timestamp_ms (if unset) and field_last_modified_timestamp_ms
      */
-    public void onFieldTextUpdated(ViewState state, int length) {
+    public void onFieldTextUpdated(ViewState state, AutofillValue value) {
         mEventInternal.ifPresent(event -> {
-                    int timestamp = getElapsedTime();
-                    // Focused id should be set before this is called
-                    if (state == null || state.id == null || state.id.getViewId() != event.mFocusedId) {
-                        // if these don't match, the currently field different than before
-                        Slog.w(
-                                TAG,
-                                "Bad view state for: " + event.mFocusedId);
-                        return;
-                    }
+            int timestamp = getElapsedTime();
+            // Focused id should be set before this is called
+            if (state == null || state.id == null || state.id.getViewId() != event.mFocusedId) {
+                // if these don't match, the currently field different than before
+                Slog.w(
+                        TAG,
+                        "Bad view state for: " + event.mFocusedId + ", state: " + state);
+                return;
+            }
 
-                    // Text changed because filling into form, just log Autofill timestamp
-                    if ((state.getState() & ViewState.STATE_AUTOFILLED) != 0) {
-                        event.mAutofilledTimestampMs = timestamp;
-                        return;
-                    }
+            updateTextFieldLength(value);
 
-                    // Set length variables
-                    if (event.mFieldFirstLength == DEFAULT_VALUE_INT) {
-                        event.mFieldFirstLength = length;
-                    }
-                    event.mFieldLastLength = length;
+            // Text changed because filling into form, just log Autofill timestamp
+            if ((state.getState() & ViewState.STATE_AUTOFILLED) != 0) {
+                event.mAutofilledTimestampMs = timestamp;
+                return;
+            }
 
-                    // Set timestamp variables
-                    if (event.mFieldModifiedFirstTimestampMs == DEFAULT_VALUE_INT) {
-                        event.mFieldModifiedFirstTimestampMs = timestamp;
-                    }
-                    event.mFieldModifiedLastTimestampMs = timestamp;
+
+            // Set timestamp variables
+            if (event.mFieldModifiedFirstTimestampMs == DEFAULT_VALUE_INT) {
+                event.mFieldModifiedFirstTimestampMs = timestamp;
+            }
+            event.mFieldModifiedLastTimestampMs = timestamp;
         });
     }
 
@@ -796,7 +833,10 @@ public final class PresentationStatsEventLogger {
         });
     }
 
-    public void logAndEndEvent() {
+    /**
+     * Finish and log the event.
+     */
+    public void logAndEndEvent(String caller) {
         if (!mEventInternal.isPresent()) {
             Slog.w(TAG, "Shouldn't be logging AutofillPresentationEventReported again for same "
                     + "event");
@@ -804,7 +844,8 @@ public final class PresentationStatsEventLogger {
         }
         PresentationStatsEventInternal event = mEventInternal.get();
         if (sVerbose) {
-            Slog.v(TAG, "Log AutofillPresentationEventReported:"
+            Slog.v(TAG, "(" + caller + ") "
+                    + "Log AutofillPresentationEventReported:"
                     + " requestId=" + event.mRequestId
                     + " sessionId=" + mSessionId
                     + " mNoPresentationEventReason=" + event.mNoPresentationReason
@@ -926,7 +967,7 @@ public final class PresentationStatsEventLogger {
         mEventInternal = Optional.empty();
     }
 
-    private static final class PresentationStatsEventInternal {
+    static final class PresentationStatsEventInternal {
         int mRequestId;
         @NotShownReason int mNoPresentationReason = NOT_SHOWN_REASON_UNKNOWN;
         boolean mIsDatasetAvailable;
diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java
index 6dea8b052173..c75fd0b7e025 100644
--- a/services/autofill/java/com/android/server/autofill/Session.java
+++ b/services/autofill/java/com/android/server/autofill/Session.java
@@ -28,7 +28,6 @@ import static android.service.autofill.Dataset.PICK_REASON_PROVIDER_DETECTION_ON
 import static android.service.autofill.Dataset.PICK_REASON_PROVIDER_DETECTION_PREFERRED_WITH_PCC;
 import static android.service.autofill.Dataset.PICK_REASON_UNKNOWN;
 import static android.service.autofill.FillEventHistory.Event.UI_TYPE_CREDMAN_BOTTOM_SHEET;
-import static android.service.autofill.FillEventHistory.Event.UI_TYPE_DIALOG;
 import static android.service.autofill.FillEventHistory.Event.UI_TYPE_INLINE;
 import static android.service.autofill.FillEventHistory.Event.UI_TYPE_UNKNOWN;
 import static android.service.autofill.FillRequest.FLAG_MANUAL_REQUEST;
@@ -67,10 +66,10 @@ import static com.android.server.autofill.FillResponseEventLogger.RESPONSE_STATU
 import static com.android.server.autofill.FillResponseEventLogger.RESPONSE_STATUS_SUCCESS;
 import static com.android.server.autofill.FillResponseEventLogger.RESPONSE_STATUS_TIMEOUT;
 import static com.android.server.autofill.FillResponseEventLogger.RESPONSE_STATUS_TRANSACTION_TOO_LARGE;
+import static com.android.server.autofill.Helper.SaveInfoStats;
 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.SaveInfoStats;
 import static com.android.server.autofill.Helper.sDebug;
 import static com.android.server.autofill.Helper.sVerbose;
 import static com.android.server.autofill.Helper.toArray;
@@ -78,6 +77,7 @@ import static com.android.server.autofill.PresentationStatsEventLogger.AUTHENTIC
 import static com.android.server.autofill.PresentationStatsEventLogger.AUTHENTICATION_RESULT_SUCCESS;
 import static com.android.server.autofill.PresentationStatsEventLogger.AUTHENTICATION_TYPE_DATASET_AUTHENTICATION;
 import static com.android.server.autofill.PresentationStatsEventLogger.AUTHENTICATION_TYPE_FULL_AUTHENTICATION;
+import static com.android.server.autofill.PresentationStatsEventLogger.NOT_SHOWN_REASON_ANY_SHOWN;
 import static com.android.server.autofill.PresentationStatsEventLogger.NOT_SHOWN_REASON_NO_FOCUS;
 import static com.android.server.autofill.PresentationStatsEventLogger.NOT_SHOWN_REASON_REQUEST_FAILED;
 import static com.android.server.autofill.PresentationStatsEventLogger.NOT_SHOWN_REASON_REQUEST_TIMEOUT;
@@ -1288,11 +1288,11 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
      * Clears the existing response for the partition, reads a new structure, and then requests a
      * new fill response from the fill service.
      *
-     * <p> Also asks the IME to make an inline suggestions request if it's enabled.
+     * <p>Also asks the IME to make an inline suggestions request if it's enabled.
      */
     @GuardedBy("mLock")
-    private void requestNewFillResponseLocked(@NonNull ViewState viewState, int newState,
-            int flags) {
+    private Optional<Integer> requestNewFillResponseLocked(
+            @NonNull ViewState viewState, int newState, int flags) {
         boolean isSecondary = shouldRequestSecondaryProvider(flags);
         final FillResponse existingResponse = isSecondary
                 ? viewState.getSecondaryResponse() : viewState.getResponse();
@@ -1333,7 +1333,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
             mFillRequestEventLogger.maybeSetIsAugmented(true);
             mFillRequestEventLogger.logAndEndEvent();
             triggerAugmentedAutofillLocked(flags);
-            return;
+            return Optional.empty();
         }
 
         viewState.setState(newState);
@@ -1353,11 +1353,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                     + ", flags=" + flags);
         }
         boolean isCredmanRequested = (flags & FLAG_VIEW_REQUESTS_CREDMAN_SERVICE) != 0;
-        mPresentationStatsEventLogger.maybeSetRequestId(requestId);
-        mPresentationStatsEventLogger.maybeSetIsCredentialRequest(isCredmanRequested);
-        mPresentationStatsEventLogger.maybeSetFieldClassificationRequestId(
-                mFieldClassificationIdSnapshot);
-        mPresentationStatsEventLogger.maybeSetAutofillServiceUid(getAutofillServiceUid());
         mFillRequestEventLogger.maybeSetRequestId(requestId);
         mFillRequestEventLogger.maybeSetAutofillServiceUid(getAutofillServiceUid());
         mSaveEventLogger.maybeSetAutofillServiceUid(getAutofillServiceUid());
@@ -1417,6 +1412,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
 
         // Now request the assist structure data.
         requestAssistStructureLocked(requestId, flags);
+
+        return Optional.of(requestId);
     }
 
     private boolean isRequestSupportFillDialog(int flags) {
@@ -1662,6 +1659,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         final LogMaker requestLog;
 
         synchronized (mLock) {
+            mPresentationStatsEventLogger.maybeSetRequestId(requestId);
             // Start a new FillResponse logger for the success case.
             mFillResponseEventLogger.startLogForNewResponse();
             mFillResponseEventLogger.maybeSetRequestId(requestId);
@@ -2419,7 +2417,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                         NOT_SHOWN_REASON_REQUEST_FAILED);
                 mFillResponseEventLogger.maybeSetResponseStatus(RESPONSE_STATUS_FAILURE);
             }
-            mPresentationStatsEventLogger.logAndEndEvent();
+            mPresentationStatsEventLogger.logAndEndEvent("fill request failure");
             mFillResponseEventLogger.maybeSetLatencyResponseProcessingMillis();
             mFillResponseEventLogger.logAndEndEvent();
         }
@@ -2642,6 +2640,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
     public void onShown(int uiType, int numDatasetsShown) {
         synchronized (mLock) {
             mPresentationStatsEventLogger.maybeSetDisplayPresentationType(uiType);
+            mPresentationStatsEventLogger.maybeSetNoPresentationEventReason(
+                    NOT_SHOWN_REASON_ANY_SHOWN);
 
             if (uiType == UI_TYPE_INLINE) {
                 // Inline Suggestions are inflated one at a time
@@ -2657,7 +2657,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 }
                 mLoggedInlineDatasetShown = true;
             } else {
-                mPresentationStatsEventLogger.maybeSetCountShown(numDatasetsShown);
+                mPresentationStatsEventLogger.logWhenDatasetShown(numDatasetsShown);
                 // Explicitly sets maybeSetSuggestionPresentedTimestampMs
                 mPresentationStatsEventLogger.maybeSetSuggestionPresentedTimestampMs();
                 mService.logDatasetShown(this.id, mClientState, uiType);
@@ -2800,7 +2800,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
             if (mCurrentViewId == null) {
                 return;
             }
+            mPresentationStatsEventLogger.logAndEndEvent("fallback from fill dialog");
+            startNewEventForPresentationStatsEventLogger();
             final ViewState currentView = mViewStates.get(mCurrentViewId);
+            logPresentationStatsOnViewEnteredLocked(currentView.getResponse(), false);
             currentView.maybeCallOnFillReady(mFlags);
         }
     }
@@ -2850,7 +2853,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         if (requestId == AUGMENTED_AUTOFILL_REQUEST_ID) {
             setAuthenticationResultForAugmentedAutofillLocked(data, authenticationId);
             // Augmented autofill is not logged.
-            mPresentationStatsEventLogger.logAndEndEvent();
+            mPresentationStatsEventLogger.logAndEndEvent("authentication - augmented");
             return;
         }
         if (mResponses == null) {
@@ -2859,7 +2862,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
             Slog.w(TAG, "setAuthenticationResultLocked(" + authenticationId + "): no responses");
             mPresentationStatsEventLogger.maybeSetAuthenticationResult(
                     AUTHENTICATION_RESULT_FAILURE);
-            mPresentationStatsEventLogger.logAndEndEvent();
+            mPresentationStatsEventLogger.logAndEndEvent("authentication - no response");
             removeFromService();
             return;
         }
@@ -2870,7 +2873,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
             Slog.w(TAG, "no authenticated response");
             mPresentationStatsEventLogger.maybeSetAuthenticationResult(
                     AUTHENTICATION_RESULT_FAILURE);
-            mPresentationStatsEventLogger.logAndEndEvent();
+            mPresentationStatsEventLogger.logAndEndEvent("authentication - bad response");
             removeFromService();
             return;
         }
@@ -2885,7 +2888,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 Slog.w(TAG, "no dataset with index " + datasetIdx + " on fill response");
                 mPresentationStatsEventLogger.maybeSetAuthenticationResult(
                         AUTHENTICATION_RESULT_FAILURE);
-                mPresentationStatsEventLogger.logAndEndEvent();
+                mPresentationStatsEventLogger.logAndEndEvent("authentication - no datasets");
                 removeFromService();
                 return;
             }
@@ -3330,7 +3333,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
 
         mPresentationStatsEventLogger.maybeSetNoPresentationEventReason(
                 PresentationStatsEventLogger.getNoPresentationEventReason(commitReason));
-        mPresentationStatsEventLogger.logAndEndEvent();
+        mPresentationStatsEventLogger.logAndEndEvent("Context committed");
 
         final int flags = lastResponse.getFlags();
         if ((flags & FillResponse.FLAG_TRACK_CONTEXT_COMMITED) == 0) {
@@ -4299,6 +4302,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
      * Starts (if necessary) a new fill request upon entering a view.
      *
      * <p>A new request will be started in 2 scenarios:
+     *
      * <ol>
      *   <li>If the user manually requested autofill.
      *   <li>If the view is part of a new partition.
@@ -4307,18 +4311,17 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
      * @param id The id of the view that is entered.
      * @param viewState The view that is entered.
      * @param flags The flag that was passed by the AutofillManager.
-     *
      * @return {@code true} if a new fill response is requested.
      */
     @GuardedBy("mLock")
-    private boolean requestNewFillResponseOnViewEnteredIfNecessaryLocked(@NonNull AutofillId id,
-            @NonNull ViewState viewState, int flags) {
+    private Optional<Integer> requestNewFillResponseOnViewEnteredIfNecessaryLocked(
+            @NonNull AutofillId id, @NonNull ViewState viewState, int flags) {
         // Force new response for manual request
         if ((flags & FLAG_MANUAL_REQUEST) != 0) {
             mSessionFlags.mAugmentedAutofillOnly = false;
             if (sDebug) Slog.d(TAG, "Re-starting session on view " + id + " and flags " + flags);
-            requestNewFillResponseLocked(viewState, ViewState.STATE_RESTARTED_SESSION, flags);
-            return true;
+            return requestNewFillResponseLocked(
+                    viewState, ViewState.STATE_RESTARTED_SESSION, flags);
         }
 
         // If it's not, then check if it should start a partition.
@@ -4331,15 +4334,15 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
             // Sometimes activity contain IMPORTANT_FOR_AUTOFILL_NO fields which marks session as
             // augmentedOnly, but other fields are still fillable by standard autofill.
             mSessionFlags.mAugmentedAutofillOnly = false;
-            requestNewFillResponseLocked(viewState, ViewState.STATE_STARTED_PARTITION, flags);
-            return true;
+            return requestNewFillResponseLocked(
+                    viewState, ViewState.STATE_STARTED_PARTITION, flags);
         }
 
         if (sVerbose) {
             Slog.v(TAG, "Not starting new partition for view " + id + ": "
                     + viewState.getStateAsString());
         }
-        return false;
+        return Optional.empty();
     }
 
     /**
@@ -4428,31 +4431,32 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
     void updateLocked(AutofillId id, Rect virtualBounds, AutofillValue value, int action,
             int flags) {
         if (mDestroyed) {
-            Slog.w(TAG, "Call to Session#updateLocked() rejected - session: "
-                    + id + " destroyed");
+            Slog.w(TAG, "updateLocked(" + id + "):  rejected - session: destroyed");
             return;
         }
         if (action == ACTION_RESPONSE_EXPIRED) {
             mSessionFlags.mExpiredResponse = true;
             if (sDebug) {
-                Slog.d(TAG, "Set the response has expired.");
+                Slog.d(TAG, "updateLocked(" + id + "): Set the response has expired.");
             }
             mPresentationStatsEventLogger.maybeSetNoPresentationEventReasonIfNoReasonExists(
                         NOT_SHOWN_REASON_VIEW_CHANGED);
-            mPresentationStatsEventLogger.logAndEndEvent();
+            mPresentationStatsEventLogger.logAndEndEvent("ACTION_RESPONSE_EXPIRED");
             return;
         }
 
         id.setSessionId(this.id);
-        if (sVerbose) {
-            Slog.v(TAG, "updateLocked(" + this.id + "): id=" + id + ", action="
-                    + actionAsString(action) + ", flags=" + flags);
-        }
         ViewState viewState = mViewStates.get(id);
         if (sVerbose) {
-            Slog.v(TAG, "updateLocked(" + this.id + "): mCurrentViewId=" + mCurrentViewId
-                    + ", mExpiredResponse=" + mSessionFlags.mExpiredResponse
-                    + ", viewState=" + viewState);
+            Slog.v(
+                    TAG,
+                    "updateLocked(" + id + "): "
+                            + "id=" + this.id
+                            + ", action=" + actionAsString(action)
+                            + ", flags=" + flags
+                            + ", mCurrentViewId=" + mCurrentViewId
+                            + ", mExpiredResponse=" + mSessionFlags.mExpiredResponse
+                            + ", viewState=" + viewState);
         }
 
         if (viewState == null) {
@@ -4505,14 +4509,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                     mSessionFlags.mFillDialogDisabled = true;
                     mPreviouslyFillDialogPotentiallyStarted = false;
                 } else {
-                    // Set the default reason for now if the user doesn't trigger any focus event
-                    // on the autofillable view. This can be changed downstream when more
-                    // information is available or session is committed.
-                    mPresentationStatsEventLogger.maybeSetNoPresentationEventReason(
-                            NOT_SHOWN_REASON_NO_FOCUS);
                     mPreviouslyFillDialogPotentiallyStarted = true;
                 }
-                requestNewFillResponseLocked(viewState, ViewState.STATE_STARTED_SESSION, flags);
+                Optional<Integer> maybeRequestId =
+                        requestNewFillResponseLocked(
+                                viewState, ViewState.STATE_STARTED_SESSION, flags);
+                if (maybeRequestId.isPresent()) {
+                    mPresentationStatsEventLogger.maybeSetRequestId(maybeRequestId.get());
+                }
                 break;
             case ACTION_VALUE_CHANGED:
                 if (mCompatMode && (viewState.getState() & ViewState.STATE_URL_BAR) != 0) {
@@ -4577,8 +4581,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 }
                 boolean isCredmanRequested = (flags & FLAG_VIEW_REQUESTS_CREDMAN_SERVICE) != 0;
                 if (shouldRequestSecondaryProvider(flags)) {
-                    if (requestNewFillResponseOnViewEnteredIfNecessaryLocked(
-                            id, viewState, flags)) {
+                    Optional<Integer> maybeRequestIdCred =
+                            requestNewFillResponseOnViewEnteredIfNecessaryLocked(
+                                    id, viewState, flags);
+                    if (maybeRequestIdCred.isPresent()) {
                         Slog.v(TAG, "Started a new fill request for secondary provider.");
                         return;
                     }
@@ -4622,17 +4628,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                     mLogViewEntered = true;
                 }
 
-                // Previously, fill request will only start whenever a view is entered.
-                // 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 && !isSameViewAgain) {
-                    // TODO(b/319872477): Re-consider this logic below
-                    mPresentationStatsEventLogger.maybeSetNoPresentationEventReason(
-                            NOT_SHOWN_REASON_VIEW_FOCUS_CHANGED);
-                    mPresentationStatsEventLogger.logAndEndEvent();
-                }
-
+                // Trigger augmented autofill if applicable
                 if ((flags & FLAG_MANUAL_REQUEST) == 0) {
                     // Not a manual request
                     if (mAugmentedAutofillableIds != null && mAugmentedAutofillableIds.contains(
@@ -4658,26 +4654,25 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                         return;
                     }
                 }
-                // If previous request was FillDialog request, a logger event was already started
-                if (!wasPreviouslyFillDialog) {
+
+                Optional<Integer> maybeNewRequestId =
+                        requestNewFillResponseOnViewEnteredIfNecessaryLocked(id, viewState, flags);
+
+                // Previously, fill request will only start whenever a view is entered.
+                // 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
+                        && (!isSameViewEntered || maybeNewRequestId.isPresent())) {
                     startNewEventForPresentationStatsEventLogger();
-                }
-                if (requestNewFillResponseOnViewEnteredIfNecessaryLocked(id, viewState, flags)) {
-                    // If a new request was issued even if previously it was fill dialog request,
-                    // we should end the log event, and start a new one. However, it leaves us
-                    // susceptible to race condition. But since mPresentationStatsEventLogger is
-                    // lock guarded, we should be safe.
-                    if (wasPreviouslyFillDialog) {
-                        mPresentationStatsEventLogger.logAndEndEvent();
-                        startNewEventForPresentationStatsEventLogger();
+                    if (maybeNewRequestId.isPresent()) {
+                        mPresentationStatsEventLogger.maybeSetRequestId(maybeNewRequestId.get());
                     }
-                    return;
                 }
 
-                FillResponse response = viewState.getResponse();
-                if (response != null) {
-                    logPresentationStatsOnViewEnteredLocked(response, isCredmanRequested);
-                }
+                logPresentationStatsOnViewEnteredLocked(
+                        viewState.getResponse(), isCredmanRequested);
+                mPresentationStatsEventLogger.updateTextFieldLength(value);
 
                 if (isSameViewEntered) {
                     setFillDialogDisabledAndStartInput();
@@ -4719,13 +4714,17 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
     @GuardedBy("mLock")
     private void logPresentationStatsOnViewEnteredLocked(FillResponse response,
             boolean isCredmanRequested) {
-        mPresentationStatsEventLogger.maybeSetRequestId(response.getRequestId());
         mPresentationStatsEventLogger.maybeSetIsCredentialRequest(isCredmanRequested);
         mPresentationStatsEventLogger.maybeSetFieldClassificationRequestId(
                 mFieldClassificationIdSnapshot);
-        mPresentationStatsEventLogger.maybeSetAvailableCount(
-                response.getDatasets(), mCurrentViewId);
+        mPresentationStatsEventLogger.maybeSetAutofillServiceUid(getAutofillServiceUid());
         mPresentationStatsEventLogger.maybeSetFocusedId(mCurrentViewId);
+
+        if (response != null) {
+            mPresentationStatsEventLogger.maybeSetRequestId(response.getRequestId());
+            mPresentationStatsEventLogger.maybeSetAvailableCount(
+                    response.getDatasets(), mCurrentViewId);
+        }
     }
 
     @GuardedBy("mLock")
@@ -4796,8 +4795,12 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
 
         viewState.setCurrentValue(value);
         final String filterText = textValue;
-
         final AutofillValue filledValue = viewState.getAutofilledValue();
+
+        if (textValue != null) {
+            mPresentationStatsEventLogger.onFieldTextUpdated(viewState, value);
+        }
+
         if (filledValue != null) {
             if (filledValue.equals(value)) {
                 // When the update is caused by autofilling the view, just update the
@@ -4821,9 +4824,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 currentView.maybeCallOnFillReady(flags);
             }
         }
-        if (textValue != null) {
-            mPresentationStatsEventLogger.onFieldTextUpdated(viewState, textValue.length());
-        }
 
         if (viewState.id.equals(this.mCurrentViewId)
                 && (viewState.getState() & ViewState.STATE_INLINE_SHOWN) != 0) {
@@ -4888,7 +4888,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 mSaveEventLogger.logAndEndEvent();
                 mPresentationStatsEventLogger.maybeSetNoPresentationEventReason(
                     NOT_SHOWN_REASON_SESSION_COMMITTED_PREMATURELY);
-                mPresentationStatsEventLogger.logAndEndEvent();
+                mPresentationStatsEventLogger.logAndEndEvent("on fill ready");
                 return;
             }
         }
@@ -4920,7 +4920,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 synchronized (mLock) {
                     final ViewState currentView = mViewStates.get(mCurrentViewId);
                     currentView.setState(ViewState.STATE_FILL_DIALOG_SHOWN);
-                    mPresentationStatsEventLogger.maybeSetDisplayPresentationType(UI_TYPE_DIALOG);
                 }
                 // Just show fill dialog once, so disabled after shown.
                 // Note: Cannot disable before requestShowFillDialog() because the method
@@ -6086,6 +6085,11 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
     private void startNewEventForPresentationStatsEventLogger() {
         synchronized (mLock) {
             mPresentationStatsEventLogger.startNewEvent();
+            // Set the default reason for now if the user doesn't trigger any focus event
+            // on the autofillable view. This can be changed downstream when more
+            // information is available or session is committed.
+            mPresentationStatsEventLogger.maybeSetNoPresentationEventReason(
+                    NOT_SHOWN_REASON_NO_FOCUS);
             mPresentationStatsEventLogger.maybeSetDetectionPreference(
                     getDetectionPreferenceForLogging());
             mPresentationStatsEventLogger.maybeSetAutofillServiceUid(getAutofillServiceUid());
@@ -6724,7 +6728,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
             SystemClock.elapsedRealtime() - mStartTime);
         mFillRequestEventLogger.logAndEndEvent();
         mFillResponseEventLogger.logAndEndEvent();
-        mPresentationStatsEventLogger.logAndEndEvent();
+        mPresentationStatsEventLogger.logAndEndEvent("log all events");
         mSaveEventLogger.logAndEndEvent();
         mSessionCommittedEventLogger.logAndEndEvent();
     }
diff --git a/services/autofill/java/com/android/server/autofill/ui/AutoFillUI.java b/services/autofill/java/com/android/server/autofill/ui/AutoFillUI.java
index a10039f9bf6c..2446a6dfee89 100644
--- a/services/autofill/java/com/android/server/autofill/ui/AutoFillUI.java
+++ b/services/autofill/java/com/android/server/autofill/ui/AutoFillUI.java
@@ -461,9 +461,9 @@ public final class AutoFillUI {
                         }
 
                         @Override
-                        public void onShown() {
+                        public void onShown(int datasetsShown) {
                             if (mCallback != null) {
-                                mCallback.onShown(UI_TYPE_DIALOG, response.getDatasets().size());
+                                mCallback.onShown(UI_TYPE_DIALOG, datasetsShown);
                             }
                         }
 
diff --git a/services/autofill/java/com/android/server/autofill/ui/DialogFillUi.java b/services/autofill/java/com/android/server/autofill/ui/DialogFillUi.java
index 5a71b895a57c..c7b6be60dd3e 100644
--- a/services/autofill/java/com/android/server/autofill/ui/DialogFillUi.java
+++ b/services/autofill/java/com/android/server/autofill/ui/DialogFillUi.java
@@ -85,7 +85,7 @@ final class DialogFillUi {
         void onDatasetPicked(@NonNull Dataset dataset);
         void onDismissed();
         void onCanceled();
-        void onShown();
+        void onShown(int datasetsShown);
         void startIntentSender(IntentSender intentSender);
     }
 
@@ -155,7 +155,8 @@ final class DialogFillUi {
         mDialog.setContentView(decor);
         setDialogParamsAsBottomSheet();
         mDialog.setOnCancelListener((d) -> mCallback.onCanceled());
-        mDialog.setOnShowListener((d) -> mCallback.onShown());
+        int datasetsShown = (mAdapter != null) ? mAdapter.getCount() : 0;
+        mDialog.setOnShowListener((d) -> mCallback.onShown(datasetsShown));
         show();
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/autofill/PresentationEventLoggerTest.java b/services/tests/servicestests/src/com/android/server/autofill/PresentationEventLoggerTest.java
new file mode 100644
index 000000000000..75258f0aa7e0
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/autofill/PresentationEventLoggerTest.java
@@ -0,0 +1,132 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.android.server.autofill;
+
+
+import static com.google.common.truth.Truth.assertThat;
+
+import android.view.autofill.AutofillId;
+import android.view.autofill.AutofillValue;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class PresentationEventLoggerTest {
+
+    @Test
+    public void testViewEntered() {
+        PresentationStatsEventLogger pEventLogger =
+                PresentationStatsEventLogger.createPresentationLog(1, 1, 1);
+
+        AutofillId id = new AutofillId(13);
+        AutofillValue initialValue = AutofillValue.forText("hello");
+        AutofillValue lastValue = AutofillValue.forText("hello world");
+        ViewState vState = new ViewState(id, null, 0, false);
+
+        pEventLogger.startNewEvent();
+        pEventLogger.maybeSetFocusedId(id);
+        pEventLogger.onFieldTextUpdated(vState, initialValue);
+        pEventLogger.onFieldTextUpdated(vState, lastValue);
+
+        PresentationStatsEventLogger.PresentationStatsEventInternal event =
+                pEventLogger.getInternalEvent().get();
+        assertThat(event).isNotNull();
+        assertThat(event.mFieldFirstLength).isEqualTo(initialValue.getTextValue().length());
+        assertThat(event.mFieldLastLength).isEqualTo(lastValue.getTextValue().length());
+        assertThat(event.mFieldModifiedFirstTimestampMs).isNotEqualTo(-1);
+        assertThat(event.mFieldModifiedLastTimestampMs).isNotEqualTo(-1);
+    }
+
+    @Test
+    public void testViewAutofilled() {
+        PresentationStatsEventLogger pEventLogger =
+                PresentationStatsEventLogger.createPresentationLog(1, 1, 1);
+
+        String newTextValue = "hello";
+        AutofillValue value = AutofillValue.forText(newTextValue);
+        AutofillId id = new AutofillId(13);
+        ViewState vState = new ViewState(id, null, ViewState.STATE_AUTOFILLED, false);
+
+        pEventLogger.startNewEvent();
+        pEventLogger.maybeSetFocusedId(id);
+        pEventLogger.onFieldTextUpdated(vState, value);
+
+        PresentationStatsEventLogger.PresentationStatsEventInternal event =
+                pEventLogger.getInternalEvent().get();
+        assertThat(event).isNotNull();
+        assertThat(event.mFieldFirstLength).isEqualTo(newTextValue.length());
+        assertThat(event.mFieldLastLength).isEqualTo(newTextValue.length());
+        assertThat(event.mAutofilledTimestampMs).isNotEqualTo(-1);
+        assertThat(event.mFieldModifiedFirstTimestampMs).isEqualTo(-1);
+        assertThat(event.mFieldModifiedLastTimestampMs).isEqualTo(-1);
+    }
+
+    @Test
+    public void testModifiedOnDifferentView() {
+        PresentationStatsEventLogger pEventLogger =
+                PresentationStatsEventLogger.createPresentationLog(1, 1, 1);
+
+        String newTextValue = "hello";
+        AutofillValue value = AutofillValue.forText(newTextValue);
+        AutofillId id = new AutofillId(13);
+        ViewState vState = new ViewState(id, null, ViewState.STATE_AUTOFILLED, false);
+
+        pEventLogger.startNewEvent();
+        pEventLogger.onFieldTextUpdated(vState, value);
+
+        PresentationStatsEventLogger.PresentationStatsEventInternal event =
+                pEventLogger.getInternalEvent().get();
+        assertThat(event).isNotNull();
+        assertThat(event.mFieldFirstLength).isEqualTo(-1);
+        assertThat(event.mFieldLastLength).isEqualTo(-1);
+        assertThat(event.mFieldModifiedFirstTimestampMs).isEqualTo(-1);
+        assertThat(event.mFieldModifiedLastTimestampMs).isEqualTo(-1);
+        assertThat(event.mAutofilledTimestampMs).isEqualTo(-1);
+    }
+
+    @Test
+    public void testSetCountShown() {
+        PresentationStatsEventLogger pEventLogger =
+                PresentationStatsEventLogger.createPresentationLog(1, 1, 1);
+
+        pEventLogger.startNewEvent();
+        pEventLogger.logWhenDatasetShown(7);
+
+        PresentationStatsEventLogger.PresentationStatsEventInternal event =
+                pEventLogger.getInternalEvent().get();
+        assertThat(event).isNotNull();
+        assertThat(event.mCountShown).isEqualTo(7);
+        assertThat(event.mNoPresentationReason)
+                .isEqualTo(PresentationStatsEventLogger.NOT_SHOWN_REASON_ANY_SHOWN);
+    }
+
+    @Test
+    public void testFillDialogShownThenInline() {
+        PresentationStatsEventLogger pEventLogger =
+                PresentationStatsEventLogger.createPresentationLog(1, 1, 1);
+
+        pEventLogger.startNewEvent();
+        pEventLogger.maybeSetDisplayPresentationType(3);
+        pEventLogger.maybeSetDisplayPresentationType(2);
+
+        PresentationStatsEventLogger.PresentationStatsEventInternal event =
+                pEventLogger.getInternalEvent().get();
+        assertThat(event).isNotNull();
+        assertThat(event.mDisplayPresentationType).isEqualTo(3);
+    }
+}