Handle autofill auth scenarios where the FillContext cannot be retrieved:

Test: manually modified code to force a null FillContext and verified the
      session is properly cleared (even in the client side)
Test: cts-tradefed run commandAndExit cts-dev -m CtsAutoFillServiceTestCases

Fixes: 65374274

Change-Id: Ica482a48b0be34b89e36f9a34078fdcd48134537

diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java
index 2b7a671..1bec9ee 100644 (file)
--- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java
+++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java
@@ -631,7 +631,7 @@ final class AutofillManagerServiceImpl {
 
     void destroySessionsLocked() {
         if (mSessions.size() == 0) {
-            mUi.destroyAll(null, null);
+            mUi.destroyAll(null, null, false);
             return;
         }
         while (mSessions.size() > 0) {

diff --git a/services/autofill/java/com/android/server/autofill/RemoteFillService.java b/services/autofill/java/com/android/server/autofill/RemoteFillService.java
index f315148..af55807 100644 (file)
--- a/services/autofill/java/com/android/server/autofill/RemoteFillService.java
+++ b/services/autofill/java/com/android/server/autofill/RemoteFillService.java
@@ -578,9 +578,8 @@ final class RemoteFillService implements DeathRecipient {
         public void run() {
             synchronized (mLock) {
                 if (isCancelledLocked()) {
-                    // TODO(b/653742740): we should probably return here, but for now we're justing
-                    // logging to confirm this is the problem if it happens again.
-                    Slog.e(LOG_TAG, "run() called after canceled: " + mRequest);
+                    if (sDebug) Slog.d(LOG_TAG, "run() called after canceled: " + mRequest);
+                    return;
                 }
             }
             final RemoteFillService remoteService = getService();

diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java
index dd0c874..1386e8e 100644 (file)
--- a/services/autofill/java/com/android/server/autofill/Session.java
+++ b/services/autofill/java/com/android/server/autofill/Session.java
@@ -577,6 +577,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
     // FillServiceCallbacks
     @Override
     public void authenticate(int requestId, int datasetIndex, IntentSender intent, Bundle extras) {
+        if (sDebug) {
+            Slog.d(TAG, "authenticate(): requestId=" + requestId + "; datasetIdx=" + datasetIndex
+                    + "; intentSender=" + intent);
+        }
         final Intent fillInIntent;
         synchronized (mLock) {
             if (mDestroyed) {
@@ -585,6 +589,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
                 return;
             }
             fillInIntent = createAuthFillInIntentLocked(requestId, extras);
+            if (fillInIntent == null) {
+                forceRemoveSelfLocked();
+                return;
+            }
         }
         mService.setAuthenticationSelected(id);
 
@@ -1537,6 +1545,10 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
     }
 
     void autoFill(int requestId, int datasetIndex, Dataset dataset, boolean generateEvent) {
+        if (sDebug) {
+            Slog.d(TAG, "autoFill(): requestId=" + requestId  + "; datasetIdx=" + datasetIndex
+                    + "; dataset=" + dataset);
+        }
         synchronized (mLock) {
             if (mDestroyed) {
                 Slog.w(TAG, "Call to Session#autoFill() rejected - session: "
@@ -1557,10 +1569,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
             mService.logDatasetAuthenticationSelected(dataset.getId(), id);
             setViewStatesLocked(null, dataset, ViewState.STATE_WAITING_DATASET_AUTH, false);
             final Intent fillInIntent = createAuthFillInIntentLocked(requestId, mClientState);
-
+            if (fillInIntent == null) {
+                forceRemoveSelfLocked();
+                return;
+            }
             final int authenticationId = AutofillManager.makeAuthenticationId(requestId,
                     datasetIndex);
             startAuthentication(authenticationId, dataset.getAuthentication(), fillInIntent);
+
         }
     }
 
@@ -1570,14 +1586,16 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         }
     }
 
+    // TODO: this should never be null, but we got at least one occurrence, probably due to a race.
+    @Nullable
     private Intent createAuthFillInIntentLocked(int requestId, Bundle extras) {
         final Intent fillInIntent = new Intent();
 
         final FillContext context = getFillContextByRequestIdLocked(requestId);
         if (context == null) {
-            // TODO(b/653742740): this will crash system_server. We need to handle it, but we're
-            // keeping it crashing for now so we can diagnose when it happens again
-            Slog.wtf(TAG, "no FillContext for requestId" + requestId + "; mContexts= " + mContexts);
+            Slog.wtf(TAG, "createAuthFillInIntentLocked(): no FillContext. requestId=" + requestId
+                    + "; mContexts= " + mContexts);
+            return null;
         }
         fillInIntent.putExtra(AutofillManager.EXTRA_ASSIST_STRUCTURE, context.getStructure());
         fillInIntent.putExtra(AutofillManager.EXTRA_CLIENT_STATE, extras);
@@ -1717,7 +1735,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
         if (mDestroyed) {
             return null;
         }
-        mUi.destroyAll(mPendingSaveUi, this);
+        mUi.destroyAll(mPendingSaveUi, this, true);
         mUi.clearCallback(this);
         mDestroyed = true;
         mMetricsLogger.action(MetricsEvent.AUTOFILL_SESSION_FINISHED, mPackageName);
@@ -1733,7 +1751,16 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState
 
         mPendingSaveUi = null;
         removeSelfLocked();
-        mUi.destroyAll(mPendingSaveUi, this);
+
+        mHandlerCaller.getHandler().post(() -> {
+            try {
+                mClient.setState(mService.isEnabled(), true, false);
+            } catch (RemoteException e) {
+                Slog.w(TAG, "error updating client state: " + e);
+            }
+        });
+
+        mUi.destroyAll(mPendingSaveUi, this, false);
     }
 
     /**

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 cac2bff..688894b 100644 (file)
--- a/services/autofill/java/com/android/server/autofill/ui/AutoFillUI.java
+++ b/services/autofill/java/com/android/server/autofill/ui/AutoFillUI.java
@@ -271,7 +271,7 @@ public final class AutoFillUI {
                     if (mCallback != null) {
                         mCallback.save();
                     }
-                    destroySaveUiUiThread(pendingSaveUi);
+                    destroySaveUiUiThread(pendingSaveUi, true);
                 }
 
                 @Override
@@ -289,7 +289,7 @@ public final class AutoFillUI {
                     if (mCallback != null) {
                         mCallback.cancelSave();
                     }
-                    destroySaveUiUiThread(pendingSaveUi);
+                    destroySaveUiUiThread(pendingSaveUi, true);
                 }
 
                 @Override
@@ -331,8 +331,8 @@ public final class AutoFillUI {
      * Destroy all UI affordances.
      */
     public void destroyAll(@Nullable PendingUi pendingSaveUi,
-            @Nullable AutoFillUiCallback callback) {
-        mHandler.post(() -> destroyAllUiThread(pendingSaveUi, callback));
+            @Nullable AutoFillUiCallback callback, boolean notifyClient) {
+        mHandler.post(() -> destroyAllUiThread(pendingSaveUi, callback, notifyClient));
     }
 
     public void dump(PrintWriter pw) {
@@ -375,7 +375,7 @@ public final class AutoFillUI {
     }
 
     @android.annotation.UiThread
-    private void destroySaveUiUiThread(@Nullable PendingUi pendingSaveUi) {
+    private void destroySaveUiUiThread(@Nullable PendingUi pendingSaveUi, boolean notifyClient) {
         if (mSaveUi == null) {
             // Calling destroySaveUiUiThread() twice is normal - it usually happens when the
             // first call is made after the SaveUI is hidden and the second when the session is
@@ -387,7 +387,7 @@ public final class AutoFillUI {
         if (sDebug) Slog.d(TAG, "destroySaveUiUiThread(): " + pendingSaveUi);
         mSaveUi.destroy();
         mSaveUi = null;
-        if (pendingSaveUi != null) {
+        if (pendingSaveUi != null && notifyClient) {
             try {
                 if (sDebug) Slog.d(TAG, "destroySaveUiUiThread(): notifying client");
                 pendingSaveUi.client.setSaveUiState(pendingSaveUi.id, false);
@@ -399,9 +399,9 @@ public final class AutoFillUI {
 
     @android.annotation.UiThread
     private void destroyAllUiThread(@Nullable PendingUi pendingSaveUi,
-            @Nullable AutoFillUiCallback callback) {
+            @Nullable AutoFillUiCallback callback, boolean notifyClient) {
         hideFillUiUiThread(callback);
-        destroySaveUiUiThread(pendingSaveUi);
+        destroySaveUiUiThread(pendingSaveUi, notifyClient);
     }
 
     @android.annotation.UiThread
@@ -413,7 +413,7 @@ public final class AutoFillUI {
                 Slog.d(TAG, "hideAllUiThread(): "
                         + "destroying Save UI because pending restoration is finished");
             }
-            destroySaveUiUiThread(pendingSaveUi);
+            destroySaveUiUiThread(pendingSaveUi, true);
         }
     }
 }