Android: ensuring onDestroy called between each test

The previous onDestroy in tests change only blocked until onDestroy with certain
subclasses of ChromeActivityTestRule which called launchActivity. This change
blocks all tests on onDestroy.

Bug: 908174
Change-Id: I074054c28d002e012d0f6ebc95a53aadf4175eaa
Reviewed-on: https://chromium-review.googlesource.com/c/1473330
Commit-Queue: Sam Maier <smaier@chromium.org>
Auto-Submit: Sam Maier <smaier@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636023}

base/BUILD.gn

@@ -3170,6 +3170,7 @@ if (is_android) {
       "test/android/javatests/src/org/chromium/base/test/BaseChromiumRunnerCommon.java",
       "test/android/javatests/src/org/chromium/base/test/BaseTestResult.java",
       "test/android/javatests/src/org/chromium/base/test/LifetimeAssertRule.java",
+      "test/android/javatests/src/org/chromium/base/test/DestroyActivitiesRule.java",
       "test/android/javatests/src/org/chromium/base/test/ScreenshotOnFailureStatement.java",
       "test/android/javatests/src/org/chromium/base/test/SetUpTestRule.java",
       "test/android/javatests/src/org/chromium/base/test/SetUpStatement.java",

base/android/java/src/org/chromium/base/ApplicationStatus.java

@@ -216,7 +216,7 @@ public void onWindowFocusChanged(boolean hasFocus) {
         }
     }
 
