Merge "Fix null RollbackManager in RollbackHealthObserver and minor todos"

3 files changed, 68 insertions, 26 deletions

diff --git a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
index f50e776ad22d..b3cc6deb2810 100644
--- a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
+++ b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java
@@ -19,6 +19,7 @@ package com.android.server.rollback;
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageInstaller;
+import android.content.pm.VersionedPackage;
 import android.content.rollback.PackageRollbackInfo;
 import android.content.rollback.RollbackInfo;
 import android.content.rollback.RollbackManager;
@@ -41,12 +42,10 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
     private static final String TAG = "RollbackPackageHealthObserver";
     private static final String NAME = "rollback-observer";
     private Context mContext;
-    private RollbackManager mRollbackManager;
     private Handler mHandler;
 
     RollbackPackageHealthObserver(Context context) {
         mContext = context;
-        mRollbackManager = mContext.getSystemService(RollbackManager.class);
         HandlerThread handlerThread = new HandlerThread("RollbackPackageHealthObserver");
         handlerThread.start();
         mHandler = handlerThread.getThreadHandler();
@@ -55,7 +54,9 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
 
     @Override
     public int onHealthCheckFailed(String packageName, long versionCode) {
-        RollbackInfo rollback = getAvailableRollback(packageName, versionCode);
+        RollbackInfo rollback =
+                getAvailableRollback(mContext.getSystemService(RollbackManager.class),
+                    packageName, versionCode);
         if (rollback == null) {
             // Don't handle the notification, no rollbacks available for the package
             return PackageHealthObserverImpact.USER_IMPACT_NONE;
@@ -66,7 +67,8 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
 
     @Override
     public boolean execute(String packageName, long versionCode) {
-        RollbackInfo rollback = getAvailableRollback(packageName, versionCode);
+        RollbackManager rollbackManager = mContext.getSystemService(RollbackManager.class);
+        RollbackInfo rollback = getAvailableRollback(rollbackManager, packageName, versionCode);
         if (rollback == null) {
             // Expected a rollback to be available, what happened?
             return false;
@@ -86,12 +88,9 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
         });
 
         // TODO(zezeozue): Log initiated metrics
-        // TODO: Pass the package as a cause package instead of using
-        // Collections.emptyList once the version of the failing package is
-        // easily available.
         mHandler.post(() ->
-                mRollbackManager.commitRollback(rollback.getRollbackId(),
-                    Collections.emptyList(),
+                rollbackManager.commitRollback(rollback.getRollbackId(),
+                    Collections.singletonList(new VersionedPackage(packageName, versionCode)),
                     rollbackReceiver.getIntentSender()));
         // Assume rollback executed successfully
         return true;
@@ -110,8 +109,9 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve
         PackageWatchdog.getInstance(mContext).startObservingHealth(this, packages, durationMs);
     }
 
-    private RollbackInfo getAvailableRollback(String packageName, long versionCode) {
-        for (RollbackInfo rollback : mRollbackManager.getAvailableRollbacks()) {
+    private RollbackInfo getAvailableRollback(RollbackManager rollbackManager,
+            String packageName, long versionCode) {
+        for (RollbackInfo rollback : rollbackManager.getAvailableRollbacks()) {
             for (PackageRollbackInfo packageRollback : rollback.getPackages()) {
                 if (packageName.equals(packageRollback.getPackageName())
                         && packageRollback.getVersionRolledBackFrom().getVersionCode()
diff --git a/tests/RollbackTest/TestApp/src/com/android/tests/rollback/testapp/CrashingMainActivity.java b/tests/RollbackTest/TestApp/src/com/android/tests/rollback/testapp/CrashingMainActivity.java
index 02a439b5dd69..2310c829347e 100644
--- a/tests/RollbackTest/TestApp/src/com/android/tests/rollback/testapp/CrashingMainActivity.java
+++ b/tests/RollbackTest/TestApp/src/com/android/tests/rollback/testapp/CrashingMainActivity.java
@@ -17,17 +17,30 @@
 package com.android.tests.rollback.testapp;
 
 import android.app.Activity;
+import android.content.Context;
+import android.content.Intent;
+import android.content.SharedPreferences;
 import android.os.Bundle;
 
 /**
  * A crashing test app for testing apk rollback support.
  */
 public class CrashingMainActivity extends Activity {
-
     @Override
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
-
+        incrementCountAndBroadcast();
         throw new RuntimeException("Intended force crash");
     }
+
+    public void incrementCountAndBroadcast() {
+        SharedPreferences preferences = getSharedPreferences("prefs", Context.MODE_PRIVATE);
+        SharedPreferences.Editor editor = preferences.edit();
+        int count = preferences.getInt("crash_count", 0);
+        editor.putInt("crash_count", ++count).commit();
+
+        Intent intent = new Intent("com.android.tests.rollback.CRASH");
+        intent.putExtra("count", count);
+        sendBroadcast(intent);
+    }
 }
diff --git a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java
index e128a6ce9f7c..4b277ae850c5 100644
--- a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java
+++ b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java
@@ -17,6 +17,7 @@
 package com.android.tests.rollback;
 
 import android.Manifest;
+import android.app.ActivityManager;
 import android.content.BroadcastReceiver;
 import android.content.ComponentName;
 import android.content.Context;
@@ -36,7 +37,6 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -45,6 +45,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -413,7 +414,6 @@ public class RollbackTest {
 
     /**
      * Test that app user data is rolled back.
-     * TODO: Stop ignoring this test once user data rollback is supported.
      */
     @Test
     public void testUserDataRollback() throws Exception {
@@ -568,9 +568,7 @@ public class RollbackTest {
     }
 
     /**
-     * Test rollback of multi-package installs.
-     * TODO: Stop ignoring this test once support for multi-package rollback
-     * is implemented.
+     * Test rollback of multi-package installs is implemented.
      */
     @Test
     public void testMultiPackage() throws Exception {
@@ -630,18 +628,20 @@ public class RollbackTest {
         assertEquals(versionRolledBackTo, info.getVersionRolledBackTo().getLongVersionCode());
     }
 
-    // TODO(zezeozue): Stop ignoring after fixing race between rolling back and testing version
     /**
      * Test bad update automatic rollback.
      */
-    @Ignore("Flaky")
     @Test
     public void testBadUpdateRollback() throws Exception {
+        BroadcastReceiver crashCountReceiver = null;
+        Context context = InstrumentationRegistry.getContext();
         try {
             RollbackTestUtils.adoptShellPermissionIdentity(
                     Manifest.permission.INSTALL_PACKAGES,
                     Manifest.permission.DELETE_PACKAGES,
-                    Manifest.permission.MANAGE_ROLLBACKS);
+                    Manifest.permission.MANAGE_ROLLBACKS,
+                    Manifest.permission.KILL_BACKGROUND_PROCESSES,
+                    Manifest.permission.RESTART_PACKAGES);
             RollbackManager rm = RollbackTestUtils.getRollbackManager();
 
             // Prep installation of the test apps.
@@ -669,23 +669,52 @@ public class RollbackTest {
                     rm.getAvailableRollbacks(), TEST_APP_B);
             assertRollbackInfoEquals(TEST_APP_B, 2, 1, rollbackB);
 
+            BlockingQueue<Integer> crashQueue = new SynchronousQueue<>();
+
+            IntentFilter crashCountFilter = new IntentFilter();
+            crashCountFilter.addAction("com.android.tests.rollback.CRASH");
+            crashCountFilter.addCategory(Intent.CATEGORY_DEFAULT);
+
+            crashCountReceiver = new BroadcastReceiver() {
+                    @Override
+                    public void onReceive(Context context, Intent intent) {
+                        try {
+                            // Sleep long enough for packagewatchdog to be notified of crash
+                            Thread.sleep(1000);
+                            // Kill app and close AppErrorDialog
+                            ActivityManager am = context.getSystemService(ActivityManager.class);
+                            am.killBackgroundProcesses(TEST_APP_A);
+                            // Allow another package launch
+                            crashQueue.offer(intent.getIntExtra("count", 0), 5, TimeUnit.SECONDS);
+                        } catch (InterruptedException e) {
+                            fail("Failed to communicate with test app");
+                        }
+                    }
+                };
+            context.registerReceiver(crashCountReceiver, crashCountFilter);
+
             // Start apps PackageWatchdog#TRIGGER_FAILURE_COUNT times so TEST_APP_A crashes
-            for (int i = 0; i < 5; i++) {
+            Integer crashCount = null;
+            do {
                 RollbackTestUtils.launchPackage(TEST_APP_A);
-                Thread.sleep(1000);
-            }
-            Thread.sleep(1000);
+                crashCount = crashQueue.poll(5, TimeUnit.SECONDS);
+                if (crashCount == null) {
+                    fail("Timed out waiting for crash signal from test app");
+                }
+            } while(crashCount < 5);
 
             // TEST_APP_A is automatically rolled back by the RollbackPackageHealthObserver
             assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_A));
             // Instrumented app is still the package installer
-            Context context = InstrumentationRegistry.getContext();
             String installer = context.getPackageManager().getInstallerPackageName(TEST_APP_A);
             assertEquals(INSTRUMENTED_APP, installer);
             // TEST_APP_B is untouched
             assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_B));
         } finally {
             RollbackTestUtils.dropShellPermissionIdentity();
+            if (crashCountReceiver != null) {
+                context.unregisterReceiver(crashCountReceiver);
+            }
         }
     }