From 4005afaf6f39e621e03de6ffb007fccc869bf119 Mon Sep 17 00:00:00 2001 From: Reema Bajwa Date: Tue, 11 Apr 2023 07:34:12 +0000 Subject: [PATCH] Revise logging in RemoteCredentialService and CredentialProviderInfoFactory Test: built & deployed locally Bug: 270568738 Change-Id: I7f1d983f634ca686f954ef2ab2833f5a850b7d86 --- .../CredentialProviderInfoFactory.java | 38 +++++++++---------- .../CredentialProviderService.java | 2 +- .../credentials/CredentialManagerUi.java | 4 -- .../credentials/RemoteCredentialService.java | 21 +--------- 4 files changed, 20 insertions(+), 45 deletions(-) diff --git a/core/java/android/service/credentials/CredentialProviderInfoFactory.java b/core/java/android/service/credentials/CredentialProviderInfoFactory.java index d2a4a660452cd..fb2f4ad2abe65 100644 --- a/core/java/android/service/credentials/CredentialProviderInfoFactory.java +++ b/core/java/android/service/credentials/CredentialProviderInfoFactory.java @@ -42,7 +42,6 @@ import android.os.RemoteException; import android.os.UserHandle; import android.text.TextUtils; import android.util.AttributeSet; -import android.util.Log; import android.util.Slog; import android.util.Xml; @@ -54,7 +53,6 @@ import org.xmlpull.v1.XmlPullParserException; import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -181,7 +179,8 @@ public final class CredentialProviderInfoFactory { if (disableSystemAppVerificationForTests) { Bundle metadata = serviceInfo.metaData; if (metadata == null) { - Slog.e(TAG, "isValidSystemProvider - metadata is null: " + serviceInfo); + Slog.w(TAG, "metadata is null while reading " + + "TEST_SYSTEM_PROVIDER_META_DATA_KEY: " + serviceInfo); return false; } return metadata.getBoolean( @@ -200,7 +199,7 @@ public final class CredentialProviderInfoFactory { // 1. Get the metadata for the service. final Bundle metadata = serviceInfo.metaData; if (metadata == null) { - Log.i(TAG, "populateMetadata - metadata is null"); + Slog.w(TAG, "Metadata is null for provider: " + serviceInfo.getComponentName()); return builder; } @@ -209,12 +208,13 @@ public final class CredentialProviderInfoFactory { try { resources = pm.getResourcesForApplication(serviceInfo.applicationInfo); } catch (PackageManager.NameNotFoundException e) { - Log.e(TAG, "Failed to get app resources", e); + Slog.e(TAG, "Failed to get app resources", e); } // 3. Stop if we are missing data. - if (metadata == null || resources == null) { - Log.i(TAG, "populateMetadata - resources is null"); + if (resources == null) { + Slog.w(TAG, "Resources are null for the serviceInfo being processed: " + + serviceInfo.getComponentName()); return builder; } @@ -222,7 +222,7 @@ public final class CredentialProviderInfoFactory { try { builder = extractXmlMetadata(context, builder, serviceInfo, pm, resources); } catch (Exception e) { - Log.e(TAG, "Failed to get XML metadata", e); + Slog.e(TAG, "Failed to get XML metadata", e); } return builder; @@ -259,7 +259,7 @@ public final class CredentialProviderInfoFactory { afsAttributes.getString( R.styleable.CredentialProvider_settingsSubtitle)); } catch (Exception e) { - Log.e(TAG, "Failed to get XML attr", e); + Slog.e(TAG, "Failed to get XML attr", e); } finally { if (afsAttributes != null) { afsAttributes.recycle(); @@ -267,10 +267,10 @@ public final class CredentialProviderInfoFactory { } builder.addCapabilities(parseXmlProviderOuterCapabilities(parser, resources)); } else { - Log.e(TAG, "Meta-data does not start with credential-provider-service tag"); + Slog.w(TAG, "Meta-data does not start with credential-provider-service tag"); } } catch (IOException | XmlPullParserException e) { - Log.e(TAG, "Error parsing credential provider service meta-data", e); + Slog.e(TAG, "Error parsing credential provider service meta-data", e); } return builder; @@ -329,7 +329,7 @@ public final class CredentialProviderInfoFactory { return si; } } catch (RemoteException e) { - Slog.v(TAG, e.getMessage()); + Slog.e(TAG, "Unable to get serviceInfo", e); } throw new PackageManager.NameNotFoundException(serviceComponent.toString()); } @@ -377,10 +377,8 @@ public final class CredentialProviderInfoFactory { } services.add(serviceInfo); - } catch (SecurityException e) { - Slog.e(TAG, "Error getting info for " + serviceInfo + ": " + e); - } catch (PackageManager.NameNotFoundException e) { - Slog.e(TAG, "Error getting info for " + serviceInfo + ": " + e); + } catch (SecurityException | PackageManager.NameNotFoundException e) { + Slog.e(TAG, "Error getting info for " + serviceInfo, e); } } return services; @@ -432,7 +430,7 @@ public final class CredentialProviderInfoFactory { return pp; } catch (SecurityException e) { // If the current user is not enrolled in DPM then this can throw a security error. - Log.e(TAG, "Failed to get device policy: " + e); + Slog.e(TAG, "Failed to get device policy: " + e); } return null; @@ -593,7 +591,7 @@ public final class CredentialProviderInfoFactory { for (ResolveInfo resolveInfo : resolveInfos) { final ServiceInfo serviceInfo = resolveInfo.serviceInfo; if (serviceInfo == null) { - Log.i(TAG, "No serviceInfo found for resolveInfo so skipping this provider"); + Slog.d(TAG, "No serviceInfo found for resolveInfo, so skipping provider"); continue; } @@ -608,10 +606,8 @@ public final class CredentialProviderInfoFactory { if (!cpi.isSystemProvider()) { services.add(cpi); } - } catch (SecurityException e) { - Slog.e(TAG, "Error getting info for " + serviceInfo + ": " + e); } catch (Exception e) { - Slog.e(TAG, "Error getting info for " + serviceInfo + ": " + e); + Slog.e(TAG, "Error getting info for " + serviceInfo, e); } } return services; diff --git a/core/java/android/service/credentials/CredentialProviderService.java b/core/java/android/service/credentials/CredentialProviderService.java index e87333fe49418..53a5fd5c634db 100644 --- a/core/java/android/service/credentials/CredentialProviderService.java +++ b/core/java/android/service/credentials/CredentialProviderService.java @@ -226,7 +226,7 @@ public abstract class CredentialProviderService extends Service { if (SERVICE_INTERFACE.equals(intent.getAction())) { return mInterface.asBinder(); } - Log.i(TAG, "Failed to bind with intent: " + intent); + Log.d(TAG, "Failed to bind with intent: " + intent); return null; } diff --git a/services/credentials/java/com/android/server/credentials/CredentialManagerUi.java b/services/credentials/java/com/android/server/credentials/CredentialManagerUi.java index 87509067f9932..0dee7a44375d3 100644 --- a/services/credentials/java/com/android/server/credentials/CredentialManagerUi.java +++ b/services/credentials/java/com/android/server/credentials/CredentialManagerUi.java @@ -33,7 +33,6 @@ import android.os.IBinder; import android.os.Looper; import android.os.ResultReceiver; import android.service.credentials.CredentialProviderInfoFactory; -import android.util.Log; import android.util.Slog; import java.util.ArrayList; @@ -124,7 +123,6 @@ public class CredentialManagerUi { public CredentialManagerUi(Context context, int userId, CredentialManagerUiCallback callbacks, Set enabledProviders) { - Log.i(TAG, "In CredentialManagerUi constructor"); mContext = context; mUserId = userId; mCallbacks = callbacks; @@ -151,8 +149,6 @@ public class CredentialManagerUi { */ public PendingIntent createPendingIntent( RequestInfo requestInfo, ArrayList providerDataList) { - Log.i(TAG, "In createPendingIntent"); - List allProviders = CredentialProviderInfoFactory.getCredentialProviderServices( mContext, diff --git a/services/credentials/java/com/android/server/credentials/RemoteCredentialService.java b/services/credentials/java/com/android/server/credentials/RemoteCredentialService.java index c1e9bc6a5c3ca..0ad73c9452842 100644 --- a/services/credentials/java/com/android/server/credentials/RemoteCredentialService.java +++ b/services/credentials/java/com/android/server/credentials/RemoteCredentialService.java @@ -40,7 +40,6 @@ import android.service.credentials.IBeginGetCredentialCallback; import android.service.credentials.IClearCredentialStateCallback; import android.service.credentials.ICredentialProviderService; import android.text.format.DateUtils; -import android.util.Log; import android.util.Slog; import com.android.internal.infra.ServiceConnector; @@ -122,7 +121,6 @@ public class RemoteCredentialService extends ServiceConnector.Impl callback) { - Log.i(TAG, "In onGetCredentials in RemoteCredentialService"); AtomicReference cancellationSink = new AtomicReference<>(); AtomicReference> futureRef = new AtomicReference<>(); @@ -142,7 +140,6 @@ public class RemoteCredentialService extends ServiceConnector.Impl callback) { - Log.i(TAG, "In onCreateCredential in RemoteCredentialService"); AtomicReference cancellationSink = new AtomicReference<>(); AtomicReference> futureRef = new AtomicReference<>(); @@ -197,14 +193,11 @@ public class RemoteCredentialService extends ServiceConnector.Impl callback) { - Log.i(TAG, "In onClearCredentialState in RemoteCredentialService"); AtomicReference cancellationSink = new AtomicReference<>(); AtomicReference> futureRef = new AtomicReference<>(); @@ -258,14 +250,11 @@ public class RemoteCredentialService extends ServiceConnector.Impl cancellationSink, ProviderCallbacks callback) { if (error == null) { - Log.i(TAG, "In RemoteCredentialService execute error is null"); callback.onProviderResponseSuccess(result); } else { if (error instanceof TimeoutException) { - Log.i(TAG, "In RemoteCredentialService execute error is timeout"); + Slog.d(TAG, "Remote provider response timed tuo for: " + mComponentName); dispatchCancellationSignal(cancellationSink.get()); callback.onProviderResponseFailure( CredentialProviderErrors.ERROR_TIMEOUT, null); } else if (error instanceof CancellationException) { - Log.i(TAG, "In RemoteCredentialService execute error is cancellation"); + Slog.d(TAG, "Cancellation exception for remote provider: " + mComponentName); dispatchCancellationSignal(cancellationSink.get()); callback.onProviderResponseFailure( CredentialProviderErrors.ERROR_TASK_CANCELED, null); } else if (error instanceof GetCredentialException) { - Log.i(TAG, "In RemoteCredentialService execute error is provider get" - + "error"); callback.onProviderResponseFailure( CredentialProviderErrors.ERROR_PROVIDER_FAILURE, (GetCredentialException) error); } else if (error instanceof CreateCredentialException) { - Log.i(TAG, "In RemoteCredentialService execute error is provider create " - + "error"); callback.onProviderResponseFailure( CredentialProviderErrors.ERROR_PROVIDER_FAILURE, (CreateCredentialException) error); } else { - Log.i(TAG, "In RemoteCredentialService execute error is unknown"); callback.onProviderResponseFailure( CredentialProviderErrors.ERROR_UNKNOWN, (Exception) error);