Fixes #541: Crash on Home Screen

The Android Lifecycle Architecture component does not have fine-grained enough lifecycle event callbacks to safely manage Rx subscriptions in Fragment lifecycles. Added autodispose to simplify.

app/build.gradle

@@ -165,6 +165,8 @@ dependencies {
     implementation Deps.androidx_fragment
     implementation Deps.android_arch_navigation
     implementation Deps.android_arch_navigation_ui
+
+    implementation Deps.autodispose
 }
 
 

app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt

@@ -4,7 +4,6 @@
 
 package org.mozilla.fenix.browser
 
-import android.annotation.SuppressLint
 import android.content.Context
 import android.content.Intent
 import android.os.Bundle
@@ -35,16 +34,16 @@ import org.mozilla.fenix.DefaultThemeManager
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.FindInPageIntegration
-import org.mozilla.fenix.ext.requireComponents
-import org.mozilla.fenix.ext.share
-import org.mozilla.fenix.mvi.ActionBusFactory
-import org.mozilla.fenix.mvi.getSafeManagedObservable
 import org.mozilla.fenix.components.toolbar.SearchAction
 import org.mozilla.fenix.components.toolbar.SearchState
 import org.mozilla.fenix.components.toolbar.ToolbarComponent
 import org.mozilla.fenix.components.toolbar.ToolbarIntegration
 import org.mozilla.fenix.components.toolbar.ToolbarMenu
 import org.mozilla.fenix.components.toolbar.ToolbarUIView
