Merge "Fix WMS instance leakage of WmTests" (49c0c87c) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/wm/HighRefreshRateDenylist.java

+3 −15

Original line number	Diff line number	Diff line
		@@ -41,9 +41,6 @@ class HighRefreshRateDenylist {
		private final String[] mDefaultDenylist;
		private final Object mLock = new Object();

		private DeviceConfigInterface mDeviceConfig;
		private OnPropertiesChangedListener mListener = new OnPropertiesChangedListener();

		static HighRefreshRateDenylist create(@NonNull Resources r) {
		return new HighRefreshRateDenylist(r, DeviceConfigInterface.REAL);
		}
		@@ -51,10 +48,9 @@ class HighRefreshRateDenylist {
		@VisibleForTesting
		HighRefreshRateDenylist(Resources r, DeviceConfigInterface deviceConfig) {
		mDefaultDenylist = r.getStringArray(R.array.config_highRefreshRateBlacklist);
		mDeviceConfig = deviceConfig;
		mDeviceConfig.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,
		BackgroundThread.getExecutor(), mListener);
		final String property = mDeviceConfig.getProperty(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,
		deviceConfig.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,
		BackgroundThread.getExecutor(), new OnPropertiesChangedListener());
		final String property = deviceConfig.getProperty(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,
		KEY_HIGH_REFRESH_RATE_BLACKLIST);
		updateDenylist(property);
		}
		@@ -95,14 +91,6 @@ class HighRefreshRateDenylist {
		}
		}

		/** Used to prevent WmTests leaking issues. */
		@VisibleForTesting
		void dispose() {
		mDeviceConfig.removeOnPropertiesChangedListener(mListener);
		mDeviceConfig = null;
		mDenylistedPackages.clear();
		}

		private class OnPropertiesChangedListener implements DeviceConfig.OnPropertiesChangedListener {
		public void onPropertiesChanged(@NonNull DeviceConfig.Properties properties) {
		if (properties.getKeyset().contains(KEY_HIGH_REFRESH_RATE_BLACKLIST)) {

services/core/java/com/android/server/wm/WindowManagerConstants.java

+0 −6

Original line number	Diff line number	Diff line
		@@ -91,12 +91,6 @@ final class WindowManagerConstants {
		updateSystemGestureExcludedByPreQStickyImmersive();
		}

		@VisibleForTesting
		void dispose() {
		mDeviceConfig.removeOnPropertiesChangedListener(mListenerAndroid);
		mDeviceConfig.removeOnPropertiesChangedListener(mListenerWindowManager);
		}

		private void onAndroidPropertiesChanged(DeviceConfig.Properties properties) {
		synchronized (mGlobalLock) {
		boolean updateSystemGestureExclusionLimit = false;

services/tests/wmtests/src/com/android/server/wm/HighRefreshRateDenylistTest.java

+0 −6

Original line number	Diff line number	Diff line
		@@ -33,7 +33,6 @@ import com.android.internal.R;
		import com.android.internal.util.Preconditions;
		import com.android.server.testutils.FakeDeviceConfigInterface;

		import org.junit.After;
		import org.junit.Test;

		import java.util.concurrent.Executor;
		@@ -51,11 +50,6 @@ public class HighRefreshRateDenylistTest {

		private HighRefreshRateDenylist mDenylist;

		@After
		public void tearDown() {
		mDenylist.dispose();
		}

		@Test
		public void testDefaultDenylist() {
		final Resources r = createResources(APP1, APP2);

services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java

+42 −10

Original line number	Diff line number	Diff line
		@@ -18,6 +18,7 @@ package com.android.server.wm;

		import static android.app.WindowConfiguration.ACTIVITY_TYPE_HOME;
		import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN;
		import static android.provider.DeviceConfig.NAMESPACE_CONSTRAIN_DISPLAY_APIS;
		import static android.testing.DexmakerShareClassLoaderRule.runWithDexmakerShareClassLoader;
		import static android.view.Display.DEFAULT_DISPLAY;

		@@ -27,6 +28,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.any;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.anyBoolean;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.anyInt;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.anyString;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.eq;
		@@ -36,6 +38,9 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.nullable;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.spy;
		import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;

		import static org.mockito.Mockito.CALLS_REAL_METHODS;
		import static org.mockito.Mockito.withSettings;

		import android.app.ActivityManagerInternal;
		import android.app.AppOpsManager;
		import android.app.usage.UsageStatsManagerInternal;
		@@ -57,6 +62,7 @@ import android.os.PowerManagerInternal;
		import android.os.PowerSaveState;
		import android.os.StrictMode;
		import android.os.UserHandle;
		import android.provider.DeviceConfig;
		import android.util.Log;
		import android.view.InputChannel;
		import android.view.Surface;
		@@ -83,10 +89,12 @@ import com.android.server.uri.UriGrantsManagerInternal;
		import org.junit.rules.TestRule;
		import org.junit.runner.Description;
		import org.junit.runners.model.Statement;
		import org.mockito.MockSettings;
		import org.mockito.Mockito;
		import org.mockito.quality.Strictness;

		import java.io.File;
		import java.util.ArrayList;
		import java.util.concurrent.atomic.AtomicBoolean;
		import java.util.function.Supplier;

		@@ -107,6 +115,13 @@ public class SystemServicesTestRule implements TestRule {
		.getSystemService(PowerManager.class).newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);
		private PowerManager.WakeLock mStubbedWakeLock;

		/**
		* The captured listeners will be unregistered in {@link #tearDown()} to avoid keeping static
		* references of test instances from DeviceConfig.
		*/
		private final ArrayList<DeviceConfig.OnPropertiesChangedListener> mDeviceConfigListeners =
		new ArrayList<>();

		private Description mDescription;
		private Context mContext;
		private StaticMockitoSession mMockitoSession;
		@@ -144,19 +159,27 @@ public class SystemServicesTestRule implements TestRule {
		Log.e("SystemServicesTestRule", "Suppressed: ", throwable);
		t.addSuppressed(throwable);
		}
		throw t;
		throwable = t;
		}
		}
		if (throwable != null) throw throwable;
		}
		}
		};
		}

		private void setUp() {
		// Use stubOnly() to reduce memory usage if it doesn't need verification.
		final MockSettings spyStubOnly = withSettings().stubOnly()
		.defaultAnswer(CALLS_REAL_METHODS);
		final MockSettings mockStubOnly = withSettings().stubOnly();
		// Return mocked services: LocalServices.getService
		// Avoid leakage: DeviceConfig.addOnPropertiesChangedListener, LockGuard.installLock
		// Watchdog.getInstance/addMonitor
		mMockitoSession = mockitoSession()
		.spyStatic(LocalServices.class)
		.mockStatic(LockGuard.class)
		.mockStatic(Watchdog.class)
		.mockStatic(LocalServices.class, spyStubOnly)
		.mockStatic(DeviceConfig.class, spyStubOnly)
		.mockStatic(LockGuard.class, mockStubOnly)
		.mockStatic(Watchdog.class, mockStubOnly)
		.strictness(Strictness.LENIENT)
		.startMocking();

		@@ -168,6 +191,16 @@ public class SystemServicesTestRule implements TestRule {

		private void setUpSystemCore() {
		doReturn(mock(Watchdog.class)).when(Watchdog::getInstance);
		doAnswer(invocation -> {
		// Exclude CONSTRAIN_DISPLAY_APIS because ActivityRecord#sConstrainDisplayApisConfig
		// only registers once and it doesn't reference to outside.
		if (!NAMESPACE_CONSTRAIN_DISPLAY_APIS.equals(invocation.getArgument(0))) {
		mDeviceConfigListeners.add(invocation.getArgument(2));
		}
		// SizeCompatTests uses setNeverConstrainDisplayApisFlag, and ActivityRecordTests
		// uses splash_screen_exception_list. So still execute real registration.
		return invocation.callRealMethod();
		}).when(() -> DeviceConfig.addOnPropertiesChangedListener(anyString(), any(), any()));

		mContext = getInstrumentation().getTargetContext();
		spyOn(mContext);
		@@ -346,11 +379,10 @@ public class SystemServicesTestRule implements TestRule {
		// Unregister display listener from root to avoid issues with subsequent tests.
		mContext.getSystemService(DisplayManager.class)
		.unregisterDisplayListener(mAtmService.mRootWindowContainer);
		// The constructor of WindowManagerService registers WindowManagerConstants and
		// HighRefreshRateBlacklist with DeviceConfig. We need to undo that here to avoid
		// leaking mWmService.
		mWmService.mConstants.dispose();
		mWmService.mHighRefreshRateDenylist.dispose();

		for (int i = mDeviceConfigListeners.size() - 1; i >= 0; i--) {
		DeviceConfig.removeOnPropertiesChangedListener(mDeviceConfigListeners.get(i));
		}

		// This makes sure the posted messages without delay are processed, e.g.
		// DisplayPolicy#release, WindowManagerService#setAnimationScale.

services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRuleTest.java

+39 −34

Original line number	Diff line number	Diff line
		@@ -16,59 +16,64 @@

		package com.android.server.wm;

		import static junit.framework.Assert.assertTrue;
		import static org.junit.Assert.assertEquals;
		import static org.junit.Assert.assertNotNull;
		import static org.junit.Assert.assertThrows;
		import static org.junit.Assert.fail;

		import android.platform.test.annotations.Presubmit;

		import org.junit.Rule;
		import com.android.internal.util.GcUtils;

		import org.junit.Test;
		import org.junit.rules.ExpectedException;
		import org.junit.runners.model.Statement;

		import java.io.IOException;
		import java.lang.ref.WeakReference;
		import java.util.ArrayList;
		import java.util.function.Predicate;

		@Presubmit
		public class SystemServicesTestRuleTest {
		@Rule
		public ExpectedException mExpectedException = ExpectedException.none();

		@Test
		public void testRule_rethrows_unchecked_exceptions() throws Throwable {
		final SystemServicesTestRule mWmsRule = new SystemServicesTestRule();
		Statement statement = new Statement() {
		@Override
		public void evaluate() throws Throwable {
		throw new RuntimeException("A failing test!");
		}
		};
		mExpectedException.expect(RuntimeException.class);
		mWmsRule.apply(statement, null /* Description*/).evaluate();
		public void testRule_rethrows_throwable() {
		assertThrows(Throwable.class, () -> applyRule(rule -> false));
		}

		@Test
		public void testRule_rethrows_checked_exceptions() throws Throwable {
		final SystemServicesTestRule mWmsRule = new SystemServicesTestRule();
		Statement statement = new Statement() {
		@Override
		public void evaluate() throws Throwable {
		throw new IOException("A failing test!");
		public void testRule_ranSuccessfully() throws Throwable {
		final int iterations = 5;
		final ArrayList<WeakReference<WindowManagerService>> wmsRefs = new ArrayList<>();
		for (int i = 0; i < iterations; i++) {
		applyRule(rule -> {
		final WindowManagerService wms = rule.getWindowManagerService();
		assertNotNull(wms);
		wmsRefs.add(new WeakReference<>(wms));
		return true;
		});
		}
		};
		mExpectedException.expect(IOException.class);
		mWmsRule.apply(statement, null /* Description*/).evaluate();
		assertEquals(iterations, wmsRefs.size());

		GcUtils.runGcAndFinalizersSync();
		// Only ensure that at least one instance is released because some references may be kept
		// temporally by the message of other thread or single static reference.
		for (int i = wmsRefs.size() - 1; i >= 0; i--) {
		if (wmsRefs.get(i).get() == null) {
		return;
		}
		}
		fail("WMS instance is leaked");
		}

		@Test
		public void testRule_ranSuccessfully() throws Throwable {
		final boolean[] testRan = {false};
		final SystemServicesTestRule mWmsRule = new SystemServicesTestRule();
		Statement statement = new Statement() {
		private static void applyRule(Predicate<SystemServicesTestRule> action) throws Throwable {
		final SystemServicesTestRule wmsRule = new SystemServicesTestRule();
		wmsRule.apply(new Statement() {
		@Override
		public void evaluate() throws Throwable {
		testRan[0] = true;
		if (!action.test(wmsRule)) {
		throw new Throwable("A failing test!");
		}
		}
		};
		mWmsRule.apply(statement, null /* Description*/).evaluate();
		assertTrue(testRan[0]);
		}, null /* description */).evaluate();
		}
		}