Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid references to the Fragment's Views after onDestroyView() #785

Merged
merged 1 commit into from Jan 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -87,6 +87,13 @@ private void subscribeToModel(final ProductViewModel model) {
});
}

@Override
public void onDestroyView() {
mBinding = null;
mCommentAdapter = null;
super.onDestroyView();
}

/** Creates product fragment for specific product ID */
public static ProductFragment forProduct(int productId) {
ProductFragment fragment = new ProductFragment();
Expand Down
Expand Up @@ -90,6 +90,13 @@ private void subscribeUi(LiveData<List<ProductEntity>> liveData) {
});
}

@Override
public void onDestroyView() {
mBinding = null;
mProductAdapter = null;
super.onDestroyView();
}

private final ProductClickCallback mProductClickCallback = product -> {
if (getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.STARTED)) {
((MainActivity) requireActivity()).show(product);
Expand Down
Expand Up @@ -15,6 +15,10 @@
*/
package com.android.example.github.util

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.fragment.app.DialogFragment
import androidx.fragment.app.Fragment
import androidx.test.platform.app.InstrumentationRegistry
Expand Down Expand Up @@ -47,7 +51,6 @@ class AutoClearedValueTest {

@Test
fun clearOnReplace() {
testFragment.testValue = "foo"
activityRule.activity.replaceFragment(TestFragment())
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
try {
Expand All @@ -57,9 +60,22 @@ class AutoClearedValueTest {
}
}

@Test
fun clearOnReplaceBackStack() {
activityRule.activity.replaceFragment(TestFragment(), true)
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
try {
testFragment.testValue
Assert.fail("should throw if accessed when not set")
} catch (ex: IllegalStateException) {
}
activityRule.activity.supportFragmentManager.popBackStack()
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
assertThat(testFragment.testValue, `is`("foo"))
}

@Test
fun dontClearForChildFragment() {
testFragment.testValue = "foo"
testFragment.childFragmentManager.beginTransaction()
.add(Fragment(), "foo").commit()
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
Expand All @@ -68,15 +84,20 @@ class AutoClearedValueTest {

@Test
fun dontClearForDialog() {
testFragment.testValue = "foo"
val dialogFragment = DialogFragment()
dialogFragment.show(testFragment.fragmentManager!!, "dialog")
dialogFragment.show(testFragment.parentFragmentManager, "dialog")
dialogFragment.dismiss()
InstrumentationRegistry.getInstrumentation().waitForIdleSync()
assertThat(testFragment.testValue, `is`("foo"))
}

class TestFragment : Fragment() {
var testValue by autoCleared<String>()

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?,
savedInstanceState: Bundle?): View? {
testValue = "foo"
return View(context)
}
}
}
Expand Up @@ -46,9 +46,14 @@ class SingleFragmentActivity : AppCompatActivity() {
.commit()
}

fun replaceFragment(fragment: Fragment) {
fun replaceFragment(fragment: Fragment, addToBackStack: Boolean = false) {
supportFragmentManager.beginTransaction()
.replace(R.id.container, fragment)
.apply {
if (addToBackStack) {
addToBackStack(null)
}
}
.commit()
}
}
Expand Up @@ -16,26 +16,31 @@

package com.android.example.github.util

import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleObserver
import androidx.lifecycle.OnLifecycleEvent
import androidx.fragment.app.Fragment
import androidx.lifecycle.DefaultLifecycleObserver
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.observe
import kotlin.properties.ReadWriteProperty
import kotlin.reflect.KProperty

/**
* A lazy property that gets cleaned up when the fragment is destroyed.
* A lazy property that gets cleaned up when the fragment's view is destroyed.
*
* Accessing this variable in a destroyed fragment will throw NPE.
* Accessing this variable while the fragment's view is destroyed will throw NPE.
*/
class AutoClearedValue<T : Any>(val fragment: Fragment) : ReadWriteProperty<Fragment, T> {
private var _value: T? = null

init {
fragment.lifecycle.addObserver(object : LifecycleObserver {
@OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
fun onDestroy() {
_value = null
fragment.lifecycle.addObserver(object: DefaultLifecycleObserver {
override fun onCreate(owner: LifecycleOwner) {
fragment.viewLifecycleOwnerLiveData.observe(fragment) { viewLifecycleOwner ->
viewLifecycleOwner?.lifecycle?.addObserver(object: DefaultLifecycleObserver {
override fun onDestroy(owner: LifecycleOwner) {
_value = null
}
})
}
ianhanniballake marked this conversation as resolved.
Show resolved Hide resolved
}
})
}
Expand Down
Expand Up @@ -33,17 +33,14 @@ import com.example.android.navigationadvancedsample.R
*/
class Leaderboard : Fragment() {

private lateinit var recyclerView: RecyclerView
private lateinit var viewAdapter: RecyclerView.Adapter<*>

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?,
savedInstanceState: Bundle?): View? {
// Inflate the layout for this fragment
val view = inflater.inflate(R.layout.fragment_leaderboard, container, false)

viewAdapter = MyAdapter(Array(10) { "Person ${it + 1}" })
val viewAdapter = MyAdapter(Array(10) { "Person ${it + 1}" })

recyclerView = view.findViewById<RecyclerView>(R.id.leaderboard_list).apply {
view.findViewById<RecyclerView>(R.id.leaderboard_list).run {
// use this setting to improve performance if you know that changes
// in content do not change the layout size of the RecyclerView
setHasFixedSize(true)
Expand Down
Expand Up @@ -32,24 +32,20 @@ import androidx.navigation.Navigation
*/
class Leaderboard : Fragment() {

private lateinit var recyclerView: RecyclerView
private lateinit var viewAdapter: RecyclerView.Adapter<*>

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?,
savedInstanceState: Bundle?): View? {
// Inflate the layout for this fragment
val view = inflater.inflate(R.layout.fragment_leaderboard, container, false)

viewAdapter = MyAdapter(arrayOf("Flo", "Ly", "Jo"))
val viewAdapter = MyAdapter(arrayOf("Flo", "Ly", "Jo"))

recyclerView = view.findViewById<RecyclerView>(R.id.leaderboard_list).apply {
view.findViewById<RecyclerView>(R.id.leaderboard_list).run {
// use this setting to improve performance if you know that changes
// in content do not change the layout size of the RecyclerView
setHasFixedSize(true)

// specify an viewAdapter (see also next example)
adapter = viewAdapter

}
return view
}
Expand Down