Synchronously clean up AndroidViewBinding fragments
When an AndroidViewBinding component that inflated
a fragment is removed from composition and receives its
onRelease call, the Fragment state should be
synchronously cleaned up so that no additional state
is left behind in cases of backward writes or the
hot reload cases used by Live Edit (which does not
wait between disposing a previous composition and
starting the new composition).
Relnote: "`AndroidViewBinding` now synchronously
removes `Fragment` instances inflated by including a
`FragmentContainerView` in your layout as part of its
`onRelease` by using `commitNow` (instead of the `commit`
it was using previously), thus fixing issues with
Live Edit's method with replacing the composition upon
changes."
Test: newly added tests pass
BUG: 296343270
Change-Id: I58fbffefb19ed896ffedc835185718a24a86949e
diff --git a/compose/ui/ui-viewbinding/samples/src/androidTest/AndroidManifest.xml b/compose/ui/ui-viewbinding/samples/src/androidTest/AndroidManifest.xml
index cc8347b..c201303d 100644
--- a/compose/ui/ui-viewbinding/samples/src/androidTest/AndroidManifest.xml
+++ b/compose/ui/ui-viewbinding/samples/src/androidTest/AndroidManifest.xml
@@ -19,5 +19,6 @@
<activity android:name="androidx.compose.ui.samples.InflatedFragmentActivity"/>
<activity android:name="androidx.compose.ui.samples.ChildInflatedFragmentActivity"/>
<activity android:name="androidx.compose.ui.samples.EmptyFragmentActivity"/>
+ <activity android:name="androidx.compose.ui.samples.ComposeInflatedFragmentActivity"/>
</application>
</manifest>
diff --git a/compose/ui/ui-viewbinding/samples/src/androidTest/java/androidx/compose/ui/samples/FragmentRemoveTest.kt b/compose/ui/ui-viewbinding/samples/src/androidTest/java/androidx/compose/ui/samples/FragmentRemoveTest.kt
index 4f82926..ec98ec8 100644
--- a/compose/ui/ui-viewbinding/samples/src/androidTest/java/androidx/compose/ui/samples/FragmentRemoveTest.kt
+++ b/compose/ui/ui-viewbinding/samples/src/androidTest/java/androidx/compose/ui/samples/FragmentRemoveTest.kt
@@ -16,17 +16,28 @@
package androidx.compose.ui.samples
+import android.os.Bundle
+import android.view.ViewGroup
+import androidx.activity.compose.setContent
+import androidx.compose.foundation.layout.requiredSize
+import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.test.junit4.createAndroidComposeRule
+import androidx.compose.ui.unit.dp
import androidx.compose.ui.viewbinding.samples.R
import androidx.compose.ui.viewbinding.samples.databinding.SampleEditTextLayoutBinding
import androidx.compose.ui.viewbinding.samples.databinding.TestFragmentLayoutBinding
import androidx.compose.ui.viewinterop.AndroidViewBinding
import androidx.fragment.app.FragmentActivity
+import androidx.lifecycle.Lifecycle
+import androidx.test.core.app.ActivityScenario
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
+import androidx.testutils.withActivity
import com.google.common.truth.Truth.assertThat
import com.google.common.truth.Truth.assertWithMessage
import org.junit.Rule
@@ -116,6 +127,97 @@
// State should be reset back to the default
assertThat(binding.editText.text.toString()).isEqualTo("Default")
}
+
+ @Test
+ fun testRemovalRemovesStateOnBackwardWrite() {
+ var showStateA by mutableStateOf(true)
+
+ rule.setContent {
+ if (showStateA) {
+ AndroidViewBinding(TestFragmentLayoutBinding::inflate)
+ } else {
+ SideEffect {
+ showStateA = true
+ }
+ }
+ }
+
+ var fragment = rule.activity.supportFragmentManager
+ .findFragmentById(R.id.fragment_container)
+ assertWithMessage("Fragment should be present when AndroidViewBinding is in the hierarchy")
+ .that(fragment)
+ .isNotNull()
+
+ var binding = SampleEditTextLayoutBinding.bind(fragment!!.requireView())
+ assertThat(binding.editText.text.toString()).isEqualTo("Default")
+
+ // Update the state to allow verifying the state is destroyed when the
+ // AndroidViewBinding is removed from composition
+ rule.runOnUiThread {
+ binding.editText.setText("Updated")
+ }
+
+ showStateA = false
+
+ rule.waitForIdle()
+
+ fragment = rule.activity.supportFragmentManager
+ .findFragmentById(R.id.fragment_container)
+ assertWithMessage("Fragment should be present when AndroidViewBinding is in the hierarchy")
+ .that(fragment)
+ .isNotNull()
+ binding = SampleEditTextLayoutBinding.bind(fragment!!.requireView())
+
+ // State should be reset back to the default
+ assertThat(binding.editText.text.toString()).isEqualTo("Default")
+ }
+
+ @Test
+ fun testRemovalRemovesStateOnCompositionDisposalAndRecreation() {
+ with(ActivityScenario.launch(ComposeInflatedFragmentActivity::class.java)) {
+ withActivity {
+ val fragment = supportFragmentManager.findFragmentById(R.id.fragment_container)!!
+ assertThat(fragment.lifecycle.currentState).isEqualTo(Lifecycle.State.RESUMED)
+ val binding = SampleEditTextLayoutBinding.bind(fragment.requireView())
+ assertThat(binding.editText.text.toString()).isEqualTo("Default")
+
+ // Update the state to make sure it gets saved and restored properly
+ binding.editText.setText("Updated")
+
+ // detach - attach to dispose composition and compose it again
+ val root = composeView!!.parent as ViewGroup
+ root.removeView(composeView)
+ root.addView(composeView)
+ }
+
+ withActivity {
+ val fragment = supportFragmentManager.findFragmentById(R.id.fragment_container)
+ assertThat(fragment).isNotNull()
+ assertThat(fragment!!.lifecycle.currentState).isEqualTo(Lifecycle.State.RESUMED)
+ val recreatedBinding = SampleEditTextLayoutBinding.bind(
+ fragment.requireView())
+ assertThat(recreatedBinding.editText.text.toString()).isEqualTo("Default")
+ }
+ }
+ }
}
class EmptyFragmentActivity : FragmentActivity()
+
+class ComposeInflatedFragmentActivity : FragmentActivity() {
+ var composeView: ComposeView? = null
+
+ override fun onCreate(savedInstanceState: Bundle?) {
+ super.onCreate(savedInstanceState)
+ setContent {
+ AndroidViewBinding(
+ TestFragmentLayoutBinding::inflate,
+ Modifier.requiredSize(50.dp),
+ )
+ }
+
+ composeView = window.decorView
+ .findViewById<ViewGroup>(android.R.id.content)
+ .getChildAt(0) as? ComposeView
+ }
+}
diff --git a/compose/ui/ui-viewbinding/src/main/java/androidx/compose/ui/viewinterop/AndroidViewBinding.kt b/compose/ui/ui-viewbinding/src/main/java/androidx/compose/ui/viewinterop/AndroidViewBinding.kt
index 1424583..5f2a288 100644
--- a/compose/ui/ui-viewbinding/src/main/java/androidx/compose/ui/viewinterop/AndroidViewBinding.kt
+++ b/compose/ui/ui-viewbinding/src/main/java/androidx/compose/ui/viewinterop/AndroidViewBinding.kt
@@ -31,7 +31,7 @@
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentActivity
import androidx.fragment.app.FragmentContainerView
-import androidx.fragment.app.commit
+import androidx.fragment.app.commitNow
import androidx.fragment.app.findFragment
import androidx.viewbinding.ViewBinding
@@ -195,7 +195,7 @@
if (existingFragment != null && !fragmentManager.isStateSaved) {
// If the state isn't saved, that means that some state change
// has removed this Composable from the hierarchy
- fragmentManager.commit {
+ fragmentManager.commitNow {
remove(existingFragment)
}
}