-    private static boolean isInitialized() {
+    public static boolean isInitialized() {
         synchronized (sActivityInfo) {
             return sCurrentApplicationState != ApplicationState.UNKNOWN;
         }

base/test/android/javatests/src/org/chromium/base/test/BaseJUnit4ClassRunner.java

@@ -174,7 +174,7 @@ protected List<MethodRule> getDefaultMethodRules() {
      */
     @CallSuper
     protected List<TestRule> getDefaultTestRules() {
-        return Collections.singletonList(new LifetimeAssertRule());
+        return Arrays.asList(new DestroyActivitiesRule(), new LifetimeAssertRule());
     }
 
     /**

base/test/android/javatests/src/org/chromium/base/test/DestroyActivitiesRule.java

@@ -0,0 +1,72 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.base.test;
+
+import android.app.Activity;
+import android.os.Build;
+
+import org.junit.rules.ExternalResource;
+
+import org.chromium.base.ActivityState;
+import org.chromium.base.ApplicationStatus;
+import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.test.util.CallbackHelper;
+
+import java.util.concurrent.TimeoutException;
+
+/**
+ * This is to ensure all calls to onDestroy() are performed before starting the next test.
+ * We could probably remove this when crbug.com/932130 is fixed.
+ */
+public class DestroyActivitiesRule extends ExternalResource {
+    private static final String TAG = "DestroyActivities";
+    @Override
+    public void after() {
+        if (ApplicationStatus.isInitialized()) {
+            CallbackHelper allDestroyedCalledback = new CallbackHelper();
+            ApplicationStatus.ActivityStateListener activityStateListener =
+                    (activity, newState) -> {
+                switch (newState) {
+                    case ActivityState.DESTROYED:
+                        if (ApplicationStatus.isEveryActivityDestroyed()) {
+                            allDestroyedCalledback.notifyCalled();
+                        }
+                        break;
+                    case ActivityState.CREATED:
+                        if (!activity.isFinishing()) {
+                            activity.finish();
+                        }
+                        break;
+                }
+            };
+            ApplicationStatus.registerStateListenerForAllActivities(activityStateListener);
+            ThreadUtils.runOnUiThread(() -> {
+                for (Activity a : ApplicationStatus.getRunningActivities()) {
+                    if (!a.isFinishing()) {
+                        a.finish();
+                    }
+                }
+            });
+            try {
+                if (!ApplicationStatus.isEveryActivityDestroyed()) {
+                    allDestroyedCalledback.waitForCallback();
+                }
+            } catch (InterruptedException | TimeoutException e) {
+                // There appears to be a framework bug on KitKat where onStop and onDestroy are not
+                // called for a handful of tests.
+                if (Build.VERSION.SDK_INT > Build.VERSION_CODES.KITKAT) {
+                    Log.w(TAG, "Activity failed to be destroyed after a test");
+                    for (Activity a : ApplicationStatus.getRunningActivities()) {
+                        ApplicationStatus.onStateChangeForTesting(a, ActivityState.DESTROYED);
+                    }
+                    throw new RuntimeException(e);
+                }
+            } finally {
+                ApplicationStatus.unregisterActivityStateListener(activityStateListener);
+            }
+        }
+    }
+}

chrome/android/javatests/src/org/chromium/chrome/browser/invalidation/DelayedInvalidationsControllerTest.java

@@ -12,6 +12,7 @@
 import android.support.test.filters.SmallTest;
 import android.support.test.rule.UiThreadTestRule;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
@@ -75,6 +76,11 @@ public void setUp() throws Exception {
         setApplicationState(ActivityState.RESUMED);
     }
 
+    @After
+    public void tearDown() {
+        setApplicationState(ActivityState.DESTROYED);
+    }
+
     private void setApplicationState(int newState) {
         ApplicationStatus.onStateChangeForTesting(mPlaceholderActivity, newState);
     }

chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabWindowManagerTest.java

@@ -9,6 +9,7 @@
 import android.support.test.filters.SmallTest;
 import android.support.test.rule.UiThreadTestRule;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
@@ -24,12 +25,16 @@
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.tabmodel.MockTabModelSelector;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * Test for {@link TabWindowManager} APIs.  Makes sure the class handles multiple {@link Activity}s
  * requesting {@link TabModelSelector}s, {@link Activity}s getting destroyed, etc..
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
 public class TabWindowManagerTest {
+    private List<Activity> mActivities = new ArrayList<>();
     private final TabModelSelectorFactory mMockTabModelSelectorFactory =
             new TabModelSelectorFactory() {
                 @Override
@@ -41,16 +46,30 @@ public TabModelSelector buildSelector(Activity activity,
 
     private ChromeActivity buildActivity() {
         ChromeActivity activity = new CustomTabActivity();
+        mActivities.add(activity);
         ApplicationStatus.onStateChangeForTesting(activity, ActivityState.CREATED);
         return activity;
     }
 
+    private void destroyActivity(Activity a) {
+        mActivities.remove(a);
+        ApplicationStatus.onStateChangeForTesting(a, ActivityState.DESTROYED);
+    }
+
     private MockTabModelSelector requestSelector(ChromeActivity activity, int requestedIndex) {
         final TabWindowManager manager = TabWindowManager.getInstance();
         manager.setTabModelSelectorFactory(mMockTabModelSelectorFactory);
         return (MockTabModelSelector) manager.requestSelector(activity, activity, requestedIndex);
     }
 
+    @After
+    public void tearDown() {
+        for (Activity a : mActivities) {
+            ApplicationStatus.onStateChangeForTesting(a, ActivityState.DESTROYED);
+        }
+        mActivities.clear();
+    }
+
     @Rule
     public UiThreadTestRule mRule = new UiThreadTestRule();
 
@@ -103,11 +122,13 @@ public void testMultipleActivities() {
     @UiThreadTest
     public void testTooManyActivities() {
         for (int i = 0; i < TabWindowManager.MAX_SIMULTANEOUS_SELECTORS; i++) {
-            Assert.assertNotNull("Could not build selector", requestSelector(buildActivity(), 0));
+            ChromeActivity a = buildActivity();
+            Assert.assertNotNull("Could not build selector", requestSelector(a, 0));
         }
 
-        Assert.assertNull("Built selectors past the max number supported",
-                requestSelector(buildActivity(), 0));
+        ChromeActivity activity = buildActivity();
+        Assert.assertNull(
+                "Built selectors past the max number supported", requestSelector(activity, 0));
     }
 
     /**
@@ -176,7 +197,7 @@ public void testActivityDeathRemovesSingle() {
         Assert.assertNotNull("Was not able to build the TabModelSelector", selector0);
         Assert.assertEquals("Unexpected model index", 0, manager.getIndexForWindow(activity0));
 
-        ApplicationStatus.onStateChangeForTesting(activity0, ActivityState.DESTROYED);
+        destroyActivity(activity0);
 
         Assert.assertEquals("Still found model", TabWindowManager.INVALID_WINDOW_INDEX,
                 manager.getIndexForWindow(activity0));
@@ -199,7 +220,7 @@ public void testActivityDeathLetsModelReassign() {
         Assert.assertNotNull("Was not able to build the TabModelSelector", selector0);
         Assert.assertEquals("Unexpected model index", 0, manager.getIndexForWindow(activity0));
 
-        ApplicationStatus.onStateChangeForTesting(activity0, ActivityState.DESTROYED);
+        destroyActivity(activity0);
 
         Assert.assertEquals("Still found model", TabWindowManager.INVALID_WINDOW_INDEX,
                 manager.getIndexForWindow(activity0));
@@ -235,7 +256,7 @@ public void testActivityDeathWithMultipleActivities() {
         Assert.assertEquals("Unexpected model index", 0, manager.getIndexForWindow(activity0));
         Assert.assertEquals("Unexpected model index", 1, manager.getIndexForWindow(activity1));
 
-        ApplicationStatus.onStateChangeForTesting(activity1, ActivityState.DESTROYED);
+        destroyActivity(activity1);
 
         Assert.assertEquals("Still found model", TabWindowManager.INVALID_WINDOW_INDEX,
                 manager.getIndexForWindow(activity1));

chrome/android/javatests/src/org/chromium/chrome/browser/touchless/NoTouchActivityTest.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.touchless;
 
+import android.os.Build;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
 
@@ -16,6 +17,7 @@
 
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.MinAndroidSdkLevel;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.MockSafeBrowsingApiHandler;
 import org.chromium.chrome.browser.ntp.NewTabPage;
@@ -33,6 +35,8 @@
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
 @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE, ChromeSwitches.NO_TOUCH_MODE})
+// These tests do not have onDestroy called on KitKat, due to what is likely a Android bug.
+@MinAndroidSdkLevel(Build.VERSION_CODES.LOLLIPOP)
 public class NoTouchActivityTest {
     private static final String TEST_PATH = "/chrome/test/data/android/simple.html";
 

chrome/test/android/javatests/src/org/chromium/chrome/test/ChromeActivityTestRule.java

@@ -145,10 +145,6 @@ protected void afterActivityFinished() {
         try {
             ApplicationTestUtils.tearDown(InstrumentationRegistry.getTargetContext());
             Thread.setDefaultUncaughtExceptionHandler(mDefaultUncaughtExceptionHandler);
-            if (mSetActivity != null) {
-                // This is to ensure onDestroy() is performed before starting the next test.
-                ApplicationTestUtils.finishActivity(mSetActivity);
-            }
         } catch (Exception e) {
             throw new RuntimeException("Failed to tearDown", e);
         }