Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2d6d916f authored by Bryce Lee's avatar Bryce Lee
Browse files

Address memory leak in EdgeBackGestureHandler. 

EdgeBackGestureHandler listens to changes in GestureInteractor by
collecting the blocked activity flow. Previously, this flow was
untracked and therefore not cleaned up when the
EdgeBackGestureHandler was discarded. This changelist addresse
this by retaining the returned Job and canceling it when the
EdgeBackGestureHandler is not enabled.

Test: traced memory usage in hprof to ensure only a single
      instance is retained.
Fixes: 355835858
Flag: EXEMPT bugfix.
Change-Id: I0c1519326cadc4d048e98a7960e3f8bcc2b0ced4
parent be1c803e

packages/SystemUI/multivalentTests/src/com/android/systemui/gesture/domain/GestureInteractorTest.kt

+57 −69
Original line number Diff line number Diff line
@@ -16,120 +16,108 @@ 


package com.android.systemui.gesture.domain



import android.app.ActivityManager

import android.content.ComponentName

import androidx.test.ext.junit.runners.AndroidJUnit4

import androidx.test.filters.SmallTest

import com.android.systemui.SysuiTestCase

import com.android.systemui.coroutines.collectLastValue

import com.android.systemui.navigationbar.gestural.data.respository.GestureRepository

import com.android.systemui.kosmos.backgroundCoroutineContext

import com.android.systemui.kosmos.testDispatcher

import com.android.systemui.navigationbar.gestural.data.gestureRepository

import com.android.systemui.navigationbar.gestural.domain.GestureInteractor

import com.android.systemui.shared.system.activityManagerWrapper

import com.android.systemui.shared.system.taskStackChangeListeners

import com.android.systemui.testKosmos

import com.google.common.truth.Truth.assertThat

import kotlinx.coroutines.Dispatchers

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.test.StandardTestDispatcher

import kotlinx.coroutines.test.TestScope

import kotlinx.coroutines.test.resetMain

import kotlinx.coroutines.test.runCurrent

import kotlinx.coroutines.test.runTest

import kotlinx.coroutines.test.setMain

import org.junit.After

import org.junit.Before

import org.junit.Rule

import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.Mock

import org.mockito.junit.MockitoJUnit

import org.mockito.junit.MockitoRule

import org.mockito.kotlin.any

import org.mockito.kotlin.argumentCaptor

import org.mockito.kotlin.mock

import org.mockito.kotlin.never

import org.mockito.kotlin.verify

import org.mockito.kotlin.spy

import org.mockito.kotlin.whenever



@RunWith(AndroidJUnit4::class)

@SmallTest

class GestureInteractorTest : SysuiTestCase() {

    @Rule @JvmField val mockitoRule: MockitoRule = MockitoJUnit.rule()

    private val kosmos = testKosmos()



    val dispatcher = StandardTestDispatcher()

    val dispatcher = kosmos.testDispatcher

    val repository = spy(kosmos.gestureRepository)

    val testScope = TestScope(dispatcher)



    @Mock private lateinit var gestureRepository: GestureRepository

    private val underTest by lazy { createInteractor() }



