From 291772e64d42ef51477353e17d3fc81df9a5e760 Mon Sep 17 00:00:00 2001
From: Reema Bajwa <reemabajwa@google.com>
Date: Fri, 7 Apr 2023 00:16:36 +0000
Subject: [PATCH] Revise logging in session classes

Bug: 270568738
Test: built locally

Change-Id: Idada00ff355a89efe26b7e77a2a8e74f2123737a
---
 .../credentials/ClearRequestSession.java      | 11 +++++-----
 .../credentials/CreateRequestSession.java     | 11 +++++-----
 .../server/credentials/GetRequestSession.java | 11 +++++-----
 .../credentials/PrepareGetRequestSession.java | 10 ++++-----
 .../server/credentials/RequestSession.java    | 21 +++++++------------
 5 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/services/credentials/java/com/android/server/credentials/ClearRequestSession.java b/services/credentials/java/com/android/server/credentials/ClearRequestSession.java
index 5c77aa22ece8a..19a0c5e8adcb1 100644
--- a/services/credentials/java/com/android/server/credentials/ClearRequestSession.java
+++ b/services/credentials/java/com/android/server/credentials/ClearRequestSession.java
@@ -27,7 +27,7 @@ import android.credentials.ui.RequestInfo;
 import android.os.CancellationSignal;
 import android.os.RemoteException;
 import android.service.credentials.CallingAppInfo;
-import android.util.Log;
+import android.util.Slog;
 
 import java.util.ArrayList;
 import java.util.Set;
@@ -67,7 +67,8 @@ public final class ClearRequestSession extends RequestSession<ClearCredentialSta
                 .createNewSession(mContext, mUserId, providerInfo,
                         this, remoteCredentialService);
         if (providerClearSession != null) {
-            Log.i(TAG, "In startProviderSession - provider session created and being added");
+            Slog.d(TAG, "In startProviderSession - provider session created "
+                    + "and being added for: " + providerInfo.getComponentName());
             mProviders.put(providerClearSession.getComponentName().flattenToString(),
                     providerClearSession);
         }
@@ -77,12 +78,12 @@ public final class ClearRequestSession extends RequestSession<ClearCredentialSta
     @Override // from provider session
     public void onProviderStatusChanged(ProviderSession.Status status,
             ComponentName componentName, ProviderSession.CredentialsSource source) {
-        Log.i(TAG, "in onStatusChanged with status: " + status);
+        Slog.d(TAG, "in onStatusChanged with status: " + status + ", and source: " + source);
         if (ProviderSession.isTerminatingStatus(status)) {
-            Log.i(TAG, "in onStatusChanged terminating status");
+            Slog.d(TAG, "in onProviderStatusChanged terminating status");
             onProviderTerminated(componentName);
         } else if (ProviderSession.isCompletionStatus(status)) {
-            Log.i(TAG, "in onStatusChanged isCompletionStatus status");
+            Slog.d(TAG, "in onProviderStatusChanged isCompletionStatus status");
             onProviderResponseComplete(componentName);
         }
     }
diff --git a/services/credentials/java/com/android/server/credentials/CreateRequestSession.java b/services/credentials/java/com/android/server/credentials/CreateRequestSession.java
index 02aaf867fa7ba..a04143afadcd4 100644
--- a/services/credentials/java/com/android/server/credentials/CreateRequestSession.java
+++ b/services/credentials/java/com/android/server/credentials/CreateRequestSession.java
@@ -33,7 +33,7 @@ import android.os.CancellationSignal;
 import android.os.RemoteException;
 import android.service.credentials.CallingAppInfo;
 import android.service.credentials.PermissionUtils;
-import android.util.Log;
+import android.util.Slog;
 
 import com.android.server.credentials.metrics.ProviderStatusForMetrics;
 
@@ -77,7 +77,8 @@ public final class CreateRequestSession extends RequestSession<CreateCredentialR
                 .createNewSession(mContext, mUserId, providerInfo,
                         this, remoteCredentialService);
         if (providerCreateSession != null) {
-            Log.i(TAG, "In startProviderSession - provider session created and being added");
+            Slog.d(TAG, "In initiateProviderSession - provider session created and "
+                    + "being added for: " + providerInfo.getComponentName());
             mProviders.put(providerCreateSession.getComponentName().flattenToString(),
                     providerCreateSession);
         }
