Fixes activity leak in Wallpaper Picker.
Fix: 280060800
Test: unit tests expanded
Test: manually verified that long-pressing an affordance icon brings up
its configuration activity.
Test: manually verified that tapping the button in an enablement dialog
that's shown when the user clicks on a disabled affordance icon opens
its target activity.
Change-Id: Ie36566d777276458531f304dfd82aa2ca6a86379
diff --git a/src/com/android/customization/module/ThemePickerInjector.kt b/src/com/android/customization/module/ThemePickerInjector.kt
index 7f27650..75d88d6 100644
--- a/src/com/android/customization/module/ThemePickerInjector.kt
+++ b/src/com/android/customization/module/ThemePickerInjector.kt
@@ -255,9 +255,7 @@
getKeyguardQuickAffordancePickerInteractor(context),
getWallpaperInteractor(context),
getCurrentWallpaperInfoFactory(context),
- ) { intent ->
- context.startActivity(intent)
- }
+ )
.also { keyguardQuickAffordancePickerViewModelFactory = it }
}
diff --git a/src/com/android/customization/picker/quickaffordance/ui/binder/KeyguardQuickAffordancePickerBinder.kt b/src/com/android/customization/picker/quickaffordance/ui/binder/KeyguardQuickAffordancePickerBinder.kt
index 4395f5e..68367c8 100644
--- a/src/com/android/customization/picker/quickaffordance/ui/binder/KeyguardQuickAffordancePickerBinder.kt
+++ b/src/com/android/customization/picker/quickaffordance/ui/binder/KeyguardQuickAffordancePickerBinder.kt
@@ -126,6 +126,15 @@
}
}
}
+
+ launch {
+ viewModel.activityStartRequests.collect { intent ->
+ if (intent != null) {
+ view.context.startActivity(intent)
+ viewModel.onActivityStarted()
+ }
+ }
+ }
}
}
}
diff --git a/src/com/android/customization/picker/quickaffordance/ui/viewmodel/KeyguardQuickAffordancePickerViewModel.kt b/src/com/android/customization/picker/quickaffordance/ui/viewmodel/KeyguardQuickAffordancePickerViewModel.kt
index 0a52e8f..05845f8 100644
--- a/src/com/android/customization/picker/quickaffordance/ui/viewmodel/KeyguardQuickAffordancePickerViewModel.kt
+++ b/src/com/android/customization/picker/quickaffordance/ui/viewmodel/KeyguardQuickAffordancePickerViewModel.kt
@@ -64,7 +64,6 @@
private val quickAffordanceInteractor: KeyguardQuickAffordancePickerInteractor,
private val wallpaperInteractor: WallpaperInteractor,
private val wallpaperInfoFactory: CurrentWallpaperInfoFactory,
- private val activityStarter: (Intent) -> Unit,
) : ViewModel() {
@SuppressLint("StaticFieldLeak") private val applicationContext = context.applicationContext
@@ -272,7 +271,7 @@
},
onLongClicked =
if (affordance.configureIntent != null) {
- { activityStarter(affordance.configureIntent) }
+ { requestActivityStart(affordance.configureIntent) }
} else {
null
},
@@ -318,11 +317,34 @@
*/
val dialog: Flow<DialogViewModel?> = _dialog.asStateFlow()
+ private val _activityStartRequests = MutableStateFlow<Intent?>(null)
+ /**
+ * Requests to start an activity with the given [Intent].
+ *
+ * Important: once the activity is started, the [Intent] should be consumed by calling
+ * [onActivityStarted].
+ */
+ val activityStartRequests: StateFlow<Intent?> = _activityStartRequests.asStateFlow()
+
/** Notifies that the dialog has been dismissed in the UI. */
fun onDialogDismissed() {
_dialog.value = null
}
+ /**
+ * Notifies that an activity request from [activityStartRequests] has been fulfilled (e.g. the
+ * activity was started and the view-model can forget needing to start this activity).
+ */
+ fun onActivityStarted() {
+ _activityStartRequests.value = null
+ }
+
+ private fun requestActivityStart(
+ intent: Intent,
+ ) {
+ _activityStartRequests.value = intent
+ }
+
private fun showEnablementDialog(
icon: Drawable,
name: String,
@@ -361,7 +383,7 @@
style = ButtonStyle.Primary,
onClicked = {
actionComponentName.toIntent()?.let { intent ->
- activityStarter(intent)
+ requestActivityStart(intent)
}
}
),
@@ -462,7 +484,6 @@
private val quickAffordanceInteractor: KeyguardQuickAffordancePickerInteractor,
private val wallpaperInteractor: WallpaperInteractor,
private val wallpaperInfoFactory: CurrentWallpaperInfoFactory,
- private val activityStarter: (Intent) -> Unit,
) : ViewModelProvider.Factory {
override fun <T : ViewModel> create(modelClass: Class<T>): T {
@Suppress("UNCHECKED_CAST")
@@ -471,7 +492,6 @@
quickAffordanceInteractor = quickAffordanceInteractor,
wallpaperInteractor = wallpaperInteractor,
wallpaperInfoFactory = wallpaperInfoFactory,
- activityStarter = activityStarter,
)
as T
}
diff --git a/tests/src/com/android/customization/model/picker/quickaffordance/ui/viewmodel/KeyguardQuickAffordancePickerViewModelTest.kt b/tests/src/com/android/customization/model/picker/quickaffordance/ui/viewmodel/KeyguardQuickAffordancePickerViewModelTest.kt
index 1b241ca..43ca2ab 100644
--- a/tests/src/com/android/customization/model/picker/quickaffordance/ui/viewmodel/KeyguardQuickAffordancePickerViewModelTest.kt
+++ b/tests/src/com/android/customization/model/picker/quickaffordance/ui/viewmodel/KeyguardQuickAffordancePickerViewModelTest.kt
@@ -72,8 +72,6 @@
private lateinit var quickAffordanceInteractor: KeyguardQuickAffordancePickerInteractor
private lateinit var wallpaperInteractor: WallpaperInteractor
- private var latestStartedActivityIntent: Intent? = null
-
@Before
fun setUp() {
InjectorProvider.setInjector(TestInjector())
@@ -115,7 +113,6 @@
quickAffordanceInteractor = quickAffordanceInteractor,
wallpaperInteractor = wallpaperInteractor,
wallpaperInfoFactory = TestCurrentWallpaperInfoFactory(context),
- activityStarter = { intent -> latestStartedActivityIntent = intent },
)
.create(KeyguardQuickAffordancePickerViewModel::class.java)
}
@@ -249,6 +246,7 @@
val slots = collectLastValue(underTest.slots)
val quickAffordances = collectLastValue(underTest.quickAffordances)
val dialog = collectLastValue(underTest.dialog)
+ val activityStartRequest = collectLastValue(underTest.activityStartRequests)
val enablementInstructions = listOf("instruction1", "instruction2")
val enablementActionText = "enablementActionText"
@@ -283,10 +281,15 @@
.isEqualTo(Text.Loaded(enablementActionText))
// When the button is clicked, we expect an intent of the given enablement action
- // component name is launched.
+ // component name to be emitted.
dialog()?.buttons?.first()?.onClicked?.invoke()
- assertThat(latestStartedActivityIntent?.`package`).isEqualTo(packageName)
- assertThat(latestStartedActivityIntent?.action).isEqualTo(action)
+ assertThat(activityStartRequest()?.`package`).isEqualTo(packageName)
+ assertThat(activityStartRequest()?.action).isEqualTo(action)
+
+ // Once we report that the activity was started, the activity start request should be
+ // nullified.
+ underTest.onActivityStarted()
+ assertThat(activityStartRequest()).isNull()
// Once we report that the dialog has been dismissed by the user, we expect there to be
// no dialog to be shown:
@@ -298,6 +301,7 @@
fun `Start settings activity when long-pressing an affordance`() =
testScope.runTest {
val quickAffordances = collectLastValue(underTest.quickAffordances)
+ val activityStartRequest = collectLastValue(underTest.activityStartRequests)
// Lets add a configurable affordance to the picker:
val configureIntent = Intent("some.action")
@@ -315,7 +319,11 @@
// Lets try to long-click the affordance:
quickAffordances()?.get(affordanceIndex + 1)?.onLongClicked?.invoke()
- assertThat(latestStartedActivityIntent).isEqualTo(configureIntent)
+ assertThat(activityStartRequest()).isEqualTo(configureIntent)
+ // Once we report that the activity was started, the activity start request should be
+ // nullified.
+ underTest.onActivityStarted()
+ assertThat(activityStartRequest()).isNull()
}
@Test