Merge "[PCC]: Fixes for PCC Create session only once. Previously, multiple sessions could be created, and can result in multiple requsets to PCC. Add local logs for debugging. Fix to show datasets from PCC. Seems like it stopped working from a previous change. Upcoming PccFieldClassificationTest should prevent such regressions." into udc-dev am: bdeb39fec4

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/22974417

Change-Id: I4709fa56b6de6c19894c794a190b234bf3ce9f29
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

3 files changed, 121 insertions, 41 deletions

diff --git a/core/java/android/view/autofill/AutofillManager.java b/core/java/android/view/autofill/AutofillManager.java
index e39b3a182b28..6ff4b74c68a6 100644
--- a/core/java/android/view/autofill/AutofillManager.java
+++ b/core/java/android/view/autofill/AutofillManager.java
@@ -1477,14 +1477,22 @@ public final class AutofillManager {
         // to PCC classification service.
         if (AutofillFeatureFlags.isAutofillPccClassificationEnabled()) {
             synchronized (mLock) {
-                final boolean clientAdded = tryAddServiceClientIfNeededLocked();
-                if (clientAdded){
-                    startSessionLocked(/* id= */ AutofillId.NO_AUTOFILL_ID,
-                        /* bounds= */ null, /* value= */ null, /* flags= */ FLAG_PCC_DETECTION);
-                } else {
-                    if (sVerbose) {
-                        Log.v(TAG, "not starting session: no service client");
+                // If session has already been created, that'd mean we already have issued the
+                // detection request previously. It is possible in cases like autofocus that this
+                // method isn't invoked, so the server should still handle such cases where fill
+                // request comes in but PCC Detection hasn't been triggered. There is no benefit to
+                // trigger PCC Detection separately in those cases.
+                if (!isActiveLocked()) {
+                    final boolean clientAdded = tryAddServiceClientIfNeededLocked();
+                    if (clientAdded) {
+                        startSessionLocked(/* id= */ AutofillId.NO_AUTOFILL_ID, /* bounds= */ null,
+                                /* value= */ null, /* flags= */ FLAG_PCC_DETECTION);
+                    } else {
+                        if (sVerbose) {
+                            Log.v(TAG, "not starting session: no service client");
+                        }
                     }
+
                 }
             }
         }
diff --git a/services/autofill/java/com/android/server/autofill/RemoteFieldClassificationService.java b/services/autofill/java/com/android/server/autofill/RemoteFieldClassificationService.java
index feae56e89784..b8bac61b346b 100644
--- a/services/autofill/java/com/android/server/autofill/RemoteFieldClassificationService.java
+++ b/services/autofill/java/com/android/server/autofill/RemoteFieldClassificationService.java
@@ -157,6 +157,8 @@ final class RemoteFieldClassificationService
                                         if (sDebug) {
                                             Log.d(TAG, "onSuccess Response: " + response);
                                         }
+                                        fieldClassificationServiceCallbacks
+                                                .onClassificationRequestSuccess(response);
                                     }
 
                                     @Override
@@ -165,6 +167,8 @@ final class RemoteFieldClassificationService
                                         if (sDebug) {
                                             Log.d(TAG, "onFailure");
                                         }
+                                        fieldClassificationServiceCallbacks
+                                                .onClassificationRequestFailure(0, null);
                                     }
 
                                     @Override
diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java
index 3736262e0673..9e46d739be0f 100644
--- a/services/autofill/java/com/android/server/autofill/Session.java
+++ b/services/autofill/java/com/android/server/autofill/Session.java
@@ -204,6 +204,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         RemoteFieldClassificationService.FieldClassificationServiceCallbacks {
     private static final String TAG = "AutofillSession";
 
+    // This should never be true in production. This is only for local debugging.
+    // Otherwise it will spam logcat.
+    private static final boolean DBG = false;
+
     private static final String ACTION_DELAYED_FILL =
             "android.service.autofill.action.DELAYED_FILL";
     private static final String EXTRA_REQUEST_ID = "android.service.autofill.extra.REQUEST_ID";
@@ -1240,6 +1244,8 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
 
     @GuardedBy("mLock")
     private void requestAssistStructureForPccLocked(int flags) {
+        if (!mClassificationState.shouldTriggerRequest()) return;
+        mClassificationState.updatePendingRequest();
         // Get request id
         int requestId;
         // TODO(b/158623971): Update this to prevent possible overflow
@@ -1571,12 +1577,18 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         // TODO(b/266379948): Ideally wait for PCC request to finish for a while more
         // (say 100ms) before proceeding further on.
 
+        if (DBG) {
+            Slog.d(TAG, "DBG: Initial response: " + response);
+        }
         synchronized (mLock) {
             response = getEffectiveFillResponse(response);
             if (isEmptyResponse(response)) {
                 // Treat it as a null response.
                 processNullResponseLocked(requestId, requestFlags);
             }
+            if (DBG) {
+                Slog.d(TAG, "DBG: Processed response: " + response);
+            }
             processResponseLocked(response, null, requestFlags);
         }
     }
@@ -1601,12 +1613,25 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         DatasetComputationContainer autofillProviderContainer = new DatasetComputationContainer();
         computeDatasetsForProviderAndUpdateContainer(response, autofillProviderContainer);
 
+        if (DBG) {
+            Slog.d(TAG, "DBG: computeDatasetsForProviderAndUpdateContainer: "
+                    + autofillProviderContainer);
+        }
         if (!mService.getMaster().isPccClassificationEnabled())  {
+            if (sVerbose) {
+                Slog.v(TAG, "PCC classification is disabled");
+            }
             return createShallowCopy(response, autofillProviderContainer);
         }
         synchronized (mLock) {
             if (mClassificationState.mState != ClassificationState.STATE_RESPONSE
                     || mClassificationState.mLastFieldClassificationResponse == null) {
+                if (sVerbose) {
+                    Slog.v(TAG, "PCC classification no last response:"
+                            + (mClassificationState.mLastFieldClassificationResponse == null)
+                            +   " ,ineligible state="
+                            + (mClassificationState.mState != ClassificationState.STATE_RESPONSE));
+                }
                 return createShallowCopy(response, autofillProviderContainer);
             }
             if (!mClassificationState.processResponse()) return response;
@@ -1614,11 +1639,22 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         boolean preferAutofillProvider = mService.getMaster().preferProviderOverPcc();
         boolean shouldUseFallback = mService.getMaster().shouldUsePccFallback();
         if (preferAutofillProvider && !shouldUseFallback) {
+            if (sVerbose) {
+                Slog.v(TAG, "preferAutofillProvider but no fallback");
+            }
             return createShallowCopy(response, autofillProviderContainer);
         }
 
+        if (DBG) {
+            synchronized (mLock) {
+                Slog.d(TAG, "DBG: ClassificationState: " + mClassificationState);
+            }
+        }
         DatasetComputationContainer detectionPccContainer = new DatasetComputationContainer();
         computeDatasetsForPccAndUpdateContainer(response, detectionPccContainer);
+        if (DBG) {
+            Slog.d(TAG, "DBG: computeDatasetsForPccAndUpdateContainer: " + detectionPccContainer);
+        }
 
         DatasetComputationContainer resultContainer;
         if (preferAutofillProvider) {
@@ -1692,6 +1728,20 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         // FillResponse.
         Set<Dataset> mDatasets = new LinkedHashSet<>();
         ArrayMap<AutofillId, Set<Dataset>> mAutofillIdToDatasetMap = new ArrayMap<>();
+
+        public String toString() {
+            final StringBuilder builder = new StringBuilder("DatasetComputationContainer[");
+            if (mAutofillIds != null) {
+                builder.append(", autofillIds=").append(mAutofillIds);
+            }
+            if (mDatasets != null) {
+                builder.append(", mDatasets=").append(mDatasets);
+            }
+            if (mAutofillIdToDatasetMap != null) {
+                builder.append(", mAutofillIdToDatasetMap=").append(mAutofillIdToDatasetMap);
+            }
+            return builder.append(']').toString();
+        }
     }
 
     // Adds fallback datasets to the first container.
@@ -1843,7 +1893,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 Dataset dataset = datasets.get(i);
                 if (dataset.getAutofillDatatypes() == null
                         || dataset.getAutofillDatatypes().isEmpty()) continue;
-                if (dataset.getFieldIds() != null && dataset.getFieldIds().size() > 0) continue;
 
                 ArrayList<AutofillId> fieldIds = new ArrayList<>();
                 ArrayList<AutofillValue> fieldValues = new ArrayList<>();
@@ -1852,9 +1901,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 ArrayList<InlinePresentation> fieldInlinePresentations = new ArrayList<>();
                 ArrayList<InlinePresentation> fieldInlineTooltipPresentations = new ArrayList<>();
                 ArrayList<Dataset.DatasetFieldFilter> fieldFilters = new ArrayList<>();
+                Set<AutofillId> datasetAutofillIds = new ArraySet<>();
 
                 for (int j = 0; j < dataset.getAutofillDatatypes().size(); j++) {
-                    if (dataset.getAutofillDatatypes().get(0) == null) continue;
+                    if (dataset.getAutofillDatatypes().get(j) == null) continue;
                     String hint = dataset.getAutofillDatatypes().get(j);
 
                     if (hintsToAutofillIdMap.containsKey(hint)) {
@@ -1863,6 +1913,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
 
                         for (AutofillId autofillId : tempIds) {
                             eligibleAutofillIds.add(autofillId);
+                            datasetAutofillIds.add(autofillId);
                             // For each of the field, copy over values.
                             fieldIds.add(autofillId);
                             fieldValues.add(dataset.getFieldValues().get(j));
@@ -1876,37 +1927,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                                     dataset.getFieldInlineTooltipPresentation(j));
                             fieldFilters.add(dataset.getFilter(j));
                         }
-
-                        Dataset newDataset =
-                                new Dataset(
-                                        fieldIds,
-                                        fieldValues,
-                                        fieldPresentations,
-                                        fieldDialogPresentations,
-                                        fieldInlinePresentations,
-                                        fieldInlineTooltipPresentations,
-                                        fieldFilters,
-                                        new ArrayList<>(),
-                                        dataset.getFieldContent(),
-                                        null,
-                                        null,
-                                        null,
-                                        null,
-                                        dataset.getId(),
-                                        dataset.getAuthentication());
-                        eligibleDatasets.add(newDataset);
-
-                        // Associate this dataset with all the ids that are represented with it.
-                        Set<Dataset> newDatasets;
-                        for (AutofillId autofillId : tempIds) {
-                            if (map.containsKey(autofillId)) {
-                                newDatasets = map.get(autofillId);
-                            } else {
-                                newDatasets = new ArraySet<>();
-                            }
-                            newDatasets.add(newDataset);
-                            map.put(autofillId, newDatasets);
-                        }
                     }
                     // TODO(b/266379948):  handle the case:
                     // groupHintsToAutofillIdMap.containsKey(hint))
@@ -1914,6 +1934,34 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                     // TODO(b/266379948):  also handle the case where there could be more types in
                     // the dataset, provided by the provider, however, they aren't applicable.
                 }
+                Dataset newDataset =
+                        new Dataset(
+                                fieldIds,
+                                fieldValues,
+                                fieldPresentations,
+                                fieldDialogPresentations,
+                                fieldInlinePresentations,
+                                fieldInlineTooltipPresentations,
+                                fieldFilters,
+                                new ArrayList<>(),
+                                dataset.getFieldContent(),
+                                null,
+                                null,
+                                null,
+                                null,
+                                dataset.getId(),
+                                dataset.getAuthentication());
+                eligibleDatasets.add(newDataset);
+                Set<Dataset> newDatasets;
+                for (AutofillId autofillId : datasetAutofillIds) {
+                    if (map.containsKey(autofillId)) {
+                        newDatasets = map.get(autofillId);
+                    } else {
+                        newDatasets = new ArraySet<>();
+                    }
+                    newDatasets.add(newDataset);
+                    map.put(autofillId, newDatasets);
+                }
             }
             container.mAutofillIds = eligibleAutofillIds;
             container.mDatasets = eligibleDatasets;
@@ -5319,6 +5367,26 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
             mState = STATE_PENDING_REQUEST;
             mPendingFieldClassificationRequest = null;
         }
