From d013bcea9713d178627cc1d3e8a0f291ccbcd293 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Tue, 20 Jun 2017 17:15:45 -0700 Subject: [PATCH] Don't add FillEventHistory events to the wrong session. The AutofillSession.getFillEventHistory() method returns only the event history for the last onFillRequest(). In the scenario where the user switches activities and the server has multiple sessions open, only the events for the last session should be recorded. Test: existing CtsAutoFillServiceTestCases pass Test: LoginActivityTest.checkFillSelectionFromPreviousSessionIsDiscarded Fixes: 62802026 Change-Id: I447ed77c2167095867b35d616b5cf2ae43aa28db --- .../service/autofill/FillEventHistory.java | 15 +++++- .../autofill/AutofillManagerServiceImpl.java | 53 +++++++++++-------- .../com/android/server/autofill/Session.java | 10 ++-- 3 files changed, 48 insertions(+), 30 deletions(-) diff --git a/core/java/android/service/autofill/FillEventHistory.java b/core/java/android/service/autofill/FillEventHistory.java index 58160ccd062d4..f7dc1c58ade1b 100644 --- a/core/java/android/service/autofill/FillEventHistory.java +++ b/core/java/android/service/autofill/FillEventHistory.java @@ -54,6 +54,11 @@ public final class FillEventHistory implements Parcelable { */ private final int mServiceUid; + /** + * Not in parcel. The ID of the autofill session that created the {@link FillResponse}. + */ + private final int mSessionId; + @Nullable private final Bundle mClientState; @Nullable List mEvents; @@ -68,6 +73,11 @@ public final class FillEventHistory implements Parcelable { return mServiceUid; } + /** @hide */ + public int getSessionId() { + return mSessionId; + } + /** * Returns the client state set in the previous {@link FillResponse}. * @@ -102,9 +112,10 @@ public final class FillEventHistory implements Parcelable { /** * @hide */ - public FillEventHistory(int serviceUid, @Nullable Bundle clientState) { + public FillEventHistory(int serviceUid, int sessionId, @Nullable Bundle clientState) { mClientState = clientState; mServiceUid = serviceUid; + mSessionId = sessionId; } @Override @@ -205,7 +216,7 @@ public final class FillEventHistory implements Parcelable { new Parcelable.Creator() { @Override public FillEventHistory createFromParcel(Parcel parcel) { - FillEventHistory selection = new FillEventHistory(0, parcel.readBundle()); + FillEventHistory selection = new FillEventHistory(0, 0, parcel.readBundle()); int numEvents = parcel.readInt(); for (int i = 0; i < numEvents; i++) { diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java index 62cd0cf6d6d51..1a02e8d8eb97b 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java @@ -489,9 +489,9 @@ final class AutofillManagerServiceImpl { * Initializes the last fill selection after an autofill service returned a new * {@link FillResponse}. */ - void setLastResponse(int serviceUid, @NonNull FillResponse response) { + void setLastResponse(int serviceUid, int sessionId, @NonNull FillResponse response) { synchronized (mLock) { - mEventHistory = new FillEventHistory(serviceUid, response.getClientState()); + mEventHistory = new FillEventHistory(serviceUid, sessionId, response.getClientState()); } } @@ -504,56 +504,63 @@ final class AutofillManagerServiceImpl { } } + private boolean isValidEventLocked(String method, int sessionId) { + if (mEventHistory == null) { + Slog.w(TAG, method + ": not logging event because history is null"); + return false; + } + if (sessionId != mEventHistory.getSessionId()) { + if (sDebug) { + Slog.d(TAG, method + ": not logging event for session " + sessionId + + " because tracked session is " + mEventHistory.getSessionId()); + } + return false; + } + return true; + } + /** * Updates the last fill selection when an authentication was selected. */ - void setAuthenticationSelected() { + void setAuthenticationSelected(int sessionId) { synchronized (mLock) { - if (mEventHistory == null) { - Slog.w(TAG, "setAuthenticationSelected(): ignored when history is null"); - return; + if (isValidEventLocked("setAuthenticationSelected()", sessionId)) { + mEventHistory.addEvent(new Event(Event.TYPE_AUTHENTICATION_SELECTED, null)); } - mEventHistory.addEvent(new Event(Event.TYPE_AUTHENTICATION_SELECTED, null)); } } /** * Updates the last fill selection when an dataset authentication was selected. */ - void setDatasetAuthenticationSelected(@Nullable String selectedDataset) { + void setDatasetAuthenticationSelected(@Nullable String selectedDataset, int sessionId) { synchronized (mLock) { - if (mEventHistory == null) { - Slog.w(TAG, "setDatasetAuthenticationSelected(): ignored when history is null"); - return; + if (isValidEventLocked("setDatasetAuthenticationSelected()", sessionId)) { + mEventHistory.addEvent( + new Event(Event.TYPE_DATASET_AUTHENTICATION_SELECTED, selectedDataset)); } - mEventHistory.addEvent( - new Event(Event.TYPE_DATASET_AUTHENTICATION_SELECTED, selectedDataset)); } } /** * Updates the last fill selection when an save Ui is shown. */ - void setSaveShown() { + void setSaveShown(int sessionId) { synchronized (mLock) { - if (mEventHistory == null) { - Slog.w(TAG, "setSaveShown(): ignored when history is null"); - return; + if (isValidEventLocked("setSaveShown()", sessionId)) { + mEventHistory.addEvent(new Event(Event.TYPE_SAVE_SHOWN, null)); } - mEventHistory.addEvent(new Event(Event.TYPE_SAVE_SHOWN, null)); } } /** * Updates the last fill response when a dataset was selected. */ - void setDatasetSelected(@Nullable String selectedDataset) { + void setDatasetSelected(@Nullable String selectedDataset, int sessionId) { synchronized (mLock) { - if (mEventHistory == null) { - Slog.w(TAG, "setDatasetSelected(): ignored when history is null"); - return; + if (isValidEventLocked("setDatasetSelected()", sessionId)) { + mEventHistory.addEvent(new Event(Event.TYPE_DATASET_SELECTED, selectedDataset)); } - mEventHistory.addEvent(new Event(Event.TYPE_DATASET_SELECTED, selectedDataset)); } } diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index 4de55dcff8522..72ad752caf199 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -414,7 +414,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState return; } - mService.setLastResponse(serviceUid, response); + mService.setLastResponse(serviceUid, id, response); if ((response.getDatasets() == null || response.getDatasets().isEmpty()) && response.getAuthentication() == null) { @@ -544,7 +544,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState getFillContextByRequestIdLocked(requestId).getStructure(), extras); } - mService.setAuthenticationSelected(); + mService.setAuthenticationSelected(id); final int authenticationId = AutofillManager.makeAuthenticationId(requestId, datasetIndex); mHandlerCaller.getHandler().post(() -> startAuthentication(authenticationId, @@ -833,7 +833,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } if (atLeastOneChanged) { if (sDebug) Slog.d(TAG, "at least one field changed - showing save UI"); - mService.setSaveShown(); + mService.setSaveShown(id); getUiForShowing().showSaveUi(mService.getServiceLabel(), saveInfo, mPackageName, this); @@ -1364,14 +1364,14 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } // Autofill it directly... if (dataset.getAuthentication() == null) { - mService.setDatasetSelected(dataset.getId()); + mService.setDatasetSelected(dataset.getId(), id); autoFillApp(dataset); return; } // ...or handle authentication. - mService.setDatasetAuthenticationSelected(dataset.getId()); + mService.setDatasetAuthenticationSelected(dataset.getId(), id); setViewStatesLocked(null, dataset, ViewState.STATE_WAITING_DATASET_AUTH, false); final Intent fillInIntent = createAuthFillInIntent( getFillContextByRequestIdLocked(requestId).getStructure(), mClientState);