Merge "Revise logging in RemoteCredentialService and CredentialProviderInfoFactory" into udc-dev

4 files changed, 20 insertions, 44 deletions

diff --git a/core/java/android/service/credentials/CredentialProviderInfoFactory.java b/core/java/android/service/credentials/CredentialProviderInfoFactory.java
index c32ce6095ab3..fb2f4ad2abe6 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;
 
@@ -180,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(
@@ -199,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;
         }
 
@@ -208,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;
         }
 
@@ -221,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;
@@ -258,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();
@@ -266,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;
@@ -328,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());
     }
@@ -376,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;
@@ -431,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;
@@ -592,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;
             }
 
@@ -607,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 e87333fe4941..53a5fd5c634d 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 87509067f993..0dee7a44375d 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<ComponentName> 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<ProviderData> providerDataList) {
-        Log.i(TAG, "In createPendingIntent");
-
         List<CredentialProviderInfo> 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 c1e9bc6a5c3c..0ad73c945284 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<ICredentialPr
      */
     public void onBeginGetCredential(@NonNull BeginGetCredentialRequest request,
             ProviderCallbacks<BeginGetCredentialResponse> callback) {
-        Log.i(TAG, "In onGetCredentials in RemoteCredentialService");
         AtomicReference<ICancellationSignal> cancellationSink = new AtomicReference<>();
         AtomicReference<CompletableFuture<BeginGetCredentialResponse>> futureRef =
                 new AtomicReference<>();
@@ -142,7 +140,6 @@ public class RemoteCredentialService extends ServiceConnector.Impl<ICredentialPr
 
                             @Override
                             public void onFailure(String errorType, CharSequence message) {
-                                Log.i(TAG, "In onFailure in RemoteCredentialService");
                                 String errorMsg = message == null ? "" : String.valueOf(
                                         message);
                                 getCredentials.completeExceptionally(
@@ -182,7 +179,6 @@ public class RemoteCredentialService extends ServiceConnector.Impl<ICredentialPr
      */
     public void onBeginCreateCredential(@NonNull BeginCreateCredentialRequest request,
             ProviderCallbacks<BeginCreateCredentialResponse> callback) {
-        Log.i(TAG, "In onCreateCredential in RemoteCredentialService");
         AtomicReference<ICancellationSignal> cancellationSink = new AtomicReference<>();
         AtomicReference<CompletableFuture<BeginCreateCredentialResponse>> futureRef =
                 new AtomicReference<>();
@@ -197,14 +193,11 @@ public class RemoteCredentialService extends ServiceConnector.Impl<ICredentialPr
                                 request, new IBeginCreateCredentialCallback.Stub() {
                                     @Override
                                     public void onSuccess(BeginCreateCredentialResponse response) {
-                                        Log.i(TAG, "In onSuccess onBeginCreateCredential "
-                                                + "in RemoteCredentialService");
                                         createCredentialFuture.complete(response);
                                     }
 
                                     @Override
                                     public void onFailure(String errorType, CharSequence message) {
-                                        Log.i(TAG, "In onFailure in RemoteCredentialService");
                                         String errorMsg = message == null ? "" : String.valueOf(
                                                 message);
                                         createCredentialFuture.completeExceptionally(
@@ -244,7 +237,6 @@ public class RemoteCredentialService extends ServiceConnector.Impl<ICredentialPr
      */
     public void onClearCredentialState(@NonNull ClearCredentialStateRequest request,
             ProviderCallbacks<Void> callback) {
-        Log.i(TAG, "In onClearCredentialState in RemoteCredentialService");
         AtomicReference<ICancellationSignal> cancellationSink = new AtomicReference<>();
         AtomicReference<CompletableFuture<Void>> futureRef = new AtomicReference<>();
 
@@ -258,14 +250,11 @@ public class RemoteCredentialService extends ServiceConnector.Impl<ICredentialPr
                                 request, new IClearCredentialStateCallback.Stub() {
                                     @Override
                                     public void onSuccess() {
-                                        Log.i(TAG, "In onSuccess onClearCredentialState "
-                                                + "in RemoteCredentialService");
                                         clearCredentialFuture.complete(null);
                                     }
 
                                     @Override
                                     public void onFailure(String errorType, CharSequence message) {
-                                        Log.i(TAG, "In onFailure in RemoteCredentialService");
                                         String errorMsg = message == null ? "" :
                                                 String.valueOf(message);
                                         clearCredentialFuture.completeExceptionally(
@@ -300,35 +289,29 @@ public class RemoteCredentialService extends ServiceConnector.Impl<ICredentialPr
             AtomicReference<ICancellationSignal> cancellationSink,
             ProviderCallbacks<T> 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);