+import org.mozilla.fenix.ext.requireComponents
+import org.mozilla.fenix.ext.share
+import org.mozilla.fenix.mvi.ActionBusFactory
+import org.mozilla.fenix.mvi.getAutoDisposeObservable
 
 class BrowserFragment : Fragment(), BackHandler {
     private lateinit var toolbarComponent: ToolbarComponent
@@ -101,11 +100,10 @@ class BrowserFragment : Fragment(), BackHandler {
         return view
     }
 
-    @SuppressLint("CheckResult")
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
-    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onViewCreated(view, savedInstanceState)
-        super.onCreate(savedInstanceState)
 
-        getSafeManagedObservable<SearchAction>()
+        getAutoDisposeObservable<SearchAction>()
             .subscribe {
                 when (it) {
                     is SearchAction.ToolbarTapped -> Navigation.findNavController(toolbar)
@@ -116,10 +114,6 @@ class BrowserFragment : Fragment(), BackHandler {
                     is SearchAction.ToolbarMenuItemTapped -> handleToolbarItemInteraction(it)
                 }
             }
-    }
-
-    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
-        super.onViewCreated(view, savedInstanceState)
 
         sessionId = BrowserFragmentArgs.fromBundle(arguments!!).sessionId
 

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -4,7 +4,6 @@
 
 package org.mozilla.fenix.home
 
-import android.annotation.SuppressLint
 import android.content.res.Resources
 import android.graphics.drawable.BitmapDrawable
 import android.os.Bundle
@@ -20,8 +19,8 @@ import kotlinx.android.synthetic.main.fragment_home.view.*
 import mozilla.components.browser.menu.BrowserMenu
 import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
-import org.mozilla.fenix.DefaultThemeManager
 import org.mozilla.fenix.BrowsingModeManager
+import org.mozilla.fenix.DefaultThemeManager
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.ext.requireComponents
@@ -32,8 +31,8 @@ import org.mozilla.fenix.home.tabs.TabsComponent
 import org.mozilla.fenix.home.tabs.TabsState
 import org.mozilla.fenix.isPrivate
 import org.mozilla.fenix.mvi.ActionBusFactory
+import org.mozilla.fenix.mvi.getAutoDisposeObservable
 import org.mozilla.fenix.mvi.getManagedEmitter
-import org.mozilla.fenix.mvi.getSafeManagedObservable
 import kotlin.math.roundToInt
 
 class HomeFragment : Fragment() {
@@ -57,14 +56,13 @@ class HomeFragment : Fragment() {
         return view
     }
 
-    @SuppressLint("CheckResult")
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
 
         (activity as AppCompatActivity).supportActionBar?.hide()
         setupHomeMenu()
 
-        getSafeManagedObservable<TabsAction>()
+        getAutoDisposeObservable<TabsAction>()
             .subscribe {
                 when (it) {
                     is TabsAction.Select -> {

app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt

@@ -4,7 +4,6 @@
 
 package org.mozilla.fenix.library.history
 
-import android.annotation.SuppressLint
 import android.os.Bundle
 import android.view.LayoutInflater
 import android.view.Menu
@@ -21,13 +20,13 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.launch
-import org.mozilla.fenix.HomeActivity
 import mozilla.components.support.base.feature.BackHandler
+import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.mvi.ActionBusFactory
+import org.mozilla.fenix.mvi.getAutoDisposeObservable
 import org.mozilla.fenix.mvi.getManagedEmitter
-import org.mozilla.fenix.mvi.getSafeManagedObservable
 import kotlin.coroutines.CoroutineContext
 
 class HistoryFragment : Fragment(), CoroutineScope, BackHandler {
@@ -45,32 +44,15 @@ class HistoryFragment : Fragment(), CoroutineScope, BackHandler {
     ): View? {
         val view = inflater.inflate(R.layout.fragment_history, container, false)
         historyComponent = HistoryComponent(view.history_layout, ActionBusFactory.get(this))
-
         return view
     }
 
-    @SuppressLint("CheckResult")
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         job = Job()
 
         setHasOptionsMenu(true)
         (activity as AppCompatActivity).supportActionBar?.show()
-
-        getSafeManagedObservable<HistoryAction>()
-            .subscribe {
-                when (it) {
-                    is HistoryAction.Select -> selectItem(it.item)
-                    is HistoryAction.EnterEditMode -> getManagedEmitter<HistoryChange>()
-                        .onNext(HistoryChange.EnterEditMode(it.item))
-                    is HistoryAction.AddItemForRemoval -> getManagedEmitter<HistoryChange>()
-                        .onNext(HistoryChange.AddItemForRemoval(it.item))
-                    is HistoryAction.RemoveItemForRemoval -> getManagedEmitter<HistoryChange>()
-                        .onNext(HistoryChange.RemoveItemForRemoval(it.item))
-                    is HistoryAction.BackPressed -> getManagedEmitter<HistoryChange>()
-                        .onNext(HistoryChange.ExitEditMode)
-                }
-            }
     }
 
     private fun selectItem(item: HistoryItem) {
@@ -97,14 +79,27 @@ class HistoryFragment : Fragment(), CoroutineScope, BackHandler {
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
 
-        val eventEmitter = ActionBusFactory.get(this)
+        getAutoDisposeObservable<HistoryAction>()
+            .subscribe {
+                when (it) {
+                    is HistoryAction.Select -> selectItem(it.item)
+                    is HistoryAction.EnterEditMode -> getManagedEmitter<HistoryChange>()
+                        .onNext(HistoryChange.EnterEditMode(it.item))
+                    is HistoryAction.AddItemForRemoval -> getManagedEmitter<HistoryChange>()
+                        .onNext(HistoryChange.AddItemForRemoval(it.item))
+                    is HistoryAction.RemoveItemForRemoval -> getManagedEmitter<HistoryChange>()
+                        .onNext(HistoryChange.RemoveItemForRemoval(it.item))
+                    is HistoryAction.BackPressed -> getManagedEmitter<HistoryChange>()
+                        .onNext(HistoryChange.ExitEditMode)
+                }
+            }
 
         launch(Dispatchers.IO) {
             val items = requireComponents.core.historyStorage.getVisited()
                 .mapIndexed { id, item -> HistoryItem(id, item) }
 
             launch(Dispatchers.Main) {
-                eventEmitter.emit(HistoryChange::class.java, HistoryChange.Change(items))
+                getManagedEmitter<HistoryChange>().onNext(HistoryChange.Change(items))
             }
         }
     }

app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt

@@ -4,7 +4,6 @@
 
 package org.mozilla.fenix.search
 
-import android.annotation.SuppressLint
 import android.os.Bundle
 import android.view.LayoutInflater
 import android.view.View
@@ -15,17 +14,17 @@ import androidx.navigation.Navigation
 import kotlinx.android.synthetic.main.fragment_search.view.*
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
-import org.mozilla.fenix.mvi.ActionBusFactory
-import org.mozilla.fenix.mvi.getManagedEmitter
-import org.mozilla.fenix.mvi.getSafeManagedObservable
-import org.mozilla.fenix.search.awesomebar.AwesomeBarAction
-import org.mozilla.fenix.search.awesomebar.AwesomeBarChange
-import org.mozilla.fenix.search.awesomebar.AwesomeBarComponent
-import org.mozilla.fenix.search.awesomebar.AwesomeBarState
 import org.mozilla.fenix.components.toolbar.SearchAction
 import org.mozilla.fenix.components.toolbar.SearchState
 import org.mozilla.fenix.components.toolbar.ToolbarComponent
 import org.mozilla.fenix.components.toolbar.ToolbarUIView
+import org.mozilla.fenix.mvi.ActionBusFactory
+import org.mozilla.fenix.mvi.getAutoDisposeObservable
+import org.mozilla.fenix.mvi.getManagedEmitter
+import org.mozilla.fenix.search.awesomebar.AwesomeBarAction
+import org.mozilla.fenix.search.awesomebar.AwesomeBarChange
+import org.mozilla.fenix.search.awesomebar.AwesomeBarComponent
+import org.mozilla.fenix.search.awesomebar.AwesomeBarState
 
 class SearchFragment : Fragment() {
     private lateinit var toolbarComponent: ToolbarComponent
@@ -54,7 +53,6 @@ class SearchFragment : Fragment() {
         return view
     }
 
-    @SuppressLint("CheckResult")
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
 
@@ -66,7 +64,7 @@ class SearchFragment : Fragment() {
 
         view.toolbar_wrapper.clipToOutline = false
 
-        getSafeManagedObservable<SearchAction>()
+        getAutoDisposeObservable<SearchAction>()
             .subscribe {
                 when (it) {
                     is SearchAction.UrlCommitted -> transitionToBrowser()
@@ -76,7 +74,7 @@ class SearchFragment : Fragment() {
                 }
             }
 
-        getSafeManagedObservable<AwesomeBarAction>()
+        getAutoDisposeObservable<AwesomeBarAction>()
             .subscribe {
                 when (it) {
                     is AwesomeBarAction.ItemSelected -> transitionToBrowser()

architecture/build.gradle

@@ -37,4 +37,8 @@ dependencies {
 
     implementation Deps.rxAndroid
     implementation Deps.rxKotlin
+
+    implementation Deps.autodispose
+    implementation Deps.autodispose_android
+    implementation Deps.autodispose_android_aac
 }

architecture/src/main/java/org/mozilla/fenix/mvi/ActionBusFactory.kt

@@ -26,6 +26,9 @@ import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.LifecycleObserver
 import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.OnLifecycleEvent
+import com.uber.autodispose.AutoDispose.autoDisposable
+import com.uber.autodispose.ObservableSubscribeProxy
+import com.uber.autodispose.android.lifecycle.AndroidLifecycleScopeProvider
 import io.reactivex.Observable
 import io.reactivex.Observer
 import io.reactivex.rxkotlin.merge
@@ -103,6 +106,11 @@ class ActionBusFactory private constructor(val owner: LifecycleOwner) {
         return if (map[clazz] != null) map[clazz] as Observable<T> else create(clazz)
     }
 
+    fun <T : Action> getAutoDisposeObservable(clazz: Class<T>): ObservableSubscribeProxy<T> {
+        return getSafeManagedObservable(clazz)
+            .`as`(autoDisposable(AndroidLifecycleScopeProvider.from(owner)))
+    }
+
     fun <T : Action> getManagedEmitter(clazz: Class<T>): Observer<T> {
         return if (map[clazz] != null) map[clazz] as Observer<T> else create(clazz)
     }
@@ -135,6 +143,9 @@ inline fun <reified T : Action> LifecycleOwner.emit(event: T) =
 inline fun <reified T : Action> LifecycleOwner.getSafeManagedObservable(): Observable<T> =
     ActionBusFactory.get(this).getSafeManagedObservable(T::class.java)
 
+inline fun <reified T : Action> LifecycleOwner.getAutoDisposeObservable(): ObservableSubscribeProxy<T> =
+    ActionBusFactory.get(this).getAutoDisposeObservable(T::class.java)
+
 inline fun <reified T : Action> LifecycleOwner.getManagedEmitter(): Observer<T> =
     ActionBusFactory.get(this).getManagedEmitter(T::class.java)
 

buildSrc/src/main/java/Dependencies.kt

@@ -28,6 +28,8 @@ private object Versions {
     const val espresso_core = "2.2.2"
 
     const val android_arch_navigation = "1.0.0-beta02"
+
+    const val autodispose = "1.1.0"
 }
 
 @Suppress("unused")
@@ -109,5 +111,9 @@ object Deps {
     const val android_arch_navigation = "android.arch.navigation:navigation-fragment:${Versions.android_arch_navigation}"
     const val android_arch_navigation_ui = "android.arch.navigation:navigation-ui:${Versions.android_arch_navigation}"
 
+    const val autodispose = "com.uber.autodispose:autodispose:${Versions.autodispose}"
+    const val autodispose_android = "com.uber.autodispose:autodispose-android:${Versions.autodispose}"
+    const val autodispose_android_aac = "com.uber.autodispose:autodispose-android-archcomponents:${Versions.autodispose}"
+    const val autodispose_android_aac_test = "com.uber.autodispose:autodispose-android-archcomponents-test:${Versions.autodispose}"
 }