PSS: Fix MediaProjectionTaskView leaking in KeyguardController

TaskPreviewSizeProvider was registering itself as a configuration
changes listener when created, and never unregistering itself again.
Since a new one is created every time the app chooser is created,
it would create a new leak every time.
The class indirectly references MediaProjectionTaskView, which itself
holds onto large Bitmaps, which can make the leak costly.

Bug: 290754514
Test: Manually - Took heap dump before and after the fix
Test: TaskPreviewSizeProviderTest
Change-Id: Id03d9b3b5a94193f0e5d75a6ff602dd74b45f40c
This commit is contained in:
Chris Göllner
2023-07-14 10:31:41 +01:00
parent fe395a0ec7
commit 1dde493fea
4 changed files with 73 additions and 9 deletions

View File

@@ -30,6 +30,9 @@ import android.os.IBinder
import android.os.ResultReceiver
import android.os.UserHandle
import android.view.ViewGroup
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.LifecycleRegistry
import com.android.internal.annotations.VisibleForTesting
import com.android.internal.app.AbstractMultiProfilePagerAdapter.EmptyStateProvider
import com.android.internal.app.AbstractMultiProfilePagerAdapter.MyUserIdProvider
@@ -54,7 +57,11 @@ class MediaProjectionAppSelectorActivity(
/** This is used to override the dependency in a screenshot test */
@VisibleForTesting
private val listControllerFactory: ((userHandle: UserHandle) -> ResolverListController)?
) : ChooserActivity(), MediaProjectionAppSelectorView, MediaProjectionAppSelectorResultHandler {
) :
ChooserActivity(),
MediaProjectionAppSelectorView,
MediaProjectionAppSelectorResultHandler,
LifecycleOwner {
@Inject
constructor(
@@ -62,6 +69,8 @@ class MediaProjectionAppSelectorActivity(
activityLauncher: AsyncActivityLauncher
) : this(componentFactory, activityLauncher, listControllerFactory = null)
private val lifecycleRegistry = LifecycleRegistry(this)
override val lifecycle = lifecycleRegistry
private lateinit var configurationController: ConfigurationController
private lateinit var controller: MediaProjectionAppSelectorController
private lateinit var recentsViewController: MediaProjectionRecentsViewController
@@ -75,7 +84,9 @@ class MediaProjectionAppSelectorActivity(
override fun getLayoutResource() = R.layout.media_projection_app_selector
public override fun onCreate(bundle: Bundle?) {
lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_CREATE)
component = componentFactory.create(activity = this, view = this, resultHandler = this)
component.lifecycleObservers.forEach { lifecycle.addObserver(it) }
// Create a separate configuration controller for this activity as the configuration
// might be different from the global one
@@ -96,6 +107,26 @@ class MediaProjectionAppSelectorActivity(
controller.init()
}
override fun onStart() {
super.onStart()
lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_START)
}
override fun onResume() {
super.onResume()
lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_RESUME)
}
override fun onPause() {
lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_PAUSE)
super.onPause()
}
override fun onStop() {
lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_STOP)
super.onStop()
}
override fun onConfigurationChanged(newConfig: Configuration) {
super.onConfigurationChanged(newConfig)
configurationController.onConfigurationChanged(newConfig)
@@ -152,6 +183,8 @@ class MediaProjectionAppSelectorActivity(
}
override fun onDestroy() {
lifecycleRegistry.handleLifecycleEvent(Lifecycle.Event.ON_DESTROY)
component.lifecycleObservers.forEach { lifecycle.removeObserver(it) }
// onDestroy is also called when an app is selected, in that case we only want to send
// RECORD_CONTENT_TASK but not RECORD_CANCEL
if (!taskSelected) {

View File

@@ -20,6 +20,7 @@ import android.app.Activity
import android.content.ComponentName
import android.content.Context
import android.os.UserHandle
import androidx.lifecycle.DefaultLifecycleObserver
import com.android.launcher3.icons.IconFactory
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.media.MediaProjectionAppSelectorActivity
@@ -46,6 +47,7 @@ import dagger.Provides
import dagger.Subcomponent
import dagger.multibindings.ClassKey
import dagger.multibindings.IntoMap
import dagger.multibindings.IntoSet
import javax.inject.Qualifier
import javax.inject.Scope
import kotlinx.coroutines.CoroutineScope
@@ -100,6 +102,12 @@ interface MediaProjectionAppSelectorModule {
@MediaProjectionAppSelectorScope
fun bindAppIconLoader(impl: IconLoaderLibAppIconLoader): AppIconLoader
@Binds
@IntoSet
fun taskPreviewSizeProviderAsLifecycleObserver(
impl: TaskPreviewSizeProvider
): DefaultLifecycleObserver
companion object {
@Provides
@MediaProjectionAppSelector
@@ -166,4 +174,5 @@ interface MediaProjectionAppSelectorComponent {
@get:PersonalProfile val personalProfileUserHandle: UserHandle
@MediaProjectionAppSelector val configurationController: ConfigurationController
val lifecycleObservers: Set<DefaultLifecycleObserver>
}

View File

@@ -21,6 +21,8 @@ import android.content.res.Configuration
import android.graphics.Rect
import android.view.WindowInsets.Type
import android.view.WindowManager
import androidx.lifecycle.DefaultLifecycleObserver
import androidx.lifecycle.LifecycleOwner
import com.android.systemui.mediaprojection.appselector.MediaProjectionAppSelectorScope
import com.android.systemui.mediaprojection.appselector.view.TaskPreviewSizeProvider.TaskPreviewSizeListener
import com.android.systemui.shared.recents.utilities.Utilities.isLargeScreen
@@ -35,18 +37,22 @@ class TaskPreviewSizeProvider
constructor(
private val context: Context,
private val windowManager: WindowManager,
configurationController: ConfigurationController
) : CallbackController<TaskPreviewSizeListener>, ConfigurationListener {
private val configurationController: ConfigurationController,
) : CallbackController<TaskPreviewSizeListener>, ConfigurationListener, DefaultLifecycleObserver {
/** Returns the size of the task preview on the screen in pixels */
val size: Rect = calculateSize()
private val listeners = arrayListOf<TaskPreviewSizeListener>()
init {
override fun onCreate(owner: LifecycleOwner) {
configurationController.addCallback(this)
}
override fun onDestroy(owner: LifecycleOwner) {
configurationController.removeCallback(this)
}
override fun onConfigChanged(newConfig: Configuration) {
val newSize = calculateSize()
if (newSize != size) {

View File

@@ -26,6 +26,7 @@ import android.view.WindowInsets
import android.view.WindowManager
import android.view.WindowMetrics
import androidx.core.view.WindowInsetsCompat.Type
import androidx.lifecycle.LifecycleOwner
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.mediaprojection.appselector.view.TaskPreviewSizeProvider.TaskPreviewSizeListener
@@ -41,9 +42,10 @@ import org.junit.Test
@SmallTest
class TaskPreviewSizeProviderTest : SysuiTestCase() {
private val mockContext: Context = mock()
private val resources: Resources = mock()
private val windowManager: WindowManager = mock()
private val lifecycleOwner = mock<LifecycleOwner>()
private val mockContext = mock<Context>()
private val resources = mock<Resources>()
private val windowManager = mock<WindowManager>()
private val sizeUpdates = arrayListOf<Rect>()
private val testConfigurationController = FakeConfigurationController()
@@ -76,7 +78,7 @@ class TaskPreviewSizeProviderTest : SysuiTestCase() {
@Test
fun size_phoneDisplayAndRotate_emitsSizeUpdate() {
givenDisplay(width = 400, height = 600, isTablet = false)
createSizeProvider()
createSizeProvider().also { it.onCreate(lifecycleOwner) }
givenDisplay(width = 600, height = 400, isTablet = false)
testConfigurationController.onConfigurationChanged(Configuration())
@@ -87,7 +89,7 @@ class TaskPreviewSizeProviderTest : SysuiTestCase() {
@Test
fun size_phoneDisplayAndRotateConfigurationChange_returnsUpdatedSize() {
givenDisplay(width = 400, height = 600, isTablet = false)
val sizeProvider = createSizeProvider()
val sizeProvider = createSizeProvider().also { it.onCreate(lifecycleOwner) }
givenDisplay(width = 600, height = 400, isTablet = false)
testConfigurationController.onConfigurationChanged(Configuration())
@@ -95,6 +97,20 @@ class TaskPreviewSizeProviderTest : SysuiTestCase() {
assertThat(sizeProvider.size).isEqualTo(Rect(0, 0, 150, 100))
}
@Test
fun size_phoneDisplayAndRotateConfigurationChange_afterChooserDestroyed_doesNotUpdate() {
givenDisplay(width = 400, height = 600, isTablet = false)
val sizeProvider = createSizeProvider()
val previousSize = Rect(sizeProvider.size)
sizeProvider.onCreate(lifecycleOwner)
sizeProvider.onDestroy(lifecycleOwner)
givenDisplay(width = 600, height = 400, isTablet = false)
testConfigurationController.onConfigurationChanged(Configuration())
assertThat(sizeProvider.size).isEqualTo(previousSize)
}
private fun givenTaskbarSize(size: Int) {
val windowInsets =
WindowInsets.Builder()