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 am: bd7b3c717d

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

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

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 session has already been created, that'd mean we already have issued the
-                if (clientAdded){
+                // detection request previously. It is possible in cases like autofocus that this
-                    startSessionLocked(/* id= */ AutofillId.NO_AUTOFILL_ID,
+                // method isn't invoked, so the server should still handle such cases where fill
-                        /* bounds= */ null, /* value= */ null, /* flags= */ FLAG_PCC_DETECTION);
+                // request comes in but PCC Detection hasn't been triggered. There is no benefit to
-                } else {
+                // trigger PCC Detection separately in those cases.
-                    if (sVerbose) {
+                if (!isActiveLocked()) {
-                        Log.v(TAG, "not starting session: no service client");
+                    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");
+                        }
                     }
+
                 }
             }
         }

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

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);
     }