Merge "Fix WMS instance leakage of WmTests" into tm-dev (a8b31104) · Commits · e / os / android_frameworks_base

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

+3 −15

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

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

	static HighRefreshRateDenylist create(@NonNull Resources r) {		static HighRefreshRateDenylist create(@NonNull Resources r) {
	return new HighRefreshRateDenylist(r, DeviceConfigInterface.REAL);		return new HighRefreshRateDenylist(r, DeviceConfigInterface.REAL);
	}		}
	@@ -51,10 +48,9 @@ class HighRefreshRateDenylist {
	@VisibleForTesting		@VisibleForTesting
	HighRefreshRateDenylist(Resources r, DeviceConfigInterface deviceConfig) {		HighRefreshRateDenylist(Resources r, DeviceConfigInterface deviceConfig) {
	mDefaultDenylist = r.getStringArray(R.array.config_highRefreshRateBlacklist);		mDefaultDenylist = r.getStringArray(R.array.config_highRefreshRateBlacklist);
	mDeviceConfig = deviceConfig;		deviceConfig.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,
	mDeviceConfig.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,		BackgroundThread.getExecutor(), new OnPropertiesChangedListener());
	BackgroundThread.getExecutor(), mListener);		final String property = deviceConfig.getProperty(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,
	final String property = mDeviceConfig.getProperty(DeviceConfig.NAMESPACE_DISPLAY_MANAGER,
	KEY_HIGH_REFRESH_RATE_BLACKLIST);		KEY_HIGH_REFRESH_RATE_BLACKLIST);
	updateDenylist(property);		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 {		private class OnPropertiesChangedListener implements DeviceConfig.OnPropertiesChangedListener {
	public void onPropertiesChanged(@NonNull DeviceConfig.Properties properties) {		public void onPropertiesChanged(@NonNull DeviceConfig.Properties properties) {
	if (properties.getKeyset().contains(KEY_HIGH_REFRESH_RATE_BLACKLIST)) {		if (properties.getKeyset().contains(KEY_HIGH_REFRESH_RATE_BLACKLIST)) {

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

+0 −6

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

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

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

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

+0 −6

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

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

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

	private HighRefreshRateDenylist mDenylist;		private HighRefreshRateDenylist mDenylist;

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

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

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

+39 −11

Original line number	Original line	Diff line number	Diff line
	@@ -27,6 +27,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.anyBoolean;
	import static com.android.dx.mockito.inline.extended.ExtendedMockito.anyInt;		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.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.doNothing;
	import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;		import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
	import static com.android.dx.mockito.inline.extended.ExtendedMockito.eq;		import static com.android.dx.mockito.inline.extended.ExtendedMockito.eq;
	@@ -36,6 +37,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.spy;
	import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;		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.ActivityManagerInternal;
	import android.app.AppOpsManager;		import android.app.AppOpsManager;
	import android.app.usage.UsageStatsManagerInternal;		import android.app.usage.UsageStatsManagerInternal;
	@@ -57,6 +61,7 @@ import android.os.PowerManagerInternal;
	import android.os.PowerSaveState;		import android.os.PowerSaveState;
	import android.os.StrictMode;		import android.os.StrictMode;
	import android.os.UserHandle;		import android.os.UserHandle;
			import android.provider.DeviceConfig;
	import android.util.Log;		import android.util.Log;
	import android.view.InputChannel;		import android.view.InputChannel;
	import android.view.Surface;		import android.view.Surface;
	@@ -83,10 +88,12 @@ import com.android.server.uri.UriGrantsManagerInternal;
	import org.junit.rules.TestRule;		import org.junit.rules.TestRule;
	import org.junit.runner.Description;		import org.junit.runner.Description;
	import org.junit.runners.model.Statement;		import org.junit.runners.model.Statement;
			import org.mockito.MockSettings;
	import org.mockito.Mockito;		import org.mockito.Mockito;
	import org.mockito.quality.Strictness;		import org.mockito.quality.Strictness;

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

	@@ -106,6 +113,13 @@ public class SystemServicesTestRule implements TestRule {
	.getSystemService(PowerManager.class).newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);		.getSystemService(PowerManager.class).newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);
	private PowerManager.WakeLock mStubbedWakeLock;		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 Description mDescription;
	private Context mContext;		private Context mContext;
	private StaticMockitoSession mMockitoSession;		private StaticMockitoSession mMockitoSession;
	@@ -141,20 +155,29 @@ public class SystemServicesTestRule implements TestRule {
	Log.e("SystemServicesTestRule", "Suppressed: ", throwable);		Log.e("SystemServicesTestRule", "Suppressed: ", throwable);
	t.addSuppressed(throwable);		t.addSuppressed(throwable);
	}		}
	throw t;		throwable = t;
	}		}
	if (throwable != null) throw throwable;
	}		}
			if (throwable != null) throw throwable;
	}		}
	};		};
	}		}

	private void setUp() {		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 real operation: SurfaceControl.mirrorSurface
			// Avoid leakage: DeviceConfig.addOnPropertiesChangedListener, LockGuard.installLock
			// Watchdog.getInstance/addMonitor
	mMockitoSession = mockitoSession()		mMockitoSession = mockitoSession()
	.spyStatic(LocalServices.class)		.mockStatic(LocalServices.class, spyStubOnly)
	.spyStatic(SurfaceControl.class)		.mockStatic(DeviceConfig.class, spyStubOnly)
	.mockStatic(LockGuard.class)		.mockStatic(SurfaceControl.class, mockStubOnly)
	.mockStatic(Watchdog.class)		.mockStatic(LockGuard.class, mockStubOnly)
			.mockStatic(Watchdog.class, mockStubOnly)
	.strictness(Strictness.LENIENT)		.strictness(Strictness.LENIENT)
	.startMocking();		.startMocking();

	@@ -166,6 +189,12 @@ public class SystemServicesTestRule implements TestRule {

	private void setUpSystemCore() {		private void setUpSystemCore() {
	doReturn(mock(Watchdog.class)).when(Watchdog::getInstance);		doReturn(mock(Watchdog.class)).when(Watchdog::getInstance);
			doAnswer(invocation -> {
			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();		mContext = getInstrumentation().getTargetContext();
	spyOn(mContext);		spyOn(mContext);
	@@ -345,11 +374,10 @@ public class SystemServicesTestRule implements TestRule {
	// Unregister display listener from root to avoid issues with subsequent tests.		// Unregister display listener from root to avoid issues with subsequent tests.
	mContext.getSystemService(DisplayManager.class)		mContext.getSystemService(DisplayManager.class)
	.unregisterDisplayListener(mAtmService.mRootWindowContainer);		.unregisterDisplayListener(mAtmService.mRootWindowContainer);
	// The constructor of WindowManagerService registers WindowManagerConstants and
	// HighRefreshRateBlacklist with DeviceConfig. We need to undo that here to avoid		for (int i = mDeviceConfigListeners.size() - 1; i >= 0; i--) {
	// leaking mWmService.		DeviceConfig.removeOnPropertiesChangedListener(mDeviceConfigListeners.get(i));
	mWmService.mConstants.dispose();		}
	mWmService.mHighRefreshRateDenylist.dispose();

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

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

+39 −34

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

	package com.android.server.wm;		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 android.platform.test.annotations.Presubmit;

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

	import org.junit.Test;		import org.junit.Test;
	import org.junit.rules.ExpectedException;
	import org.junit.runners.model.Statement;		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		@Presubmit
	public class SystemServicesTestRuleTest {		public class SystemServicesTestRuleTest {
	@Rule
	public ExpectedException mExpectedException = ExpectedException.none();

	@Test		@Test
	public void testRule_rethrows_unchecked_exceptions() throws Throwable {		public void testRule_rethrows_throwable() {
	final SystemServicesTestRule mWmsRule = new SystemServicesTestRule();		assertThrows(Throwable.class, () -> applyRule(rule -> false));
	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();
	}		}

	@Test		@Test
	public void testRule_rethrows_checked_exceptions() throws Throwable {		public void testRule_ranSuccessfully() throws Throwable {
	final SystemServicesTestRule mWmsRule = new SystemServicesTestRule();		final int iterations = 5;
	Statement statement = new Statement() {		final ArrayList<WeakReference<WindowManagerService>> wmsRefs = new ArrayList<>();
	@Override		for (int i = 0; i < iterations; i++) {
	public void evaluate() throws Throwable {		applyRule(rule -> {
	throw new IOException("A failing test!");		final WindowManagerService wms = rule.getWindowManagerService();
			assertNotNull(wms);
			wmsRefs.add(new WeakReference<>(wms));
			return true;
			});
	}		}
	};		assertEquals(iterations, wmsRefs.size());
	mExpectedException.expect(IOException.class);
	mWmsRule.apply(statement, null /* Description*/).evaluate();		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		private static void applyRule(Predicate<SystemServicesTestRule> action) throws Throwable {
	public void testRule_ranSuccessfully() throws Throwable {		final SystemServicesTestRule wmsRule = new SystemServicesTestRule();
	final boolean[] testRan = {false};		wmsRule.apply(new Statement() {
	final SystemServicesTestRule mWmsRule = new SystemServicesTestRule();
	Statement statement = new Statement() {
	@Override		@Override
	public void evaluate() throws Throwable {		public void evaluate() throws Throwable {
	testRan[0] = true;		if (!action.test(wmsRule)) {
			throw new Throwable("A failing test!");
			}
	}		}
	};		}, null /* description */).evaluate();
	mWmsRule.apply(statement, null /* Description*/).evaluate();
	assertTrue(testRan[0]);
	}		}
	}		}