From 3150dbf7a57c7f0332db9c9197ad046023bd6ece Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Wed, 28 Mar 2018 18:06:52 +0200 Subject: [PATCH] WM Tests: Use a separate WindowManager instance per test Fixes a bunch of flakes, where the WindowManagerService instance was reused between tests, which caused delayed callbacks from a previous test affecting state of a future test. Also introduces a DexmakerShareClassLoaderRule to manage the 'dexmaker.share_classloader' property instead of sprinkling error prone System.setProperty() invocations all over the tests. Change-Id: Ic9445d1b2cef594e79365c425632aabced6343a9 Fixes: 76111404 Fixes: 75991352 Fixes: 75991878 Fixes: 75992153 Test: atest services/tests/servicestests DexmakerShareClassLoaderRuleTest packages/SystemUI/tests packages/SystemUI/shared/tests --- .../systemui/SysuiBaseFragmentTest.java | 6 +- .../com/android/systemui/SysuiTestCase.java | 9 +- .../am/ActivityStartInterceptorTest.java | 9 +- .../android/server/am/ActivityTestsBase.java | 11 +- .../server/am/LockTaskControllerTest.java | 9 +- .../android/server/am/UserControllerTest.java | 17 +- .../android/server/wm/AppTransitionTests.java | 5 +- .../server/wm/TestWindowManagerPolicy.java | 83 ++-------- .../server/wm/WindowManagerServiceRule.java | 151 ++++++++++++++++++ .../wm/WindowManagerServiceRuleTest.java | 42 +++++ .../android/server/wm/WindowTestUtils.java | 7 - .../android/server/wm/WindowTestsBase.java | 54 ++++--- .../testing/DexmakerShareClassLoaderRule.java | 129 +++++++++++++++ .../DexmakerShareClassLoaderRuleTest.java | 118 ++++++++++++++ 14 files changed, 524 insertions(+), 126 deletions(-) create mode 100644 services/tests/servicestests/src/com/android/server/wm/WindowManagerServiceRule.java create mode 100644 services/tests/servicestests/src/com/android/server/wm/WindowManagerServiceRuleTest.java create mode 100644 tests/testables/src/android/testing/DexmakerShareClassLoaderRule.java create mode 100644 tests/testables/tests/src/android/testing/DexmakerShareClassLoaderRuleTest.java diff --git a/packages/SystemUI/tests/src/com/android/systemui/SysuiBaseFragmentTest.java b/packages/SystemUI/tests/src/com/android/systemui/SysuiBaseFragmentTest.java index 8905d725de974..b6335f3681830 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/SysuiBaseFragmentTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/SysuiBaseFragmentTest.java @@ -21,6 +21,7 @@ import android.app.Fragment; import android.app.Instrumentation; import android.support.test.InstrumentationRegistry; import android.testing.BaseFragmentTest; +import android.testing.DexmakerShareClassLoaderRule; import com.android.systemui.utils.leaks.LeakCheckedTest; import com.android.systemui.utils.leaks.LeakCheckedTest.SysuiLeakCheck; @@ -36,6 +37,10 @@ public abstract class SysuiBaseFragmentTest extends BaseFragmentTest { @Rule public final SysuiLeakCheck mLeakCheck = new SysuiLeakCheck(); + @Rule + public final DexmakerShareClassLoaderRule mDexmakerShareClassLoaderRule = + new DexmakerShareClassLoaderRule(); + protected final TestableDependency mDependency = new TestableDependency(mContext); protected SysuiTestableContext mSysuiContext; private Instrumentation mRealInstrumentation; @@ -46,7 +51,6 @@ public abstract class SysuiBaseFragmentTest extends BaseFragmentTest { @Before public void SysuiSetup() { - System.setProperty("dexmaker.share_classloader", "true"); SystemUIFactory.createFromConfig(mContext); // TODO: Figure out another way to give reference to a SysuiTestableContext. mSysuiContext = (SysuiTestableContext) mContext; diff --git a/packages/SystemUI/tests/src/com/android/systemui/SysuiTestCase.java b/packages/SystemUI/tests/src/com/android/systemui/SysuiTestCase.java index 4c7c1d1608fde..7475cd7e1e360 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/SysuiTestCase.java +++ b/packages/SystemUI/tests/src/com/android/systemui/SysuiTestCase.java @@ -19,21 +19,18 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; import android.app.Instrumentation; -import android.content.Context; import android.os.Handler; import android.os.Looper; import android.os.MessageQueue; import android.os.ParcelFileDescriptor; import android.support.test.InstrumentationRegistry; -import android.support.test.filters.SmallTest; +import android.testing.DexmakerShareClassLoaderRule; import android.testing.LeakCheck; import android.util.Log; import org.junit.After; import org.junit.Before; import org.junit.Rule; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.io.FileInputStream; import java.io.IOException; @@ -51,12 +48,14 @@ public abstract class SysuiTestCase { @Rule public SysuiTestableContext mContext = new SysuiTestableContext( InstrumentationRegistry.getContext(), getLeakCheck()); + @Rule + public final DexmakerShareClassLoaderRule mDexmakerShareClassLoaderRule = + new DexmakerShareClassLoaderRule(); public TestableDependency mDependency = new TestableDependency(mContext); private Instrumentation mRealInstrumentation; @Before public void SysuiSetup() throws Exception { - System.setProperty("dexmaker.share_classloader", "true"); mContext.setTheme(R.style.Theme_SystemUI); SystemUIFactory.createFromConfig(mContext); diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityStartInterceptorTest.java b/services/tests/servicestests/src/com/android/server/am/ActivityStartInterceptorTest.java index 194f4ae51a62b..f76eb5659a11a 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityStartInterceptorTest.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityStartInterceptorTest.java @@ -34,11 +34,13 @@ import android.content.pm.UserInfo; import android.os.UserHandle; import android.os.UserManager; import android.support.test.filters.SmallTest; +import android.testing.DexmakerShareClassLoaderRule; import com.android.internal.app.UnlaunchableAppActivity; import com.android.server.LocalServices; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -64,6 +66,10 @@ public class ActivityStartInterceptorTest { 0 /* flags */); private static final String TEST_PACKAGE_NAME = "com.test.package"; + @Rule + public final DexmakerShareClassLoaderRule mDexmakerShareClassLoaderRule = + new DexmakerShareClassLoaderRule(); + @Mock private Context mContext; @Mock @@ -84,9 +90,6 @@ public class ActivityStartInterceptorTest { @Before public void setUp() { - // This property is used to allow mocking of package private classes with mockito - System.setProperty("dexmaker.share_classloader", "true"); - MockitoAnnotations.initMocks(this); mInterceptor = new ActivityStartInterceptor(mService, mSupervisor, mContext, mUserController); diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityTestsBase.java b/services/tests/servicestests/src/com/android/server/am/ActivityTestsBase.java index 6fb1b2e21c47f..ab2916b99885e 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityTestsBase.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityTestsBase.java @@ -31,6 +31,8 @@ import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.spy; import com.android.server.wm.DisplayWindowController; + +import org.junit.Rule; import org.mockito.invocation.InvocationOnMock; import android.app.IApplicationThread; @@ -47,6 +49,8 @@ import android.os.HandlerThread; import android.os.Looper; import android.service.voice.IVoiceInteractionSession; import android.support.test.InstrumentationRegistry; +import android.testing.DexmakerShareClassLoaderRule; + import com.android.server.AttributeCache; import com.android.server.wm.AppWindowContainerController; import com.android.server.wm.PinnedStackWindowController; @@ -64,6 +68,10 @@ import org.mockito.MockitoAnnotations; public class ActivityTestsBase { private static boolean sOneTimeSetupDone = false; + @Rule + public final DexmakerShareClassLoaderRule mDexmakerShareClassLoaderRule = + new DexmakerShareClassLoaderRule(); + private final Context mContext = InstrumentationRegistry.getContext(); private HandlerThread mHandlerThread; @@ -77,9 +85,6 @@ public class ActivityTestsBase { public void setUp() throws Exception { if (!sOneTimeSetupDone) { sOneTimeSetupDone = true; - - // Allows to mock package local classes and methods - System.setProperty("dexmaker.share_classloader", "true"); MockitoAnnotations.initMocks(this); } mHandlerThread = new HandlerThread("ActivityTestsBaseThread"); diff --git a/services/tests/servicestests/src/com/android/server/am/LockTaskControllerTest.java b/services/tests/servicestests/src/com/android/server/am/LockTaskControllerTest.java index 1cbb3991e4618..f46d712df65b2 100644 --- a/services/tests/servicestests/src/com/android/server/am/LockTaskControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/LockTaskControllerTest.java @@ -55,6 +55,7 @@ import android.provider.Settings; import android.support.test.InstrumentationRegistry; import android.support.test.filters.SmallTest; import android.telecom.TelecomManager; +import android.testing.DexmakerShareClassLoaderRule; import android.util.Pair; import com.android.internal.statusbar.IStatusBarService; @@ -65,6 +66,7 @@ import com.android.server.wm.WindowManagerService; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -85,6 +87,10 @@ public class LockTaskControllerTest { private static final int TEST_USER_ID = 123; private static final int TEST_UID = 10467; + @Rule + public final DexmakerShareClassLoaderRule mDexmakerShareClassLoaderRule = + new DexmakerShareClassLoaderRule(); + @Mock private ActivityStackSupervisor mSupervisor; @Mock private IDevicePolicyManager mDevicePolicyManager; @Mock private IStatusBarService mStatusBarService; @@ -100,9 +106,6 @@ public class LockTaskControllerTest { @Before public void setUp() throws Exception { - // This property is used to allow mocking of package private classes with mockito - System.setProperty("dexmaker.share_classloader", "true"); - MockitoAnnotations.initMocks(this); mContext = InstrumentationRegistry.getTargetContext(); diff --git a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java index 3c10b0394ff92..cc4f51987523f 100644 --- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java @@ -49,6 +49,7 @@ import java.util.List; import java.util.Set; import static android.content.pm.PackageManager.PERMISSION_GRANTED; +import static android.testing.DexmakerShareClassLoaderRule.runWithDexmakerShareClassLoader; import static com.android.server.am.UserController.CONTINUE_USER_SWITCH_MSG; import static com.android.server.am.UserController.REPORT_LOCKED_BOOT_COMPLETE_MSG; import static com.android.server.am.UserController.REPORT_USER_SWITCH_COMPLETE_MSG; @@ -102,13 +103,15 @@ public class UserControllerTest extends AndroidTestCase { @Override public void setUp() throws Exception { super.setUp(); - mInjector = Mockito.spy(new TestInjector(getContext())); - doNothing().when(mInjector).clearAllLockedTasks(anyString()); - doNothing().when(mInjector).startHomeActivity(anyInt(), anyString()); - doReturn(false).when(mInjector).stackSupervisorSwitchUser(anyInt(), any()); - doNothing().when(mInjector).stackSupervisorResumeFocusedStackTopActivity(); - mUserController = new UserController(mInjector); - setUpUser(TEST_USER_ID, 0); + runWithDexmakerShareClassLoader(() -> { + mInjector = Mockito.spy(new TestInjector(getContext())); + doNothing().when(mInjector).clearAllLockedTasks(anyString()); + doNothing().when(mInjector).startHomeActivity(anyInt(), anyString()); + doReturn(false).when(mInjector).stackSupervisorSwitchUser(anyInt(), any()); + doNothing().when(mInjector).stackSupervisorResumeFocusedStackTopActivity(); + mUserController = new UserController(mInjector); + setUpUser(TEST_USER_ID, 0); + }); } @Override diff --git a/services/tests/servicestests/src/com/android/server/wm/AppTransitionTests.java b/services/tests/servicestests/src/com/android/server/wm/AppTransitionTests.java index e7e55cd4404e2..8e5aec399bb56 100644 --- a/services/tests/servicestests/src/com/android/server/wm/AppTransitionTests.java +++ b/services/tests/servicestests/src/com/android/server/wm/AppTransitionTests.java @@ -28,6 +28,7 @@ import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,12 +42,14 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) public class AppTransitionTests { + @Rule + public final WindowManagerServiceRule mRule = new WindowManagerServiceRule(); private WindowManagerService mWm; @Before public void setUp() throws Exception { final Context context = InstrumentationRegistry.getTargetContext(); - mWm = TestWindowManagerPolicy.getWindowManagerService(context); + mWm = mRule.getWindowManagerService(); } @Test diff --git a/services/tests/servicestests/src/com/android/server/wm/TestWindowManagerPolicy.java b/services/tests/servicestests/src/com/android/server/wm/TestWindowManagerPolicy.java index 6d9167fda5852..f129566f2cf6a 100644 --- a/services/tests/servicestests/src/com/android/server/wm/TestWindowManagerPolicy.java +++ b/services/tests/servicestests/src/com/android/server/wm/TestWindowManagerPolicy.java @@ -16,108 +16,47 @@ package com.android.server.wm; -import static android.view.Display.DEFAULT_DISPLAY; import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_STARTING; import static android.view.WindowManager.LayoutParams.TYPE_STATUS_BAR; -import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM; - -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Matchers.anyInt; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.spy; - -import android.os.PowerSaveState; -import android.util.proto.ProtoOutputStream; -import org.mockito.invocation.InvocationOnMock; import android.annotation.Nullable; -import android.app.ActivityManagerInternal; import android.content.Context; import android.content.res.CompatibilityInfo; import android.content.res.Configuration; import android.graphics.Rect; -import android.hardware.display.DisplayManagerInternal; import android.os.Bundle; import android.os.IBinder; import android.os.RemoteException; +import android.util.proto.ProtoOutputStream; import android.view.Display; import android.view.DisplayCutout; +import android.view.IWindow; import android.view.IWindowManager; -import android.view.InputChannel; import android.view.KeyEvent; import android.view.WindowManager; import android.view.animation.Animation; -import android.os.PowerManagerInternal; import com.android.internal.policy.IKeyguardDismissCallback; import com.android.internal.policy.IShortcutService; -import com.android.server.input.InputManagerService; -import com.android.server.LocalServices; import com.android.server.policy.WindowManagerPolicy; import java.io.PrintWriter; +import java.util.function.Supplier; class TestWindowManagerPolicy implements WindowManagerPolicy { private static final String TAG = "TestWindowManagerPolicy"; - private static WindowManagerService sWm = null; + private final Supplier mWmSupplier; int rotationToReport = 0; private Runnable mRunnableWhenAddingSplashScreen; - static synchronized WindowManagerService getWindowManagerService(Context context) { - if (sWm == null) { - // We only want to do this once for the test process as we don't want WM to try to - // register a bunch of local services again. - if (LocalServices.getService(DisplayManagerInternal.class) == null) { - LocalServices.addService(DisplayManagerInternal.class, - mock(DisplayManagerInternal.class)); - } - if (LocalServices.getService(PowerManagerInternal.class) == null) { - LocalServices.addService(PowerManagerInternal.class, - mock(PowerManagerInternal.class)); - final PowerManagerInternal pm = - LocalServices.getService(PowerManagerInternal.class); - PowerSaveState state = new PowerSaveState.Builder().build(); - doReturn(state).when(pm).getLowPowerState(anyInt()); - } - if (LocalServices.getService(ActivityManagerInternal.class) == null) { - LocalServices.addService(ActivityManagerInternal.class, - mock(ActivityManagerInternal.class)); - final ActivityManagerInternal am = - LocalServices.getService(ActivityManagerInternal.class); - doAnswer((InvocationOnMock invocationOnMock) -> { - final Runnable runnable = invocationOnMock.getArgument(0); - if (runnable != null) { - runnable.run(); - } - return null; - }).when(am).notifyKeyguardFlagsChanged(any()); - } + public TestWindowManagerPolicy(Supplier wmSupplier) { - InputManagerService ims = mock(InputManagerService.class); - // InputChannel is final and can't be mocked. - InputChannel[] input = InputChannel.openInputChannelPair(TAG_WM); - if (input != null && input.length > 1) { - doReturn(input[1]).when(ims).monitorInput(anyString()); - } - - sWm = WindowManagerService.main(context, ims, true, false, - false, new TestWindowManagerPolicy()); - - sWm.onInitReady(); - - // Display creation is driven by the ActivityManagerService via ActivityStackSupervisor. - // We emulate those steps here. - sWm.mRoot.createDisplayContent(sWm.mDisplayManager.getDisplay(DEFAULT_DISPLAY), - mock(DisplayWindowController.class)); - } - return sWm; + mWmSupplier = wmSupplier; } @Override @@ -216,10 +155,12 @@ class TestWindowManagerPolicy implements WindowManagerPolicy { int logo, int windowFlags, Configuration overrideConfig, int displayId) { final com.android.server.wm.WindowState window; final AppWindowToken atoken; - synchronized (sWm.mWindowMap) { - atoken = sWm.mRoot.getAppWindowToken(appToken); + final WindowManagerService wm = mWmSupplier.get(); + synchronized (wm.mWindowMap) { + atoken = wm.mRoot.getAppWindowToken(appToken); window = WindowTestsBase.createWindow(null, TYPE_APPLICATION_STARTING, atoken, - "Starting window"); + "Starting window", 0 /* ownerId */, false /* internalWindows */, wm, + mock(Session.class), mock(IWindow.class)); atoken.startingWindow = window; } if (mRunnableWhenAddingSplashScreen != null) { @@ -227,7 +168,7 @@ class TestWindowManagerPolicy implements WindowManagerPolicy { mRunnableWhenAddingSplashScreen = null; } return () -> { - synchronized (sWm.mWindowMap) { + synchronized (wm.mWindowMap) { atoken.removeChild(window); atoken.startingWindow = null; } diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowManagerServiceRule.java b/services/tests/servicestests/src/com/android/server/wm/WindowManagerServiceRule.java new file mode 100644 index 0000000000000..32bfda32de7fb --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/wm/WindowManagerServiceRule.java @@ -0,0 +1,151 @@ +/* + * Copyright (C) 2018 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.wm; + +import static android.testing.DexmakerShareClassLoaderRule.runWithDexmakerShareClassLoader; +import static android.view.Display.DEFAULT_DISPLAY; + +import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +import android.app.ActivityManagerInternal; +import android.content.Context; +import android.hardware.display.DisplayManagerInternal; +import android.os.PowerManagerInternal; +import android.os.PowerSaveState; +import android.support.test.InstrumentationRegistry; +import android.view.InputChannel; + +import com.android.server.LocalServices; +import com.android.server.input.InputManagerService; +import com.android.server.policy.WindowManagerPolicy; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; +import org.mockito.invocation.InvocationOnMock; + +/** + * A test rule that sets up a fresh WindowManagerService instance before each test and makes sure + * to properly tear it down after. + * + *

+ * Usage: + * 

+ * {@literal @}Rule
+ *  public final WindowManagerServiceRule mWmRule = new WindowManagerServiceRule();
+ *
+ */ +public class WindowManagerServiceRule implements TestRule { + + private WindowManagerService mService; + + @Override + public Statement apply(Statement base, Description description) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + runWithDexmakerShareClassLoader(this::setUp); + try { + base.evaluate(); + } finally { + tearDown(); + } + } + + private void setUp() { + final Context context = InstrumentationRegistry.getTargetContext(); + + removeServices(); + + LocalServices.addService(DisplayManagerInternal.class, + mock(DisplayManagerInternal.class)); + + LocalServices.addService(PowerManagerInternal.class, + mock(PowerManagerInternal.class)); + final PowerManagerInternal pm = + LocalServices.getService(PowerManagerInternal.class); + PowerSaveState state = new PowerSaveState.Builder().build(); + doReturn(state).when(pm).getLowPowerState(anyInt()); + + LocalServices.addService(ActivityManagerInternal.class, + mock(ActivityManagerInternal.class)); + final ActivityManagerInternal am = + LocalServices.getService(ActivityManagerInternal.class); + doAnswer((InvocationOnMock invocationOnMock) -> { + final Runnable runnable = invocationOnMock.getArgument(0); + if (runnable != null) { + runnable.run(); + } + return null; + }).when(am).notifyKeyguardFlagsChanged(any()); + + InputManagerService ims = mock(InputManagerService.class); + // InputChannel is final and can't be mocked. + InputChannel[] input = InputChannel.openInputChannelPair(TAG_WM); + if (input != null && input.length > 1) { + doReturn(input[1]).when(ims).monitorInput(anyString()); + } + + mService = WindowManagerService.main(context, ims, true, false, + false, new TestWindowManagerPolicy( + WindowManagerServiceRule.this::getWindowManagerService)); + + mService.onInitReady(); + + // Display creation is driven by the ActivityManagerService via ActivityStackSupervisor. + // We emulate those steps here. + mService.mRoot.createDisplayContent( + mService.mDisplayManager.getDisplay(DEFAULT_DISPLAY), + mock(DisplayWindowController.class)); + } + + private void removeServices() { + LocalServices.removeServiceForTest(DisplayManagerInternal.class); + LocalServices.removeServiceForTest(PowerManagerInternal.class); + LocalServices.removeServiceForTest(ActivityManagerInternal.class); + LocalServices.removeServiceForTest(WindowManagerInternal.class); + LocalServices.removeServiceForTest(WindowManagerPolicy.class); + } + + private void tearDown() { + waitUntilWindowManagerHandlersIdle(); + removeServices(); + mService = null; + } + }; + } + + public WindowManagerService getWindowManagerService() { + return mService; + } + + public void waitUntilWindowManagerHandlersIdle() { + final WindowManagerService wm = getWindowManagerService(); + if (wm != null) { + wm.mH.runWithScissors(() -> { }, 0); + wm.mAnimationHandler.runWithScissors(() -> { }, 0); + SurfaceAnimationThread.getHandler().runWithScissors(() -> { }, 0); + } + } +} diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowManagerServiceRuleTest.java b/services/tests/servicestests/src/com/android/server/wm/WindowManagerServiceRuleTest.java new file mode 100644 index 0000000000000..6cf6d7bc96ec4 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/wm/WindowManagerServiceRuleTest.java @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2018 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.wm; + +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.junit.Assert.assertThat; + +import android.platform.test.annotations.Presubmit; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +@Presubmit +@SmallTest +public class WindowManagerServiceRuleTest { + + @Rule + public final WindowManagerServiceRule mRule = new WindowManagerServiceRule(); + + @Test + public void testWindowManagerSetUp() { + assertThat(mRule.getWindowManagerService(), notNullValue()); + } +} \ No newline at end of file diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowTestUtils.java b/services/tests/servicestests/src/com/android/server/wm/WindowTestUtils.java index f4cdc90dd959b..d74defcbeb7c2 100644 --- a/services/tests/servicestests/src/com/android/server/wm/WindowTestUtils.java +++ b/services/tests/servicestests/src/com/android/server/wm/WindowTestUtils.java @@ -47,13 +47,6 @@ import org.mockito.invocation.InvocationOnMock; public class WindowTestUtils { public static int sNextTaskId = 0; - /** - * Retrieves an instance of {@link WindowManagerService}, creating it if necessary. - */ - public static WindowManagerService getWindowManagerService(Context context) { - return TestWindowManagerPolicy.getWindowManagerService(context); - } - /** * Retrieves an instance of a mock {@link WindowManagerService}. */ diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java index d2eee682a64ca..473a287e3d9cb 100644 --- a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java +++ b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java @@ -24,14 +24,14 @@ import static android.view.View.VISIBLE; import android.content.res.Configuration; import android.graphics.Rect; import android.hardware.display.DisplayManagerGlobal; +import android.testing.DexmakerShareClassLoaderRule; import android.util.Log; import android.view.Display; import android.view.DisplayInfo; import org.junit.Assert; import org.junit.After; import org.junit.Before; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; +import org.junit.Rule; import android.content.Context; import android.support.test.InstrumentationRegistry; @@ -66,16 +66,15 @@ import java.util.LinkedList; */ class WindowTestsBase { private static final String TAG = WindowTestsBase.class.getSimpleName(); - static WindowManagerService sWm = null; - private static final IWindow sIWindow = new TestIWindow(); - private static Session sMockSession; + WindowManagerService sWm = null; // TODO(roosa): rename to mWm in follow-up CL + private final IWindow mIWindow = new TestIWindow(); + private Session mMockSession; // The default display is removed in {@link #setUp} and then we iterate over all displays to // make sure we don't collide with any existing display. If we run into no other display, the // added display should be treated as default. This cannot be the default display private static int sNextDisplayId = DEFAULT_DISPLAY + 1; static int sNextStackId = 1000; - private static boolean sOneTimeSetupDone = false; DisplayContent mDisplayContent; DisplayInfo mDisplayInfo = new DisplayInfo(); WindowState mWallpaperWindow; @@ -90,27 +89,27 @@ class WindowTestsBase { HashSet mCommonWindows; WallpaperController mWallpaperController; - @Mock - static WindowState.PowerManagerWrapper mPowerManagerWrapper; + @Rule + public final DexmakerShareClassLoaderRule mDexmakerShareClassLoaderRule = + new DexmakerShareClassLoaderRule(); + + @Rule + public final WindowManagerServiceRule mWmRule = new WindowManagerServiceRule(); + + static WindowState.PowerManagerWrapper mPowerManagerWrapper; // TODO(roosa): make non-static. @Before public void setUp() throws Exception { // If @Before throws an exception, the error isn't logged. This will make sure any failures // in the set up are clear. This can be removed when b/37850063 is fixed. try { - if (!sOneTimeSetupDone) { - sOneTimeSetupDone = true; - - // Allows to mock package local classes and methods - System.setProperty("dexmaker.share_classloader", "true"); - MockitoAnnotations.initMocks(this); - sMockSession = mock(Session.class); - } + mMockSession = mock(Session.class); + mPowerManagerWrapper = mock(WindowState.PowerManagerWrapper.class); final Context context = InstrumentationRegistry.getTargetContext(); AttributeCache.init(context); - sWm = TestWindowManagerPolicy.getWindowManagerService(context); + sWm = mWmRule.getWindowManagerService(); beforeCreateDisplay(); mWallpaperController = new WallpaperController(sWm); @@ -217,9 +216,7 @@ class WindowTestsBase { * Waits until the main handler for WM has processed all messages. */ void waitUntilHandlersIdle() { - sWm.mH.runWithScissors(() -> { }, 0); - sWm.mAnimationHandler.runWithScissors(() -> { }, 0); - SurfaceAnimationThread.getHandler().runWithScissors(() -> { }, 0); + mWmRule.waitUntilWindowManagerHandlersIdle(); } private WindowToken createWindowToken( @@ -285,20 +282,27 @@ class WindowTestsBase { } } - static WindowState createWindow(WindowState parent, int type, WindowToken token, String name) { + WindowState createWindow(WindowState parent, int type, WindowToken token, String name) { synchronized (sWm.mWindowMap) { return createWindow(parent, type, token, name, 0 /* ownerId */, false /* ownerCanAddInternalSystemWindow */); } } - static WindowState createWindow(WindowState parent, int type, WindowToken token, String name, + WindowState createWindow(WindowState parent, int type, WindowToken token, String name, int ownerId, boolean ownerCanAddInternalSystemWindow) { - synchronized (sWm.mWindowMap) { + return createWindow(parent, type, token, name, ownerId, ownerCanAddInternalSystemWindow, + sWm, mMockSession, mIWindow); + } + + static WindowState createWindow(WindowState parent, int type, WindowToken token, + String name, int ownerId, boolean ownerCanAddInternalSystemWindow, + WindowManagerService service, Session session, IWindow iWindow) { + synchronized (service.mWindowMap) { final WindowManager.LayoutParams attrs = new WindowManager.LayoutParams(type); attrs.setTitle(name); - final WindowState w = new WindowState(sWm, sMockSession, sIWindow, token, parent, + final WindowState w = new WindowState(service, session, iWindow, token, parent, OP_NONE, 0, attrs, VISIBLE, ownerId, ownerCanAddInternalSystemWindow, mPowerManagerWrapper); @@ -357,7 +361,7 @@ class WindowTestsBase { WindowTestUtils.TestWindowState createWindowState(WindowManager.LayoutParams attrs, WindowToken token) { synchronized (sWm.mWindowMap) { - return new WindowTestUtils.TestWindowState(sWm, sMockSession, sIWindow, attrs, token); + return new WindowTestUtils.TestWindowState(sWm, mMockSession, mIWindow, attrs, token); } } diff --git a/tests/testables/src/android/testing/DexmakerShareClassLoaderRule.java b/tests/testables/src/android/testing/DexmakerShareClassLoaderRule.java new file mode 100644 index 0000000000000..1b8e58c3050d3 --- /dev/null +++ b/tests/testables/src/android/testing/DexmakerShareClassLoaderRule.java @@ -0,0 +1,129 @@ +/* + * Copyright (C) 2018 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 android.testing; + +import android.util.Log; + +import com.android.internal.annotations.VisibleForTesting; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +import java.util.ConcurrentModificationException; + + +/** + * Runs the test such that mocks created in it don't use a dedicated classloader. + * + * This allows mocking package-private methods. + * + * WARNING: This is absolutely incompatible with running tests in parallel! + */ +public class DexmakerShareClassLoaderRule implements TestRule { + + private static final String TAG = "ShareClassloaderRule"; + @VisibleForTesting + static final String DEXMAKER_SHARE_CLASSLOADER_PROPERTY = "dexmaker.share_classloader"; + + private static Thread sOwningThread = null; + + @Override + public Statement apply(Statement base, Description description) { + return apply(base::evaluate).toStatement(); + } + + /** + * Runs the runnable such that mocks created in it don't use a dedicated classloader. + * + * This allows mocking package-private methods. + * + * WARNING: This is absolutely incompatible with running tests in parallel! + */ + public static void runWithDexmakerShareClassLoader(Runnable r) { + apply(r::run).run(); + } + + /** + * Returns a statement that first makes sure that only one thread at the time is modifying + * the property. Then actually sets the property, and runs the statement. + */ + private static ThrowingRunnable apply(ThrowingRunnable r) { + return wrapInMutex(wrapInSetAndClearProperty(r)); + } + + private static ThrowingRunnable wrapInSetAndClearProperty( + ThrowingRunnable r) { + return () -> { + final String previousValue = System.getProperty(DEXMAKER_SHARE_CLASSLOADER_PROPERTY); + try { + System.setProperty(DEXMAKER_SHARE_CLASSLOADER_PROPERTY, "true"); + r.run(); + } finally { + if (previousValue != null) { + System.setProperty(DEXMAKER_SHARE_CLASSLOADER_PROPERTY, previousValue); + } else { + System.clearProperty(DEXMAKER_SHARE_CLASSLOADER_PROPERTY); + } + } + }; + } + + /** + * Runs the given statement, and while doing so prevents other threads from running statements. + */ + private static ThrowingRunnable wrapInMutex(ThrowingRunnable r) { + return () -> { + final boolean isOwner; + synchronized (DexmakerShareClassLoaderRule.class) { + isOwner = (sOwningThread == null); + if (isOwner) { + sOwningThread = Thread.currentThread(); + } else if (sOwningThread != Thread.currentThread()) { + final RuntimeException e = new ConcurrentModificationException( + "Tried to set dexmaker.share_classloader from " + Thread.currentThread() + + ", but was already set from " + sOwningThread); + // Also log in case exception gets swallowed. + Log.e(TAG, e.getMessage(), e); + throw e; + } + } + try { + r.run(); + } finally { + synchronized (DexmakerShareClassLoaderRule.class) { + if (isOwner) { + sOwningThread = null; + } + } + } + }; + } + + private interface ThrowingRunnable { + void run() throws T; + + default Statement toStatement() { + return new Statement() { + @Override + public void evaluate() throws Throwable { + ThrowingRunnable.this.run(); + } + }; + } + } +} diff --git a/tests/testables/tests/src/android/testing/DexmakerShareClassLoaderRuleTest.java b/tests/testables/tests/src/android/testing/DexmakerShareClassLoaderRuleTest.java new file mode 100644 index 0000000000000..2528d090f0ca9 --- /dev/null +++ b/tests/testables/tests/src/android/testing/DexmakerShareClassLoaderRuleTest.java @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2018 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 android.testing; + +import static android.testing.DexmakerShareClassLoaderRule.DEXMAKER_SHARE_CLASSLOADER_PROPERTY; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.model.Statement; + +import java.util.ConcurrentModificationException; + +@SmallTest +@RunWith(AndroidJUnit4.class) +public class DexmakerShareClassLoaderRuleTest { + + // Intentionally not @Rule, so we have more control. + private final DexmakerShareClassLoaderRule mRule = new DexmakerShareClassLoaderRule(); + + @Before + public void setUp() throws Exception { + System.clearProperty(DEXMAKER_SHARE_CLASSLOADER_PROPERTY); + } + + @Test + public void rule_setsProperty() throws Throwable { + mRule.apply(ASSERT_STATEMENT, null).evaluate(); + } + + @Test + public void rule_resetsProperty() throws Throwable { + mRule.apply(ASSERT_STATEMENT, null).evaluate(); + assertThat(readProperty(), is(false)); + } + + @Test + public void rule_resetsProperty_toExactValue() throws Throwable { + System.setProperty(DEXMAKER_SHARE_CLASSLOADER_PROPERTY, "asdf"); + mRule.apply(ASSERT_STATEMENT, null).evaluate(); + assertThat(System.getProperty(DEXMAKER_SHARE_CLASSLOADER_PROPERTY), is("asdf")); + } + + @Test + public void rule_preventsOtherThreadFromInterfering() throws Throwable { + mRule.apply(new Statement() { + @Override + public void evaluate() throws Throwable { + assertThat(readProperty(), is(true)); + + final Throwable[] thrown = new Throwable[1]; + final Thread t = new Thread(() -> { + try { + new DexmakerShareClassLoaderRule().apply(ASSERT_STATEMENT, null).evaluate(); + fail("Expected a ConcurrentModificationException"); + } catch (ConcurrentModificationException e) { + // Success. + } catch (Throwable tr) { + thrown[0] = tr; + } + }); + t.start(); + t.join(); + + if (thrown[0] != null) { + throw thrown[0]; + } + } + }, null).evaluate(); + assertThat(readProperty(), is(false)); + } + + @Test + public void rule_isReentrant() throws Throwable { + mRule.apply(new Statement() { + @Override + public void evaluate() throws Throwable { + assertThat(readProperty(), is(true)); + new DexmakerShareClassLoaderRule().apply(ASSERT_STATEMENT, null).evaluate(); + assertThat(readProperty(), is(true)); + } + }, null).evaluate(); + assertThat(readProperty(), is(false)); + } + + private static boolean readProperty() { + return Boolean.parseBoolean(System.getProperty(DEXMAKER_SHARE_CLASSLOADER_PROPERTY)); + } + + private static final Statement ASSERT_STATEMENT = new Statement() { + @Override + public void evaluate() throws Throwable { + assertThat(readProperty(), is(true)); + } + }; + +}