    private val underTest by lazy {

        GestureInteractor(gestureRepository, testScope.backgroundScope)

    private fun createInteractor(): GestureInteractor {

        return GestureInteractor(

            repository,

            dispatcher,

            kosmos.backgroundCoroutineContext,

            testScope,

            kosmos.activityManagerWrapper,

            kosmos.taskStackChangeListeners

        )

    }



    @Before

    fun setup() {

        Dispatchers.setMain(dispatcher)

        whenever(gestureRepository.gestureBlockedActivities).thenReturn(MutableStateFlow(setOf()))

    }

    private fun setTopActivity(componentName: ComponentName) {

        val task = mock<ActivityManager.RunningTaskInfo>()

        task.topActivity = componentName

        whenever(kosmos.activityManagerWrapper.runningTask).thenReturn(task)



    @After

    fun tearDown() {

        Dispatchers.resetMain()

        kosmos.taskStackChangeListeners.listenerImpl.onTaskStackChanged()

    }



    @Test

    fun addBlockedActivity_testCombination() =

        testScope.runTest {

            val globalComponent = mock<ComponentName>()

            whenever(gestureRepository.gestureBlockedActivities)

                .thenReturn(MutableStateFlow(setOf(globalComponent)))

            repository.addGestureBlockedActivity(globalComponent)



            val localComponent = mock<ComponentName>()



            val blocked by collectLastValue(underTest.topActivityBlocked)



            underTest.addGestureBlockedActivity(localComponent, GestureInteractor.Scope.Local)

            val lastSeen by collectLastValue(underTest.gestureBlockedActivities)

            testScope.runCurrent()

            verify(gestureRepository, never()).addGestureBlockedActivity(any())

            assertThat(lastSeen).hasSize(2)

            assertThat(lastSeen).containsExactly(globalComponent, localComponent)

        }



    @Test

    fun addBlockedActivityLocally_onlyAffectsLocalInteractor() =

        testScope.runTest {

            val component = mock<ComponentName>()

            underTest.addGestureBlockedActivity(component, GestureInteractor.Scope.Local)

            val lastSeen by collectLastValue(underTest.gestureBlockedActivities)

            testScope.runCurrent()

            verify(gestureRepository, never()).addGestureBlockedActivity(any())

            assertThat(lastSeen).contains(component)

        }

            assertThat(blocked).isFalse()



    @Test

    fun removeBlockedActivityLocally_onlyAffectsLocalInteractor() =

        testScope.runTest {

            val component = mock<ComponentName>()

            underTest.addGestureBlockedActivity(component, GestureInteractor.Scope.Local)

            val lastSeen by collectLastValue(underTest.gestureBlockedActivities)

            testScope.runCurrent()

            underTest.removeGestureBlockedActivity(component, GestureInteractor.Scope.Local)

            testScope.runCurrent()

            verify(gestureRepository, never()).removeGestureBlockedActivity(any())

            assertThat(lastSeen).isEmpty()

            setTopActivity(localComponent)



            assertThat(blocked).isTrue()

        }



    @Test

    fun addBlockedActivity_invokesRepository() =

    fun initialization_testEmit() =

        testScope.runTest {

            val component = mock<ComponentName>()

            underTest.addGestureBlockedActivity(component, GestureInteractor.Scope.Global)

            runCurrent()

            val captor = argumentCaptor<ComponentName>()

            verify(gestureRepository).addGestureBlockedActivity(captor.capture())

            assertThat(captor.firstValue).isEqualTo(component)

            val globalComponent = mock<ComponentName>()

            repository.addGestureBlockedActivity(globalComponent)

            setTopActivity(globalComponent)



            val interactor = createInteractor()



            val blocked by collectLastValue(interactor.topActivityBlocked)

            assertThat(blocked).isTrue()

        }



    @Test

    fun removeBlockedActivity_invokesRepository() =

    fun addBlockedActivityLocally_onlyAffectsLocalInteractor() =

        testScope.runTest {

            val component = mock<ComponentName>()

            underTest.removeGestureBlockedActivity(component, GestureInteractor.Scope.Global)

            runCurrent()

            val captor = argumentCaptor<ComponentName>()

            verify(gestureRepository).removeGestureBlockedActivity(captor.capture())

            assertThat(captor.firstValue).isEqualTo(component)

            val interactor1 = createInteractor()

            val interactor1Blocked by collectLastValue(interactor1.topActivityBlocked)

            val interactor2 = createInteractor()

            val interactor2Blocked by collectLastValue(interactor2.topActivityBlocked)



            val localComponent = mock<ComponentName>()



            interactor1.addGestureBlockedActivity(localComponent, GestureInteractor.Scope.Local)

            setTopActivity(localComponent)



            assertThat(interactor1Blocked).isTrue()

            assertThat(interactor2Blocked).isFalse()

        }

}

packages/SystemUI/src/com/android/systemui/navigationbar/gestural/EdgeBackGestureHandler.java

+22 −12
Original line number Diff line number Diff line
@@ -73,8 +73,8 @@ import androidx.annotation.DimenRes; 


import com.android.internal.config.sysui.SystemUiDeviceConfigFlags;

import com.android.internal.policy.GestureNavigationSettingsObserver;

import com.android.systemui.dagger.qualifiers.Background;

import com.android.systemui.contextualeducation.GestureType;

import com.android.systemui.dagger.qualifiers.Background;

import com.android.systemui.model.SysUiState;

import com.android.systemui.navigationbar.NavigationModeController;

import com.android.systemui.navigationbar.gestural.domain.GestureInteractor;
@@ -102,6 +102,8 @@ import com.android.wm.shell.back.BackAnimation; 
import com.android.wm.shell.desktopmode.DesktopMode;

import com.android.wm.shell.pip.Pip;



import kotlinx.coroutines.Job;



import java.io.PrintWriter;

import java.util.ArrayDeque;

import java.util.Date;
@@ -109,6 +111,7 @@ import java.util.Locale; 
import java.util.Map;

import java.util.Optional;

import java.util.Set;

import java.util.concurrent.CancellationException;

import java.util.concurrent.Executor;

import java.util.concurrent.atomic.AtomicBoolean;

import java.util.function.Consumer;
@@ -158,7 +161,7 @@ public class EdgeBackGestureHandler implements PluginListener<NavigationEdgeBack 
    private TaskStackChangeListener mTaskStackListener = new TaskStackChangeListener() {

        @Override

        public void onTaskStackChanged() {

            updateRunningActivityGesturesBlocked();

            updateTopActivity();

        }

        @Override

        public void onTaskCreated(int taskId, ComponentName componentName) {
@@ -222,6 +225,8 @@ public class EdgeBackGestureHandler implements PluginListener<NavigationEdgeBack 
    private final Provider<LightBarController> mLightBarControllerProvider;



    private final GestureInteractor mGestureInteractor;

    private final ArraySet<ComponentName> mBlockedActivities = new ArraySet<>();

    private Job mBlockedActivitiesJob = null;



    private final JavaAdapter mJavaAdapter;
@@ -450,9 +455,6 @@ public class EdgeBackGestureHandler implements PluginListener<NavigationEdgeBack 
        mJavaAdapter = javaAdapter;

        mLastReportedConfig.setTo(mContext.getResources().getConfiguration());



        mJavaAdapter.alwaysCollectFlow(mGestureInteractor.getGestureBlockedActivities(),

                componentNames -> updateRunningActivityGesturesBlocked());



        ComponentName recentsComponentName = ComponentName.unflattenFromString(

                context.getString(com.android.internal.R.string.config_recentsComponentName));

        if (recentsComponentName != null) {
@@ -568,12 +570,11 @@ public class EdgeBackGestureHandler implements PluginListener<NavigationEdgeBack 
        }

    }



    private void updateRunningActivityGesturesBlocked() {

    private void updateTopActivity() {

        if (edgebackGestureHandlerGetRunningTasksBackground()) {

            mBackgroundExecutor.execute(() -> mGestureBlockingActivityRunning.set(

                    isGestureBlockingActivityRunning()));

            mBackgroundExecutor.execute(() -> updateTopActivityPackageName());

        } else {

            mGestureBlockingActivityRunning.set(isGestureBlockingActivityRunning());

            updateTopActivityPackageName();

        }

    }
@@ -678,6 +679,11 @@ public class EdgeBackGestureHandler implements PluginListener<NavigationEdgeBack 
                    Log.e(TAG, "Failed to unregister window manager callbacks", e);

                }



                if (mBlockedActivitiesJob != null) {

                    mBlockedActivitiesJob.cancel(new CancellationException());

                    mBlockedActivitiesJob = null;

                }

                mBlockedActivities.clear();

            } else {

                mBackgroundExecutor.execute(mGestureNavigationSettingsObserver::register);

                updateDisplaySize();
@@ -710,6 +716,12 @@ public class EdgeBackGestureHandler implements PluginListener<NavigationEdgeBack 
                resetEdgeBackPlugin();

                mPluginManager.addPluginListener(

                        this, NavigationEdgeBackPlugin.class, /*allowMultiple=*/ false);



                // Begin listening to changes in blocked activities list

                mBlockedActivitiesJob = mJavaAdapter.alwaysCollectFlow(

                        mGestureInteractor.getTopActivityBlocked(),

                        blocked -> mGestureBlockingActivityRunning.set(blocked));



            }

            // Update the ML model resources.

            updateMLModelState();
@@ -1302,7 +1314,7 @@ public class EdgeBackGestureHandler implements PluginListener<NavigationEdgeBack 
        }

    }



    private boolean isGestureBlockingActivityRunning() {

    private void updateTopActivityPackageName() {

        ActivityManager.RunningTaskInfo runningTask =

                ActivityManagerWrapper.getInstance().getRunningTask();

        ComponentName topActivity = runningTask == null ? null : runningTask.topActivity;
@@ -1311,8 +1323,6 @@ public class EdgeBackGestureHandler implements PluginListener<NavigationEdgeBack 
        } else {

            mPackageName = "_UNKNOWN";

        }



        return topActivity != null && mGestureInteractor.areGesturesBlocked(topActivity);

    }



    public void setBackAnimation(BackAnimation backAnimation) {

packages/SystemUI/src/com/android/systemui/navigationbar/gestural/domain/GestureInteractor.kt

+52 −22
Original line number Diff line number Diff line
@@ -17,17 +17,29 @@ 
package com.android.systemui.navigationbar.gestural.domain



import android.content.ComponentName

import com.android.app.tracing.coroutines.flow.flowOn

import com.android.systemui.dagger.qualifiers.Application

import com.android.systemui.dagger.qualifiers.Background

import com.android.systemui.dagger.qualifiers.Main

import com.android.systemui.navigationbar.gestural.data.respository.GestureRepository

import com.android.systemui.shared.system.ActivityManagerWrapper

import com.android.systemui.shared.system.TaskStackChangeListener

import com.android.systemui.shared.system.TaskStackChangeListeners

import com.android.systemui.util.kotlin.combine

import com.android.systemui.util.kotlin.emitOnStart

import com.android.systemui.utils.coroutines.flow.conflatedCallbackFlow

import javax.inject.Inject

import kotlin.coroutines.CoroutineContext

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.channels.awaitClose

import kotlinx.coroutines.flow.MutableStateFlow

import kotlinx.coroutines.flow.SharingStarted

import kotlinx.coroutines.flow.StateFlow

import kotlinx.coroutines.flow.asStateFlow

import kotlinx.coroutines.flow.combine

import kotlinx.coroutines.flow.stateIn

import kotlinx.coroutines.flow.distinctUntilChanged

import kotlinx.coroutines.flow.mapLatest

import kotlinx.coroutines.launch

import kotlinx.coroutines.withContext



/**

 * {@link GestureInteractor} helps interact with gesture-related logic, including accessing the
@@ -37,7 +49,11 @@ class GestureInteractor 
@Inject

constructor(

    private val gestureRepository: GestureRepository,

    @Application private val scope: CoroutineScope

    @Main private val mainDispatcher: CoroutineDispatcher,

    @Background private val backgroundCoroutineContext: CoroutineContext,

    @Application private val scope: CoroutineScope,

    private val activityManagerWrapper: ActivityManagerWrapper,

    private val taskStackChangeListeners: TaskStackChangeListeners,

) {

    enum class Scope {

        Local,
@@ -45,16 +61,38 @@ constructor( 
    }



    private val _localGestureBlockedActivities = MutableStateFlow<Set<ComponentName>>(setOf())

    /** A {@link StateFlow} for listening to changes in Activities where gestures are blocked */

    val gestureBlockedActivities: StateFlow<Set<ComponentName>>

        get() =



    private val _topActivity =

        conflatedCallbackFlow {

                val taskListener =

                    object : TaskStackChangeListener {

                        override fun onTaskStackChanged() {

                            trySend(Unit)

                        }

                    }



                taskStackChangeListeners.registerTaskStackListener(taskListener)

                awaitClose { taskStackChangeListeners.unregisterTaskStackListener(taskListener) }

            }

            .flowOn(mainDispatcher)

            .emitOnStart()

            .mapLatest { getTopActivity() }

            .distinctUntilChanged()



    private suspend fun getTopActivity(): ComponentName? =

        withContext(backgroundCoroutineContext) {

            val runningTask = activityManagerWrapper.runningTask

            runningTask?.topActivity

        }



    val topActivityBlocked =

        combine(

            _topActivity,

            gestureRepository.gestureBlockedActivities,

            _localGestureBlockedActivities.asStateFlow()

                ) { global, local ->

                    global + local

        ) { activity, global, local ->

            activity != null && (global + local).contains(activity)

        }

                .stateIn(scope, SharingStarted.WhileSubscribed(), setOf())



    /**

     * Adds an {@link Activity} to be blocked based on component when the topmost, focused {@link
@@ -92,12 +130,4 @@ constructor( 
            }

        }

    }



    /**

     * Checks whether the specified {@link Activity} {@link ComponentName} is being blocked from

     * gestures.

     */

    fun areGesturesBlocked(activity: ComponentName): Boolean {

        return gestureBlockedActivities.value.contains(activity)

    }

}

packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/gesture/domain/GestureInteractorKosmos.kt

+13 −2
Original line number Diff line number Diff line
@@ -16,12 +16,23 @@ 


package com.android.systemui.keyguard.gesture.domain



import com.android.systemui.keyguard.gesture.data.gestureRepository

import com.android.systemui.kosmos.Kosmos

import com.android.systemui.kosmos.applicationCoroutineScope

import com.android.systemui.kosmos.backgroundCoroutineContext

import com.android.systemui.kosmos.testDispatcher

import com.android.systemui.navigationbar.gestural.data.gestureRepository

import com.android.systemui.navigationbar.gestural.domain.GestureInteractor

import com.android.systemui.shared.system.activityManagerWrapper

import com.android.systemui.shared.system.taskStackChangeListeners



val Kosmos.gestureInteractor: GestureInteractor by

    Kosmos.Fixture {

        GestureInteractor(gestureRepository = gestureRepository, scope = applicationCoroutineScope)

        GestureInteractor(

            gestureRepository = gestureRepository,

            mainDispatcher = testDispatcher,

            backgroundCoroutineContext = backgroundCoroutineContext,

            scope = applicationCoroutineScope,

            activityManagerWrapper = activityManagerWrapper,

            taskStackChangeListeners = taskStackChangeListeners

        )

    }

packages/SystemUI/tests/utils/src/com/android/systemui/keyguard/gesture/data/GestureRepositoryKosmos.kt→packages/SystemUI/tests/utils/src/com/android/systemui/navigationbar/gestural/data/GestureRepositoryKosmos.kt

+1 −1
Original line number Diff line number Diff line
@@ -14,7 +14,7 @@ 
 * limitations under the License.

 */



package com.android.systemui.keyguard.gesture.data

package com.android.systemui.navigationbar.gestural.data



import com.android.systemui.kosmos.Kosmos

import com.android.systemui.kosmos.testDispatcher