Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Fix WMS instance leakage of WmTests" am: 49c0c87c79 am: e77b1ef5a8

Browse Source 
Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/2036104

Change-Id: I4e71383e2508473adacc5468524dda0f0a308357
This commit is contained in:
Riddle Hsu
2022-03-22 12:49:07 +00:00
committed by Automerger Merge Worker
parent 0710c69c9b e77b1ef5a8
commit 0d54d43602
5 changed files with 86 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
services/core/java/com/android/server/wm/HighRefreshRateDenylist.java
View File

@@ -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)) {

6
services/core/java/com/android/server/wm/WindowManagerConstants.java
View File

@@ -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;

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

@@ -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);

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

@@ -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;
}
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.

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

@@ -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();
}
@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!");
}
};
mExpectedException.expect(IOException.class);
mWmsRule.apply(statement, null /* Description*/).evaluate();
public void testRule_rethrows_throwable() {
assertThrows(Throwable.class, () -> applyRule(rule -> false));
}
@Test
public void testRule_ranSuccessfully() throws Throwable {
final boolean[] testRan = {false};
final SystemServicesTestRule mWmsRule = new SystemServicesTestRule();
Statement statement = new Statement() {
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;
});
}
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");
}
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();
}
}