From b30145c2cf249681fceae92bd18cdec076471504 Mon Sep 17 00:00:00 2001 From: Garfield Tan Date: Fri, 2 Aug 2019 17:31:30 -0700 Subject: [PATCH] Fix some issues about AMS creation in tests. In commit ce8db89ce529343262f7f4c669dd2d8976afcff6 we fixed a mem leak issue caused by thread leak. That makes some tests using the constructor in question crash. Therefore instead of trying to restore the original state, convert these tests to use another constructor to avoid the mem leak. Also fixed another NPE when getting resources from null context and null UiHandler for some of these tests. Bug: 138840116 Test: atest FrameworksServicesTests:ActivityManagerServiceTest Test: atest FrameworksServicesTests:ActivityManagerInternalTest Test: atest FrameworksServicesTests:CoreSettingsObserverTest doesn't fail for these reasons. Test: atest FrameworksMockingServicesTests:AppCompactorTest Test: atest FrameworksServicesTests:AppErrorDialogTest Change-Id: Id349a3e1100faa5b4f5033d94c0f892d3f3745bf --- .../server/am/ActivityManagerService.java | 5 -- .../android/server/am/AppCompactorTest.java | 20 +++++++- .../am/ActivityManagerInternalTest.java | 12 ++++- .../server/am/ActivityManagerServiceTest.java | 7 ++- .../android/server/am/AppErrorDialogTest.java | 13 ++++- .../server/am/CoreSettingsObserverTest.java | 9 ++-- .../android/server/am/ServiceThreadRule.java | 51 +++++++++++++++++++ 7 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 services/tests/servicestests/src/com/android/server/am/ServiceThreadRule.java diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 4d43e4a677d95..c66b3fa9afe0d 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -2373,11 +2373,6 @@ public class ActivityManagerService extends IActivityManager.Stub } } - @VisibleForTesting - public ActivityManagerService(Injector injector) { - this(injector, null /* handlerThread */); - } - /** * Provides the basic functionality for activity task related tests when a handler thread is * given to initialize the dependency members. diff --git a/services/tests/mockingservicestests/src/com/android/server/am/AppCompactorTest.java b/services/tests/mockingservicestests/src/com/android/server/am/AppCompactorTest.java index 4a33739a8596b..101bee28025c4 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/AppCompactorTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/AppCompactorTest.java @@ -21,12 +21,17 @@ import static com.android.server.am.AppCompactor.compactActionIntToString; import static com.google.common.truth.Truth.assertThat; +import android.content.Context; import android.os.Handler; import android.os.HandlerThread; +import android.os.Process; import android.platform.test.annotations.Presubmit; import android.provider.DeviceConfig; import android.text.TextUtils; +import androidx.test.platform.app.InstrumentationRegistry; + +import com.android.server.ServiceThread; import com.android.server.appop.AppOpsService; import com.android.server.testables.TestableDeviceConfig; @@ -54,6 +59,8 @@ import java.util.concurrent.TimeUnit; @RunWith(MockitoJUnitRunner.class) public final class AppCompactorTest { + private ServiceThread mThread; + @Mock private AppOpsService mAppOpsService; private AppCompactor mCompactorUnderTest; @@ -70,7 +77,12 @@ public final class AppCompactorTest { mHandlerThread = new HandlerThread(""); mHandlerThread.start(); mHandler = new Handler(mHandlerThread.getLooper()); - ActivityManagerService ams = new ActivityManagerService(new TestInjector()); + + mThread = new ServiceThread("TestServiceThread", Process.THREAD_PRIORITY_DEFAULT, + true /* allowIo */); + mThread.start(); + + ActivityManagerService ams = new ActivityManagerService(new TestInjector(), mThread); mCompactorUnderTest = new AppCompactor(ams, new AppCompactor.PropertyChangedCallbackForTest() { @Override @@ -85,6 +97,7 @@ public final class AppCompactorTest { @After public void tearDown() { mHandlerThread.quit(); + mThread.quit(); mCountDown = null; } @@ -656,5 +669,10 @@ public final class AppCompactorTest { public Handler getUiHandler(ActivityManagerService service) { return mHandler; } + + @Override + public Context getContext() { + return InstrumentationRegistry.getInstrumentation().getContext(); + } } } diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java b/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java index 5dafe077d32e6..d57fd4b467dbf 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerInternalTest.java @@ -18,13 +18,17 @@ package com.android.server.am; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import android.app.ActivityManagerInternal; import android.os.SystemClock; import androidx.test.filters.MediumTest; +import androidx.test.platform.app.InstrumentationRegistry; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -43,6 +47,8 @@ public class ActivityManagerInternalTest { private static final long TEST_PROC_STATE_SEQ2 = 1112; private static final long TEST_PROC_STATE_SEQ3 = 1113; + @Rule public ServiceThreadRule mServiceThreadRule = new ServiceThreadRule(); + @Mock private ActivityManagerService.Injector mMockInjector; private ActivityManagerService mAms; @@ -52,7 +58,11 @@ public class ActivityManagerInternalTest { public void setUp() { MockitoAnnotations.initMocks(this); - mAms = new ActivityManagerService(mMockInjector); + doReturn(InstrumentationRegistry.getInstrumentation().getContext()).when(mMockInjector) + .getContext(); + doReturn(mServiceThreadRule.getThread().getThreadHandler()).when(mMockInjector) + .getUiHandler(any()); + mAms = new ActivityManagerService(mMockInjector, mServiceThreadRule.getThread()); mAmi = mAms.new LocalService(); } diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java index 3df6976a2caef..8a1f0467b5be8 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityManagerServiceTest.java @@ -70,13 +70,14 @@ import androidx.test.filters.FlakyTest; import androidx.test.filters.MediumTest; import androidx.test.filters.SmallTest; -import com.android.server.appop.AppOpsService; import com.android.server.am.ProcessList.IsolatedUidRange; import com.android.server.am.ProcessList.IsolatedUidRangeAllocator; +import com.android.server.appop.AppOpsService; import com.android.server.wm.ActivityTaskManagerService; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.mockito.Mock; import org.mockito.Mockito; @@ -114,6 +115,8 @@ public class ActivityManagerServiceTest { UidRecord.CHANGE_ACTIVE }; + @Rule public ServiceThreadRule mServiceThreadRule = new ServiceThreadRule(); + private Context mContext = getInstrumentation().getTargetContext(); @Mock private AppOpsService mAppOpsService; @Mock private PackageManager mPackageManager; @@ -131,7 +134,7 @@ public class ActivityManagerServiceTest { mHandlerThread.start(); mHandler = new TestHandler(mHandlerThread.getLooper()); mInjector = new TestInjector(); - mAms = new ActivityManagerService(mInjector); + mAms = new ActivityManagerService(mInjector, mServiceThreadRule.getThread()); mAms.mWaitForNetworkTimeoutMs = 2000; mAms.mActivityTaskManager = new ActivityTaskManagerService(mContext); mAms.mActivityTaskManager.initialize(null, null, mHandler.getLooper()); diff --git a/services/tests/servicestests/src/com/android/server/am/AppErrorDialogTest.java b/services/tests/servicestests/src/com/android/server/am/AppErrorDialogTest.java index 1dfce51725f1f..87cc0ffd374d8 100644 --- a/services/tests/servicestests/src/com/android/server/am/AppErrorDialogTest.java +++ b/services/tests/servicestests/src/com/android/server/am/AppErrorDialogTest.java @@ -29,6 +29,7 @@ import com.android.server.appop.AppOpsService; import com.android.server.wm.ActivityTaskManagerService; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import java.io.File; @@ -41,6 +42,9 @@ import java.io.File; @FlakyTest(bugId = 113616538) public class AppErrorDialogTest { + @Rule + public ServiceThreadRule mServiceThreadRule = new ServiceThreadRule(); + private Context mContext; private ActivityManagerService mService; @@ -55,14 +59,19 @@ public class AppErrorDialogTest { @Override public Handler getUiHandler(ActivityManagerService service) { - return null; + return mServiceThreadRule.getThread().getThreadHandler(); } @Override public boolean isNetworkRestrictedForUid(int uid) { return false; } - }); + + @Override + public Context getContext() { + return mContext; + } + }, mServiceThreadRule.getThread()); mService.mActivityTaskManager = new ActivityTaskManagerService(mContext); mService.mActivityTaskManager.initialize(null, null, mContext.getMainLooper()); } diff --git a/services/tests/servicestests/src/com/android/server/am/CoreSettingsObserverTest.java b/services/tests/servicestests/src/com/android/server/am/CoreSettingsObserverTest.java index cbdc6c3faadd6..1eb02add0122d 100644 --- a/services/tests/servicestests/src/com/android/server/am/CoreSettingsObserverTest.java +++ b/services/tests/servicestests/src/com/android/server/am/CoreSettingsObserverTest.java @@ -39,6 +39,7 @@ import com.android.server.appop.AppOpsService; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -61,6 +62,8 @@ public class CoreSettingsObserverTest { private static final float TEST_FLOAT = 3.14f; private static final String TEST_STRING = "testString"; + @Rule public ServiceThreadRule mServiceThreadRule = new ServiceThreadRule(); + private ActivityManagerService mAms; @Mock private Context mContext; @@ -90,7 +93,7 @@ public class CoreSettingsObserverTest { mContentResolver.addProvider(Settings.AUTHORITY, new FakeSettingsProvider()); when(mContext.getContentResolver()).thenReturn(mContentResolver); - mAms = new ActivityManagerService(new TestInjector()); + mAms = new ActivityManagerService(new TestInjector(), mServiceThreadRule.getThread()); mCoreSettingsObserver = new CoreSettingsObserver(mAms); } @@ -157,7 +160,7 @@ public class CoreSettingsObserverTest { private class TestInjector extends Injector { @Override public Context getContext() { - return mContext; + return getInstrumentation().getContext(); } @Override @@ -167,7 +170,7 @@ public class CoreSettingsObserverTest { @Override public Handler getUiHandler(ActivityManagerService service) { - return null; + return mServiceThreadRule.getThread().getThreadHandler(); } } } diff --git a/services/tests/servicestests/src/com/android/server/am/ServiceThreadRule.java b/services/tests/servicestests/src/com/android/server/am/ServiceThreadRule.java new file mode 100644 index 0000000000000..e86ce7e777aba --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/am/ServiceThreadRule.java @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2019 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.server.am; + +import android.os.Process; + +import com.android.server.ServiceThread; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +class ServiceThreadRule implements TestRule { + + private ServiceThread mThread; + + ServiceThread getThread() { + return mThread; + } + + @Override + public Statement apply(Statement base, Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + mThread = new ServiceThread("TestServiceThread", Process.THREAD_PRIORITY_DEFAULT, + true /* allowIo */); + mThread.start(); + try { + base.evaluate(); + } finally { + mThread.getThreadHandler().runWithScissors(mThread::quit, 0 /* timeout */); + } + } + }; + } +}