@@ -120,7 +121,7 @@ public final class CreateRequestSession extends RequestSession<CreateCredentialR
     @Override
     public void onFinalResponseReceived(ComponentName componentName,
             @Nullable CreateCredentialResponse response) {
-        Log.i(TAG, "onFinalCredentialReceived from: " + componentName.flattenToString());
+        Slog.d(TAG, "onFinalCredentialReceived from: " + componentName.flattenToString());
         mRequestSessionMetric.collectUiResponseData(/*uiReturned=*/ true, System.nanoTime());
         mRequestSessionMetric.collectChosenMetricViaCandidateTransfer(mProviders.get(
                 componentName.flattenToString()).mProviderSessionMetric
@@ -163,13 +164,13 @@ public final class CreateRequestSession extends RequestSession<CreateCredentialR
     @Override
     public void onProviderStatusChanged(ProviderSession.Status status,
             ComponentName componentName, ProviderSession.CredentialsSource source) {
-        Log.i(TAG, "in onProviderStatusChanged with status: " + status);
+        Slog.d(TAG, "in onStatusChanged with status: " + status + ", and source: " + source);
         // If all provider responses have been received, we can either need the UI,
         // or we need to respond with error. The only other case is the entry being
         // selected after the UI has been invoked which has a separate code path.
         if (!isAnyProviderPending()) {
             if (isUiInvocationNeeded()) {
-                Log.i(TAG, "in onProviderStatusChanged - isUiInvocationNeeded");
+                Slog.d(TAG, "in onProviderStatusChanged - isUiInvocationNeeded");
                 getProviderDataAndInitiateUi();
             } else {
                 respondToClientWithErrorAndFinish(CreateCredentialException.TYPE_NO_CREATE_OPTIONS,
diff --git a/services/credentials/java/com/android/server/credentials/GetRequestSession.java b/services/credentials/java/com/android/server/credentials/GetRequestSession.java
index c44e665ba699d..aeb4801628f29 100644
--- a/services/credentials/java/com/android/server/credentials/GetRequestSession.java
+++ b/services/credentials/java/com/android/server/credentials/GetRequestSession.java
@@ -30,7 +30,7 @@ import android.credentials.ui.RequestInfo;
 import android.os.CancellationSignal;
 import android.os.RemoteException;
 import android.service.credentials.CallingAppInfo;
-import android.util.Log;
+import android.util.Slog;
 
 import com.android.server.credentials.metrics.ProviderStatusForMetrics;
 
@@ -77,7 +77,8 @@ public class GetRequestSession extends RequestSession<GetCredentialRequest,
                 .createNewSession(mContext, mUserId, providerInfo,
                         this, remoteCredentialService);
         if (providerGetSession != null) {
-            Log.i(TAG, "In startProviderSession - provider session created and being added");
+            Slog.d(TAG, "In startProviderSession - provider session created and "
+                    + "being added for: " + providerInfo.getComponentName());
             mProviders.put(providerGetSession.getComponentName().flattenToString(),
                     providerGetSession);
         }
@@ -116,7 +117,7 @@ public class GetRequestSession extends RequestSession<GetCredentialRequest,
     @Override
     public void onFinalResponseReceived(ComponentName componentName,
             @Nullable GetCredentialResponse response) {
-        Log.i(TAG, "onFinalCredentialReceived from: " + componentName.flattenToString());
+        Slog.d(TAG, "onFinalResponseReceived from: " + componentName.flattenToString());
         mRequestSessionMetric.collectUiResponseData(/*uiReturned=*/ true, System.nanoTime());
         mRequestSessionMetric.collectChosenMetricViaCandidateTransfer(
                 mProviders.get(componentName.flattenToString())
@@ -160,7 +161,7 @@ public class GetRequestSession extends RequestSession<GetCredentialRequest,
     @Override
     public void onProviderStatusChanged(ProviderSession.Status status,
             ComponentName componentName, ProviderSession.CredentialsSource source) {
-        Log.i(TAG, "in onStatusChanged with status: " + status + "and source: " + source);
+        Slog.d(TAG, "in onStatusChanged with status: " + status + ", and source: " + source);
 
         // Auth entry was selected, and it did not have any underlying credentials
         if (status == ProviderSession.Status.NO_CREDENTIALS_FROM_AUTH_ENTRY) {
@@ -173,7 +174,7 @@ public class GetRequestSession extends RequestSession<GetCredentialRequest,
             // or we need to respond with error. The only other case is the entry being
             // selected after the UI has been invoked which has a separate code path.
             if (isUiInvocationNeeded()) {
-                Log.i(TAG, "in onProviderStatusChanged - isUiInvocationNeeded");
+                Slog.d(TAG, "in onProviderStatusChanged - isUiInvocationNeeded");
                 getProviderDataAndInitiateUi();
             } else {
                 respondToClientWithErrorAndFinish(GetCredentialException.TYPE_NO_CREDENTIAL,
diff --git a/services/credentials/java/com/android/server/credentials/PrepareGetRequestSession.java b/services/credentials/java/com/android/server/credentials/PrepareGetRequestSession.java
index f274e65a20c3f..9e7a87e74522b 100644
--- a/services/credentials/java/com/android/server/credentials/PrepareGetRequestSession.java
+++ b/services/credentials/java/com/android/server/credentials/PrepareGetRequestSession.java
@@ -33,7 +33,6 @@ import android.os.CancellationSignal;
 import android.os.RemoteException;
 import android.service.credentials.CallingAppInfo;
 import android.service.credentials.PermissionUtils;
-import android.util.Log;
 import android.util.Slog;
 
 import java.util.ArrayList;
@@ -67,6 +66,9 @@ public class PrepareGetRequestSession extends GetRequestSession {
     @Override
     public void onProviderStatusChanged(ProviderSession.Status status, ComponentName componentName,
             ProviderSession.CredentialsSource source) {
+        Slog.d(TAG, "in onProviderStatusChanged with status: " + status + ", and "
+                + "source: " + source);
+
         switch (source) {
             case REMOTE_PROVIDER:
                 // Remote provider's status changed. We should check if all providers are done, and
@@ -123,7 +125,7 @@ public class PrepareGetRequestSession extends GetRequestSession {
                             hasPermission,
                             credentialTypes, hasAuthenticationResults, hasRemoteResults, uiIntent));
         } catch (RemoteException e) {
-            Log.e(TAG, "EXCEPTION while mPendingCallback.onResponse", e);
+            Slog.e(TAG, "EXCEPTION while mPendingCallback.onResponse", e);
         }
     }
 
@@ -138,7 +140,7 @@ public class PrepareGetRequestSession extends GetRequestSession {
                             /*hasRemoteResults=*/ false,
                             /*pendingIntent=*/ null));
         } catch (RemoteException e) {
-            Log.e(TAG, "EXCEPTION while mPendingCallback.onResponse", e);
+            Slog.e(TAG, "EXCEPTION while mPendingCallback.onResponse", e);
         }
     }
 
@@ -179,10 +181,8 @@ public class PrepareGetRequestSession extends GetRequestSession {
     private PendingIntent getUiIntent() {
         ArrayList<ProviderData> providerDataList = new ArrayList<>();
         for (ProviderSession session : mProviders.values()) {
-            Log.i(TAG, "preparing data for : " + session.getComponentName());
             ProviderData providerData = session.prepareUiData();
             if (providerData != null) {
-                Log.i(TAG, "Provider data is not null");
                 providerDataList.add(providerData);
             }
         }
diff --git a/services/credentials/java/com/android/server/credentials/RequestSession.java b/services/credentials/java/com/android/server/credentials/RequestSession.java
index e98c5241ae002..8fd02691e190d 100644
--- a/services/credentials/java/com/android/server/credentials/RequestSession.java
+++ b/services/credentials/java/com/android/server/credentials/RequestSession.java
@@ -32,7 +32,6 @@ import android.os.Looper;
 import android.os.RemoteException;
 import android.os.UserHandle;
 import android.service.credentials.CallingAppInfo;
-import android.util.Log;
 import android.util.Slog;
 
 import com.android.internal.R;
@@ -179,7 +178,7 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
     @Override // from CredentialManagerUiCallbacks
     public void onUiSelection(UserSelectionDialogResult selection) {
         if (mRequestSessionStatus == RequestSessionStatus.COMPLETE) {
-            Log.i(TAG, "Request has already been completed. This is strange.");
+            Slog.w(TAG, "Request has already been completed. This is strange.");
             return;
         }
         if (isSessionCancelled()) {
@@ -187,13 +186,11 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
             return;
         }
         String providerId = selection.getProviderId();
-        Log.i(TAG, "onUiSelection, providerId: " + providerId);
         ProviderSession providerSession = mProviders.get(providerId);
         if (providerSession == null) {
-            Log.i(TAG, "providerSession not found in onUiSelection");
+            Slog.w(TAG, "providerSession not found in onUiSelection. This is strange.");
             return;
         }
-        Log.i(TAG, "Provider session found");
         mRequestSessionMetric.collectMetricPerBrowsingSelect(selection,
                 providerSession.mProviderSessionMetric.getCandidatePhasePerProviderMetric());
         providerSession.onUiEntrySelected(selection.getEntryKey(),
@@ -247,15 +244,13 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
     void getProviderDataAndInitiateUi() {
         ArrayList<ProviderData> providerDataList = getProviderDataForUi();
         if (!providerDataList.isEmpty()) {
-            Log.i(TAG, "provider list not empty about to initiate ui");
             launchUiWithProviderData(providerDataList);
         }
     }
 
     @NonNull
     protected ArrayList<ProviderData> getProviderDataForUi() {
-        Log.i(TAG, "In getProviderDataAndInitiateUi");
-        Log.i(TAG, "In getProviderDataAndInitiateUi providers size: " + mProviders.size());
+        Slog.d(TAG, "In getProviderDataAndInitiateUi providers size: " + mProviders.size());
         ArrayList<ProviderData> providerDataList = new ArrayList<>();
         mRequestSessionMetric.logCandidatePhaseMetrics(mProviders);
 
@@ -265,10 +260,8 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
         }
 
         for (ProviderSession session : mProviders.values()) {
-            Log.i(TAG, "preparing data for : " + session.getComponentName());
             ProviderData providerData = session.prepareUiData();
             if (providerData != null) {
-                Log.i(TAG, "Provider data is not null");
                 providerDataList.add(providerData);
             }
         }
@@ -284,7 +277,7 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
         mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(/*has_exception=*/ false,
                 ProviderStatusForMetrics.FINAL_SUCCESS);
         if (mRequestSessionStatus == RequestSessionStatus.COMPLETE) {
-            Log.i(TAG, "Request has already been completed. This is strange.");
+            Slog.w(TAG, "Request has already been completed. This is strange.");
             return;
         }
         if (isSessionCancelled()) {
@@ -300,7 +293,7 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
         } catch (RemoteException e) {
             mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(
                     /*has_exception=*/ true, ProviderStatusForMetrics.FINAL_FAILURE);
-            Log.i(TAG, "Issue while responding to client with a response : " + e.getMessage());
+            Slog.e(TAG, "Issue while responding to client with a response : " + e);
             mRequestSessionMetric.logApiCalledAtFinish(
                     /*apiStatus=*/ ApiStatus.FAILURE.getMetricCode());
         }
@@ -317,7 +310,7 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
         mRequestSessionMetric.collectFinalPhaseProviderMetricStatus(
                 /*has_exception=*/ true, ProviderStatusForMetrics.FINAL_FAILURE);
         if (mRequestSessionStatus == RequestSessionStatus.COMPLETE) {
-            Log.i(TAG, "Request has already been completed. This is strange.");
+            Slog.w(TAG, "Request has already been completed. This is strange.");
             return;
         }
         if (isSessionCancelled()) {
@@ -330,7 +323,7 @@ abstract class RequestSession<T, U, V> implements CredentialManagerUi.Credential
         try {
             invokeClientCallbackError(errorType, errorMsg);
         } catch (RemoteException e) {
-            Log.i(TAG, "Issue while responding to client with error : " + e.getMessage());
+            Slog.e(TAG, "Issue while responding to client with error : " + e);
         }
         boolean isUserCanceled = errorType.contains(MetricUtilities.USER_CANCELED_SUBSTRING);
         mRequestSessionMetric.logFailureOrUserCancel(isUserCanceled);