+
+        @GuardedBy("mLock")
+        private boolean shouldTriggerRequest() {
+            return mState == STATE_INITIAL || mState == STATE_INVALIDATED;
+        }
+
+        @GuardedBy("mLock")
+        @Override
+        public String toString() {
+            return "ClassificationState: ["
+                    + "state=" + stateToString()
+                    + ", mPendingFieldClassificationRequest=" + mPendingFieldClassificationRequest
+                    + ", mLastFieldClassificationResponse=" + mLastFieldClassificationResponse
+                    + ", mClassificationHintsMap=" + mClassificationHintsMap
+                    + ", mClassificationGroupHintsMap=" + mClassificationGroupHintsMap
+                    + ", mHintsToAutofillIdMap=" + mHintsToAutofillIdMap
+                    + ", mGroupHintsToAutofillIdMap=" + mGroupHintsToAutofillIdMap
+                    + "]";
+        }
+
     }
 
     @Override
@@ -5843,7 +5911,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         return serviceInfo == null ? Process.INVALID_UID : serviceInfo.applicationInfo.uid;
     }
 
-    // DetectionServiceCallbacks
+    // FieldClassificationServiceCallbacks
     public void onClassificationRequestSuccess(@Nullable FieldClassificationResponse response) {
         mClassificationState.updateResponseReceived(response);
     }