Address TouchMonitor memory leaks.
This changelist addresses TouchMonitor memory leaks by:
- Clearing TouchMonitor's reference to the gesture exclusion listener to
prevent circular references.
- Removing lifecycle observers and lifecycle flows when the TouchMonitor
is destroyed.
- Clearing references to touch sessions when no longer used.
Fixes: 338532926
Test: DreamOverlayServiceTest#testOnEndDream
Test: TouchMonitorTest#testDestroy_cleansUpLifecycleObserver
Flag: EXEMPT bugfix
Change-Id: I6979c8cbcd8fa9baebf962927c93325eb6eb688c
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/dreams/DreamOverlayServiceTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/dreams/DreamOverlayServiceTest.kt
index eef2337..221af6e 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/dreams/DreamOverlayServiceTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/dreams/DreamOverlayServiceTest.kt
@@ -398,6 +398,9 @@
verify(mStateController).setOverlayActive(false)
verify(mStateController).setLowLightActive(false)
verify(mStateController).setEntryAnimationsFinished(false)
+
+ // Verify touch monitor destroyed
+ verify(mTouchMonitor).destroy()
}
@Test
diff --git a/packages/SystemUI/src/com/android/systemui/ambient/touch/BouncerSwipeTouchHandler.java b/packages/SystemUI/src/com/android/systemui/ambient/touch/BouncerSwipeTouchHandler.java
index 019f498..f905241 100644
--- a/packages/SystemUI/src/com/android/systemui/ambient/touch/BouncerSwipeTouchHandler.java
+++ b/packages/SystemUI/src/com/android/systemui/ambient/touch/BouncerSwipeTouchHandler.java
@@ -269,6 +269,7 @@
}
mScrimManager.removeCallback(mScrimManagerCallback);
mCapture = null;
+ mTouchSession = null;
if (!Flags.communalBouncerDoNotModifyPluginOpen()) {
mNotificationShadeWindowController.setForcePluginOpen(false, this);
diff --git a/packages/SystemUI/src/com/android/systemui/ambient/touch/TouchMonitor.java b/packages/SystemUI/src/com/android/systemui/ambient/touch/TouchMonitor.java
index 227e4db..61b4401 100644
--- a/packages/SystemUI/src/com/android/systemui/ambient/touch/TouchMonitor.java
+++ b/packages/SystemUI/src/com/android/systemui/ambient/touch/TouchMonitor.java
@@ -49,11 +49,14 @@
import com.google.common.util.concurrent.ListenableFuture;
+import kotlinx.coroutines.Job;
+
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;
+import java.util.concurrent.CancellationException;
import java.util.concurrent.Executor;
import java.util.function.Consumer;
import java.util.stream.Collectors;
@@ -78,15 +81,7 @@
private final Lifecycle mLifecycle;
private Rect mExclusionRect = null;
- private ISystemGestureExclusionListener mGestureExclusionListener =
- new ISystemGestureExclusionListener.Stub() {
- @Override
- public void onSystemGestureExclusionChanged(int displayId,
- Region systemGestureExclusion,
- Region systemGestureExclusionUnrestricted) {
- mExclusionRect = systemGestureExclusion.getBounds();
- }
- };
+ private ISystemGestureExclusionListener mGestureExclusionListener;
private Consumer<Rect> mMaxBoundsConsumer = rect -> mMaxBounds = rect;
@@ -274,6 +269,14 @@
if (bouncerAreaExclusion()) {
mBackgroundExecutor.execute(() -> {
try {
+ mGestureExclusionListener = new ISystemGestureExclusionListener.Stub() {
+ @Override
+ public void onSystemGestureExclusionChanged(int displayId,
+ Region systemGestureExclusion,
+ Region systemGestureExclusionUnrestricted) {
+ mExclusionRect = systemGestureExclusion.getBounds();
+ }
+ };
mWindowManagerService.registerSystemGestureExclusionListener(
mGestureExclusionListener, mDisplayId);
} catch (RemoteException e) {
@@ -298,8 +301,11 @@
if (bouncerAreaExclusion()) {
mBackgroundExecutor.execute(() -> {
try {
- mWindowManagerService.unregisterSystemGestureExclusionListener(
- mGestureExclusionListener, mDisplayId);
+ if (mGestureExclusionListener != null) {
+ mWindowManagerService.unregisterSystemGestureExclusionListener(
+ mGestureExclusionListener, mDisplayId);
+ mGestureExclusionListener = null;
+ }
} catch (RemoteException e) {
// Handle the exception
Log.e(TAG, "unregisterSystemGestureExclusionListener: failed", e);
@@ -494,6 +500,10 @@
private Rect mMaxBounds;
+ private Job mBoundsFlow;
+
+ private boolean mInitialized;
+
/**
* Designated constructor for {@link TouchMonitor}
@@ -535,10 +545,35 @@
* Initializes the monitor. should only be called once after creation.
*/
public void init() {
+ if (mInitialized) {
+ throw new IllegalStateException("TouchMonitor already initialized");
+ }
+
mLifecycle.addObserver(mLifecycleObserver);
if (Flags.ambientTouchMonitorListenToDisplayChanges()) {
- collectFlow(mLifecycle, mConfigurationInteractor.getMaxBounds(), mMaxBoundsConsumer);
+ mBoundsFlow = collectFlow(mLifecycle, mConfigurationInteractor.getMaxBounds(),
+ mMaxBoundsConsumer);
}
+
+ mInitialized = true;
+ }
+
+ /**
+ * Called when the TouchMonitor should be discarded and will not be used anymore.
+ */
+ public void destroy() {
+ if (!mInitialized) {
+ throw new IllegalStateException("TouchMonitor not initialized");
+ }
+
+ stopMonitoring(true);
+
+ mLifecycle.removeObserver(mLifecycleObserver);
+ if (Flags.ambientTouchMonitorListenToDisplayChanges()) {
+ mBoundsFlow.cancel(new CancellationException());
+ }
+
+ mInitialized = false;
}
private void isolate(Set<TouchSessionImpl> sessions) {
diff --git a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
index aa7a7da..96e708f 100644
--- a/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
+++ b/packages/SystemUI/src/com/android/systemui/dreams/DreamOverlayService.java
@@ -543,7 +543,11 @@
mStateController.setEntryAnimationsFinished(false);
mDreamOverlayContainerViewController = null;
- mTouchMonitor = null;
+
+ if (mTouchMonitor != null) {
+ mTouchMonitor.destroy();
+ mTouchMonitor = null;
+ }
mWindow = null;
mStarted = false;
diff --git a/packages/SystemUI/src/com/android/systemui/util/kotlin/JavaAdapter.kt b/packages/SystemUI/src/com/android/systemui/util/kotlin/JavaAdapter.kt
index 1ec86a4..8c46f9a 100644
--- a/packages/SystemUI/src/com/android/systemui/util/kotlin/JavaAdapter.kt
+++ b/packages/SystemUI/src/com/android/systemui/util/kotlin/JavaAdapter.kt
@@ -88,8 +88,8 @@
flow: Flow<T>,
consumer: Consumer<T>,
state: Lifecycle.State = Lifecycle.State.CREATED,
-) {
- lifecycle.coroutineScope.launch {
+): Job {
+ return lifecycle.coroutineScope.launch {
lifecycle.repeatOnLifecycle(state) { flow.collect { consumer.accept(it) } }
}
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/ambient/touch/TouchMonitorTest.java b/packages/SystemUI/tests/src/com/android/systemui/ambient/touch/TouchMonitorTest.java
index d01d57e..358e8cb 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/ambient/touch/TouchMonitorTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/ambient/touch/TouchMonitorTest.java
@@ -22,6 +22,8 @@
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.atLeast;
+import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
@@ -45,6 +47,7 @@
import androidx.annotation.NonNull;
import androidx.lifecycle.Lifecycle;
+import androidx.lifecycle.LifecycleObserver;
import androidx.lifecycle.LifecycleOwner;
import androidx.lifecycle.LifecycleRegistry;
import androidx.test.filters.SmallTest;
@@ -67,6 +70,7 @@
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
+import java.util.ArrayList;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ExecutionException;
@@ -119,6 +123,8 @@
private final KosmosJavaAdapter mKosmos;
+ private ArrayList<LifecycleObserver> mLifecycleObservers = new ArrayList<>();
+
Environment(Set<TouchHandler> handlers, KosmosJavaAdapter kosmos) {
mLifecycleOwner = new SimpleLifecycleOwner();
@@ -147,8 +153,14 @@
mMonitor = new TouchMonitor(mExecutor, mBackgroundExecutor, mLifecycleRegistry,
mInputFactory, mDisplayHelper, mKosmos.getConfigurationInteractor(),
handlers, mIWindowManager, 0);
+ clearInvocations(mLifecycleRegistry);
mMonitor.init();
+ ArgumentCaptor<LifecycleObserver> observerCaptor =
+ ArgumentCaptor.forClass(LifecycleObserver.class);
+ verify(mLifecycleRegistry, atLeast(1)).addObserver(observerCaptor.capture());
+ mLifecycleObservers.addAll(observerCaptor.getAllValues());
+
updateLifecycle(Lifecycle.State.RESUMED);
// Capture creation request.
@@ -187,6 +199,16 @@
verify(mInputSession).dispose();
Mockito.clearInvocations(mInputSession);
}
+
+ void destroyMonitor() {
+ mMonitor.destroy();
+ }
+
+ void verifyLifecycleObserversUnregistered() {
+ for (LifecycleObserver observer : mLifecycleObservers) {
+ verify(mLifecycleRegistry).removeObserver(observer);
+ }
+ }
}
@Test
@@ -642,6 +664,16 @@
verify(callback).onRemoved();
}
+ @Test
+ public void testDestroy_cleansUpLifecycleObserver() {
+ final TouchHandler touchHandler = createTouchHandler();
+
+ final Environment environment = new Environment(Stream.of(touchHandler)
+ .collect(Collectors.toCollection(HashSet::new)), mKosmos);
+ environment.destroyMonitor();
+ environment.verifyLifecycleObserversUnregistered();
+ }
+
private GestureDetector.OnGestureListener registerGestureListener(TouchHandler handler) {
final GestureDetector.OnGestureListener gestureListener = Mockito.mock(
GestureDetector.OnGestureListener.class);