From 5ade8e890d34e0af036cee5047a7b0857833091f Mon Sep 17 00:00:00 2001 From: vadimt Date: Thu, 23 Sep 2021 18:00:13 -0700 Subject: [PATCH] Moving activity tracker to Launcher process This will improve diagnostics for OOP tests, like we now have a list of leaked activity classes. Also some cleanups. Bug: 187761685 Test: local runs Change-Id: I8b5711ac727874fd826cfef9c742ea97048763e0 --- .../testing/DebugTestInformationHandler.java | 58 ++++++++++++ .../launcher3/testing/TestProtocol.java | 3 + tests/Android.bp | 1 - .../launcher3/ui/AbstractLauncherUiTest.java | 40 ++------- .../launcher3/ui/ActivityLeakTracker.java | 90 ------------------- .../tapl/LauncherInstrumentation.java | 24 +++++ 6 files changed, 93 insertions(+), 123 deletions(-) delete mode 100644 tests/src/com/android/launcher3/ui/ActivityLeakTracker.java diff --git a/ext_tests/src/com/android/launcher3/testing/DebugTestInformationHandler.java b/ext_tests/src/com/android/launcher3/testing/DebugTestInformationHandler.java index dacd8a22ab..0f61d149d0 100644 --- a/ext_tests/src/com/android/launcher3/testing/DebugTestInformationHandler.java +++ b/ext_tests/src/com/android/launcher3/testing/DebugTestInformationHandler.java @@ -18,6 +18,8 @@ package com.android.launcher3.testing; import static com.android.launcher3.util.Executors.MAIN_EXECUTOR; +import android.app.Activity; +import android.app.Application; import android.content.Context; import android.os.Binder; import android.os.Bundle; @@ -31,7 +33,10 @@ import com.android.launcher3.LauncherSettings; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.LinkedList; +import java.util.Map; +import java.util.WeakHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -41,9 +46,48 @@ import java.util.concurrent.TimeUnit; public class DebugTestInformationHandler extends TestInformationHandler { private static LinkedList sLeaks; private static Collection sEvents; + private static Application.ActivityLifecycleCallbacks sActivityLifecycleCallbacks; + private static final Map sActivities = + Collections.synchronizedMap(new WeakHashMap<>()); + private static int sActivitiesCreatedCount = 0; public DebugTestInformationHandler(Context context) { init(context); + if (sActivityLifecycleCallbacks == null) { + sActivityLifecycleCallbacks = new Application.ActivityLifecycleCallbacks() { + @Override + public void onActivityCreated(Activity activity, Bundle bundle) { + sActivities.put(activity, true); + ++sActivitiesCreatedCount; + } + + @Override + public void onActivityStarted(Activity activity) { + } + + @Override + public void onActivityResumed(Activity activity) { + } + + @Override + public void onActivityPaused(Activity activity) { + } + + @Override + public void onActivityStopped(Activity activity) { + } + + @Override + public void onActivitySaveInstanceState(Activity activity, Bundle bundle) { + } + + @Override + public void onActivityDestroyed(Activity activity) { + } + }; + ((Application) context.getApplicationContext()) + .registerActivityLifecycleCallbacks(sActivityLifecycleCallbacks); + } } private static void runGcAndFinalizersSync() { @@ -160,6 +204,20 @@ public class DebugTestInformationHandler extends TestInformationHandler { } } + case TestProtocol.REQUEST_GET_ACTIVITIES_CREATED_COUNT: { + response.putInt(TestProtocol.TEST_INFO_RESPONSE_FIELD, sActivitiesCreatedCount); + return response; + } + + case TestProtocol.REQUEST_GET_ACTIVITIES: { + response.putStringArray(TestProtocol.TEST_INFO_RESPONSE_FIELD, + sActivities.keySet().stream().map( + a -> a.getClass().getSimpleName() + " (" + + (a.isDestroyed() ? "destroyed" : "current") + ")") + .toArray(String[]::new)); + return response; + } + default: return super.call(method, arg); } diff --git a/src/com/android/launcher3/testing/TestProtocol.java b/src/com/android/launcher3/testing/TestProtocol.java index ed52e20a34..b25fefb8aa 100644 --- a/src/com/android/launcher3/testing/TestProtocol.java +++ b/src/com/android/launcher3/testing/TestProtocol.java @@ -99,6 +99,9 @@ public final class TestProtocol { public static final String REQUEST_CLEAR_DATA = "clear-data"; public static final String REQUEST_IS_TABLET = "is-tablet"; public static final String REQUEST_IS_TWO_PANELS = "is-two-panel"; + public static final String REQUEST_GET_ACTIVITIES_CREATED_COUNT = + "get-activities-created-count"; + public static final String REQUEST_GET_ACTIVITIES = "get-activities"; public static Long sForcePauseTimeout; public static final String REQUEST_SET_FORCE_PAUSE_TIMEOUT = "set-force-pause-timeout"; diff --git a/tests/Android.bp b/tests/Android.bp index aeddc4cbaf..3670c37add 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -31,7 +31,6 @@ filegroup { name: "launcher-oop-tests-src", srcs: [ "src/com/android/launcher3/ui/AbstractLauncherUiTest.java", - "src/com/android/launcher3/ui/ActivityLeakTracker.java", "src/com/android/launcher3/ui/PortraitLandscapeRunner.java", "src/com/android/launcher3/util/Wait.java", "src/com/android/launcher3/util/WidgetUtils.java", diff --git a/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java b/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java index 1a6ce8ccae..b6b6cdd5b7 100644 --- a/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java +++ b/tests/src/com/android/launcher3/ui/AbstractLauncherUiTest.java @@ -36,7 +36,6 @@ import android.content.pm.PackageManager; import android.os.Debug; import android.os.Process; import android.os.RemoteException; -import android.os.StrictMode; import android.os.UserHandle; import android.os.UserManager; import android.util.Log; @@ -99,11 +98,9 @@ public abstract class AbstractLauncherUiTest { public static final long DEFAULT_UI_TIMEOUT = 10000; private static final String TAG = "AbstractLauncherUiTest"; - private static String sStrictmodeDetectedActivityLeak; private static boolean sDumpWasGenerated = false; - private static boolean sActivityLeakReported; + private static boolean sActivityLeakReported = false; private static final String SYSTEMUI_PACKAGE = "com.android.systemui"; - protected static final ActivityLeakTracker ACTIVITY_LEAK_TRACKER = new ActivityLeakTracker(); protected LooperExecutor mMainThreadExecutor = MAIN_EXECUTOR; protected final UiDevice mDevice = UiDevice.getInstance(getInstrumentation()); @@ -112,45 +109,25 @@ public abstract class AbstractLauncherUiTest { protected String mTargetPackage; private int mLauncherPid; - static { - if (TestHelpers.isInLauncherProcess()) { - StrictMode.VmPolicy.Builder builder = - new StrictMode.VmPolicy.Builder() - .penaltyLog() - .penaltyListener(Runnable::run, violation -> { - if (sStrictmodeDetectedActivityLeak == null) { - sStrictmodeDetectedActivityLeak = violation.toString() + ", " - + dumpHprofData() + "."; - } - }); - StrictMode.setVmPolicy(builder.build()); - } - } - public static void checkDetectedLeaks(LauncherInstrumentation launcher) { if (sActivityLeakReported) return; - if (sStrictmodeDetectedActivityLeak != null) { - // Report from the test thread strictmode violations detected in the main thread. - sActivityLeakReported = true; - Assert.fail(sStrictmodeDetectedActivityLeak); - } - // Check whether activity leak detector has found leaked activities. - Wait.atMost(AbstractLauncherUiTest::getActivityLeakErrorMessage, + Wait.atMost(() -> getActivityLeakErrorMessage(launcher), () -> { launcher.forceGc(); return MAIN_EXECUTOR.submit( - () -> ACTIVITY_LEAK_TRACKER.noLeakedActivities()).get(); + () -> launcher.noLeakedActivities()).get(); }, DEFAULT_UI_TIMEOUT, launcher); } - private static String getActivityLeakErrorMessage() { + private static String getActivityLeakErrorMessage(LauncherInstrumentation launcher) { sActivityLeakReported = true; - return "Activity leak detector has found leaked activities, " + dumpHprofData() + "."; + return "Activity leak detector has found leaked activities, " + + dumpHprofData(launcher) + "."; } - public static String dumpHprofData() { + public static String dumpHprofData(LauncherInstrumentation launcher) { String result; if (sDumpWasGenerated) { Log.d("b/195319692", "dump has already been generated by another test", @@ -176,8 +153,7 @@ public abstract class AbstractLauncherUiTest { result = "failed to save memory dump"; } } - return result - + ". Full list of activities: " + ACTIVITY_LEAK_TRACKER.getActivitiesList(); + return result + ". Full list of activities: " + launcher.getRootedActivitiesList(); } protected AbstractLauncherUiTest() { diff --git a/tests/src/com/android/launcher3/ui/ActivityLeakTracker.java b/tests/src/com/android/launcher3/ui/ActivityLeakTracker.java deleted file mode 100644 index 2db7472912..0000000000 --- a/tests/src/com/android/launcher3/ui/ActivityLeakTracker.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.launcher3.ui; - -import android.app.Activity; -import android.app.Application; -import android.os.Bundle; - -import androidx.test.InstrumentationRegistry; - -import com.android.launcher3.tapl.TestHelpers; - -import java.util.WeakHashMap; -import java.util.stream.Collectors; - -public class ActivityLeakTracker implements Application.ActivityLifecycleCallbacks { - private final WeakHashMap mActivities = new WeakHashMap<>(); - - private int mActivitiesCreated; - - ActivityLeakTracker() { - if (!TestHelpers.isInLauncherProcess()) return; - final Application app = - (Application) InstrumentationRegistry.getTargetContext().getApplicationContext(); - app.registerActivityLifecycleCallbacks(this); - } - - public int getActivitiesCreated() { - return mActivitiesCreated; - } - - @Override - public void onActivityCreated(Activity activity, Bundle bundle) { - mActivities.put(activity, true); - ++mActivitiesCreated; - } - - @Override - public void onActivityStarted(Activity activity) { - } - - @Override - public void onActivityResumed(Activity activity) { - } - - @Override - public void onActivityPaused(Activity activity) { - } - - @Override - public void onActivityStopped(Activity activity) { - } - - @Override - public void onActivitySaveInstanceState(Activity activity, Bundle bundle) { - } - - @Override - public void onActivityDestroyed(Activity activity) { - } - - public boolean noLeakedActivities() { - for (Activity activity : mActivities.keySet()) { - if (activity.isDestroyed()) { - return false; - } - } - - return mActivities.size() <= 2; - } - - public String getActivitiesList() { - return mActivities.keySet().stream().map(a -> a.getClass().getSimpleName()) - .collect(Collectors.joining(",")); - } -} diff --git a/tests/tapl/com/android/launcher3/tapl/LauncherInstrumentation.java b/tests/tapl/com/android/launcher3/tapl/LauncherInstrumentation.java index 98d081b70a..cb48fe6bd0 100644 --- a/tests/tapl/com/android/launcher3/tapl/LauncherInstrumentation.java +++ b/tests/tapl/com/android/launcher3/tapl/LauncherInstrumentation.java @@ -1498,6 +1498,30 @@ public final class LauncherInstrumentation { getTestInfo(TestProtocol.REQUEST_CLEAR_DATA); } + private String[] getActivities() { + return getTestInfo(TestProtocol.REQUEST_GET_ACTIVITIES) + .getStringArray(TestProtocol.TEST_INFO_RESPONSE_FIELD); + } + + public String getRootedActivitiesList() { + return String.join(", ", getActivities()); + } + + public boolean noLeakedActivities() { + final String[] activities = getActivities(); + for (String activity : activities) { + if (activity.contains("(destroyed)")) { + return false; + } + } + return activities.length <= 2; + } + + public int getActivitiesCreated() { + return getTestInfo(TestProtocol.REQUEST_GET_ACTIVITIES_CREATED_COUNT) + .getInt(TestProtocol.TEST_INFO_RESPONSE_FIELD); + } + public Closable eventsCheck() { Assert.assertTrue("Nested event checking", mEventChecker == null); disableSensorRotation();