From b8992fecfa958b54bdbbb6eeb402e49eddae6440 Mon Sep 17 00:00:00 2001 From: Reema Bajwa Date: Thu, 29 Feb 2024 18:12:34 +0000 Subject: Make sure finishSession logs the final status Bug: 315345631 Test: Built locally & deployed on device Change-Id: I28e27ae502a8902025a78ed8006d3594d362f454 --- .../java/com/android/server/autofill/Session.java | 2 -- .../credentials/GetCandidateRequestSession.java | 7 ++-- .../android/server/credentials/RequestSession.java | 40 ++++++++++++---------- .../credentials/metrics/RequestSessionMetric.java | 2 +- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index d779fbf2eabc..551297b253d1 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -4782,7 +4782,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState } if (isCredmanIntegrationActive(response)) { - Slog.d(TAG, "Attempting to add Credential Manager callback to pinned entries"); addCredentialManagerCallback(response); } @@ -5713,7 +5712,6 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState /* isPrimary= */ true); updateFillDialogTriggerIdsLocked(); updateTrackedIdsLocked(); - if (mCurrentViewId == null) { return; } diff --git a/services/credentials/java/com/android/server/credentials/GetCandidateRequestSession.java b/services/credentials/java/com/android/server/credentials/GetCandidateRequestSession.java index 3cb98eb4cd7a..eff53de75ff4 100644 --- a/services/credentials/java/com/android/server/credentials/GetCandidateRequestSession.java +++ b/services/credentials/java/com/android/server/credentials/GetCandidateRequestSession.java @@ -41,6 +41,8 @@ import android.service.credentials.CredentialProviderService; import android.service.credentials.PermissionUtils; import android.util.Slog; +import com.android.server.credentials.metrics.ApiStatus; + import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -180,7 +182,7 @@ public class GetCandidateRequestSession extends RequestSession implements CredentialManagerUi.Credential () -> { Slog.d(TAG, "Cancellation invoked from the client - clearing session"); boolean isUiActive = maybeCancelUi(); - finishSession(!isUiActive); + finishSession(!isUiActive, ApiStatus.CLIENT_CANCELED.getMetricCode()); } ); } @@ -231,7 +231,8 @@ abstract class RequestSession implements CredentialManagerUi.Credential return; } if (isSessionCancelled()) { - finishSession(/*propagateCancellation=*/true); + finishSession(/*propagateCancellation=*/true, + ApiStatus.CLIENT_CANCELED.getMetricCode()); return; } String providerId = selection.getProviderId(); @@ -257,11 +258,12 @@ abstract class RequestSession implements CredentialManagerUi.Credential } } - protected void finishSession(boolean propagateCancellation) { + protected void finishSession(boolean propagateCancellation, int apiStatus) { Slog.i(TAG, "finishing session with propagateCancellation " + propagateCancellation); if (propagateCancellation) { mProviders.values().forEach(ProviderSession::cancelProviderRemoteSession); } + mRequestSessionMetric.logApiCalledAtFinish(apiStatus); mRequestSessionStatus = RequestSessionStatus.COMPLETE; mProviders.clear(); clearRequestSessionLocked(); @@ -326,7 +328,8 @@ abstract class RequestSession implements CredentialManagerUi.Credential mRequestSessionMetric.logCandidatePhaseMetrics(mProviders); if (isSessionCancelled()) { - finishSession(/*propagateCancellation=*/true); + finishSession(/*propagateCancellation=*/true, + ApiStatus.CLIENT_CANCELED.getMetricCode()); return providerDataList; } @@ -353,23 +356,20 @@ abstract class RequestSession implements CredentialManagerUi.Credential return; } if (isSessionCancelled()) { - mRequestSessionMetric.logApiCalledAtFinish( - /*apiStatus=*/ ApiStatus.CLIENT_CANCELED.getMetricCode()); - finishSession(/*propagateCancellation=*/true); + finishSession(/*propagateCancellation=*/true, + ApiStatus.CLIENT_CANCELED.getMetricCode()); return; } try { invokeClientCallbackSuccess(response); - mRequestSessionMetric.logApiCalledAtFinish( - /*apiStatus=*/ ApiStatus.SUCCESS.getMetricCode()); + finishSession(/*propagateCancellation=*/false, + ApiStatus.SUCCESS.getMetricCode()); } catch (RemoteException e) { mRequestSessionMetric.collectFinalPhaseProviderMetricStatus( /*has_exception=*/ true, ProviderStatusForMetrics.FINAL_FAILURE); Slog.e(TAG, "Issue while responding to client with a response : " + e); - mRequestSessionMetric.logApiCalledAtFinish( - /*apiStatus=*/ ApiStatus.FAILURE.getMetricCode()); + finishSession(/*propagateCancellation=*/false, ApiStatus.FAILURE.getMetricCode()); } - finishSession(/*propagateCancellation=*/false); } /** @@ -387,9 +387,7 @@ abstract class RequestSession implements CredentialManagerUi.Credential return; } if (isSessionCancelled()) { - mRequestSessionMetric.logApiCalledAtFinish( - /*apiStatus=*/ ApiStatus.CLIENT_CANCELED.getMetricCode()); - finishSession(/*propagateCancellation=*/true); + finishSession(/*propagateCancellation=*/true, ApiStatus.CLIENT_CANCELED.getMetricCode()); return; } @@ -399,8 +397,14 @@ abstract class RequestSession implements CredentialManagerUi.Credential Slog.e(TAG, "Issue while responding to client with error : " + e); } boolean isUserCanceled = errorType.contains(MetricUtilities.USER_CANCELED_SUBSTRING); - mRequestSessionMetric.logFailureOrUserCancel(isUserCanceled); - finishSession(/*propagateCancellation=*/false); + if (isUserCanceled) { + mRequestSessionMetric.setHasExceptionFinalPhase(/* has_exception */ false); + finishSession(/*propagateCancellation=*/false, + ApiStatus.USER_CANCELED.getMetricCode()); + } else { + finishSession(/*propagateCancellation=*/false, + ApiStatus.FAILURE.getMetricCode()); + } } /** @@ -419,7 +423,7 @@ abstract class RequestSession implements CredentialManagerUi.Credential @Override public void binderDied() { Slog.d(TAG, "Client binder died - clearing session"); - finishSession(isUiWaitingForData()); + finishSession(isUiWaitingForData(), ApiStatus.CLIENT_CANCELED.getMetricCode()); } } } diff --git a/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java b/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java index 8adcfbcb974c..a77bd3e280dd 100644 --- a/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java +++ b/services/credentials/java/com/android/server/credentials/metrics/RequestSessionMetric.java @@ -247,7 +247,7 @@ public class RequestSessionMetric { * * @param exceptionBitFinalPhase represents if the final phase provider had an exception */ - private void setHasExceptionFinalPhase(boolean exceptionBitFinalPhase) { + public void setHasExceptionFinalPhase(boolean exceptionBitFinalPhase) { try { mChosenProviderFinalPhaseMetric.setHasException(exceptionBitFinalPhase); } catch (Exception e) { -- cgit v1.2.3-59-g8ed1b