Merge "Log empty string for packages without logging_parent"

2 files changed, 21 insertions, 15 deletions

diff --git a/services/core/java/com/android/server/rollback/WatchdogRollbackLogger.java b/services/core/java/com/android/server/rollback/WatchdogRollbackLogger.java
index 79e1a2912147..46ec2f8258ca 100644
--- a/services/core/java/com/android/server/rollback/WatchdogRollbackLogger.java
+++ b/services/core/java/com/android/server/rollback/WatchdogRollbackLogger.java
@@ -70,20 +70,27 @@ public final class WatchdogRollbackLogger {
         }
     }
 
+    /**
+     * Returns the logging parent of a given package if it exists, {@code null} otherwise.
+     *
+     * The logging parent is defined by the {@code android.content.pm.LOGGING_PARENT} field in the
+     * metadata of a package's AndroidManifest.xml.
+     */
     @VisibleForTesting
+    @Nullable
     static VersionedPackage getLogPackage(Context context,
             @NonNull VersionedPackage failingPackage) {
         String logPackageName;
         VersionedPackage loggingParent;
         logPackageName = getLoggingParentName(context, failingPackage.getPackageName());
         if (logPackageName == null) {
-            return failingPackage;
+            return null;
         }
         try {
             loggingParent = new VersionedPackage(logPackageName, context.getPackageManager()
                     .getPackageInfo(logPackageName, 0 /* flags */).getLongVersionCode());
         } catch (PackageManager.NameNotFoundException e) {
-            return failingPackage;
+            return null;
         }
         return loggingParent;
     }
@@ -105,18 +112,14 @@ public final class WatchdogRollbackLogger {
             return;
         }
 
-        // Identify the logging parent for this rollback. When all configurations are correct, each
-        // package in the rollback refers to the same logging parent, except for the logging parent
-        // itself. If a logging parent is missing for a package, we use the package itself for
-        // logging. This might result in over-logging, but we prefer this over no logging.
+        // Identify the logging parent for this rollback. When all configurations are correct,
+        // each package in the rollback has a logging parent set in metadata.
         final Set<String> loggingPackageNames = new ArraySet<>();
         for (PackageRollbackInfo packageRollback : rollback.getPackages()) {
             final String loggingParentName = getLoggingParentName(context,
                     packageRollback.getPackageName());
             if (loggingParentName != null) {
                 loggingPackageNames.add(loggingParentName);
-            } else {
-                loggingPackageNames.add(packageRollback.getPackageName());
             }
         }
 
@@ -168,6 +171,10 @@ public final class WatchdogRollbackLogger {
         if (logPackage != null) {
             StatsLog.logWatchdogRollbackOccurred(type, logPackage.getPackageName(),
                     logPackage.getVersionCode(), rollbackReason, failingPackageName);
+        } else {
+            // In the case that the log package is null, still log an empty string as an
+            // indication that retrieving the logging parent failed.
+            StatsLog.logWatchdogRollbackOccurred(type, "", 0, rollbackReason, failingPackageName);
         }
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/rollback/WatchdogRollbackLoggerTest.java b/services/tests/servicestests/src/com/android/server/rollback/WatchdogRollbackLoggerTest.java
index 61117f18445b..ba493d4f9646 100644
--- a/services/tests/servicestests/src/com/android/server/rollback/WatchdogRollbackLoggerTest.java
+++ b/services/tests/servicestests/src/com/android/server/rollback/WatchdogRollbackLoggerTest.java
@@ -60,18 +60,18 @@ public class WatchdogRollbackLoggerTest {
     }
 
     /**
-     * Ensures that the original package is returned if the application info has no metadata.
+     * Ensures that null is returned if the application info has no metadata.
      */
     @Test
     public void testLogPackageHasNoMetadata() throws Exception {
         when(mMockPm.getApplicationInfo(anyString(), anyInt())).thenReturn(mApplicationInfo);
         VersionedPackage logPackage = WatchdogRollbackLogger.getLogPackage(mMockContext,
                 sTestPackageV1);
-        assertThat(logPackage).isEqualTo(sTestPackageV1);
+        assertThat(logPackage).isNull();
     }
 
     /**
-     * Ensures the original package is returned if the application info does not contain a logging
+     * Ensures that null is returned if the application info does not contain a logging
      * parent key.
      */
     @Test
@@ -81,7 +81,7 @@ public class WatchdogRollbackLoggerTest {
         bundle.putString(LOGGING_PARENT_KEY, null);
         VersionedPackage logPackage = WatchdogRollbackLogger.getLogPackage(mMockContext,
                 sTestPackageV1);
-        assertThat(logPackage).isEqualTo(sTestPackageV1);
+        assertThat(logPackage).isNull();
     }
 
     /**
@@ -102,8 +102,7 @@ public class WatchdogRollbackLoggerTest {
     }
 
     /**
-     * Ensures that the original package is returned if Package Manager does not know about the
-     * logging parent.
+     * Ensures that null is returned if Package Manager does not know about the logging parent.
      */
     @Test
     public void testLogPackageNameNotFound() throws Exception {
@@ -114,6 +113,6 @@ public class WatchdogRollbackLoggerTest {
                 new PackageManager.NameNotFoundException());
         VersionedPackage logPackage = WatchdogRollbackLogger.getLogPackage(mMockContext,
                 sTestPackageV1);
-        assertThat(logPackage).isEqualTo(sTestPackageV1);
+        assertThat(logPackage).isNull();
     }
 }