Browse Source

Add method in NewBaseController to handle NPE catches for viewbinding

This is really a symptom of unreliable design around the async logic in Controllers. The data async calls should have been separate from the controller,
so that the only asynchronicity the Controller has to deal with is UI-related, and can be cancelled safely on destroyView.

In the future as those controllers are converted to something that has a separate ViewModel, this kind of catch should be completely unneeded.

Signed-off-by: Álvaro Brey <alvaro.brey@nextcloud.com>
Álvaro Brey 2 years ago
parent
commit
8e3dc066b2

+ 6 - 34
app/src/main/java/com/nextcloud/talk/controllers/ContactsController.kt

@@ -384,7 +384,6 @@ class ContactsController(args: Bundle) :
         }
         }
     }
     }
 
 
-    @Suppress("Detekt.TooGenericExceptionCaught")
     private fun fetchData() {
     private fun fetchData() {
         dispose(null)
         dispose(null)
         alreadyFetching = true
         alreadyFetching = true
@@ -440,33 +439,21 @@ class ContactsController(args: Bundle) :
                         adapter?.filterItems()
                         adapter?.filterItems()
                     }
                     }
 
 
-                    try {
+                    withNullableControllerViewBinding {
                         binding.controllerGenericRv.swipeRefreshLayout.isRefreshing = false
                         binding.controllerGenericRv.swipeRefreshLayout.isRefreshing = false
-                    } catch (npe: NullPointerException) {
-                        // view binding can be null
-                        // since this is called asynchronously and UI might have been destroyed in the meantime
-                        Log.i(TAG, "UI destroyed - view binding already gone")
                     }
                     }
                 }
                 }
 
 
                 override fun onError(e: Throwable) {
                 override fun onError(e: Throwable) {
-                    try {
+                    withNullableControllerViewBinding {
                         binding.controllerGenericRv.swipeRefreshLayout.isRefreshing = false
                         binding.controllerGenericRv.swipeRefreshLayout.isRefreshing = false
-                    } catch (npe: NullPointerException) {
-                        // view binding can be null
-                        // since this is called asynchronously and UI might have been destroyed in the meantime
-                        Log.i(TAG, "UI destroyed - view binding already gone")
                     }
                     }
                     dispose(contactsQueryDisposable)
                     dispose(contactsQueryDisposable)
                 }
                 }
 
 
                 override fun onComplete() {
                 override fun onComplete() {
-                    try {
+                    withNullableControllerViewBinding {
                         binding.controllerGenericRv.swipeRefreshLayout.isRefreshing = false
                         binding.controllerGenericRv.swipeRefreshLayout.isRefreshing = false
-                    } catch (npe: NullPointerException) {
-                        // view binding can be null
-                        // since this is called asynchronously and UI might have been destroyed in the meantime
-                        Log.i(TAG, "UI destroyed - view binding already gone")
                     }
                     }
                     dispose(contactsQueryDisposable)
                     dispose(contactsQueryDisposable)
                     alreadyFetching = false
                     alreadyFetching = false
@@ -692,7 +679,6 @@ class ContactsController(args: Bundle) :
         dispose(null)
         dispose(null)
     }
     }
 
 
-    @Suppress("Detekt.TooGenericExceptionCaught")
     override fun onQueryTextChange(newText: String): Boolean {
     override fun onQueryTextChange(newText: String): Boolean {
         if (newText != "" && adapter?.hasNewFilter(newText) == true) {
         if (newText != "" && adapter?.hasNewFilter(newText) == true) {
             adapter?.setFilter(newText)
             adapter?.setFilter(newText)
@@ -702,12 +688,8 @@ class ContactsController(args: Bundle) :
             adapter?.updateDataSet(contactItems as List<Nothing>?)
             adapter?.updateDataSet(contactItems as List<Nothing>?)
         }
         }
 
 
-        try {
+        withNullableControllerViewBinding {
             binding.controllerGenericRv.swipeRefreshLayout.isEnabled = !adapter!!.hasFilter()
             binding.controllerGenericRv.swipeRefreshLayout.isEnabled = !adapter!!.hasFilter()
-        } catch (npe: NullPointerException) {
-            // view binding can be null
-            // since this is called asynchronously and UI might have been destroyed in the meantime
-            Log.i(TAG, "UI destroyed - view binding already gone")
         }
         }
 
 
         return true
         return true
@@ -933,9 +915,8 @@ class ContactsController(args: Bundle) :
         }
         }
     }
     }
 
 
-    @Suppress("Detekt.TooGenericExceptionCaught")
     private fun toggleConversationPrivacyLayout(showInitialLayout: Boolean) {
     private fun toggleConversationPrivacyLayout(showInitialLayout: Boolean) {
-        try {
+        withNullableControllerViewBinding {
             if (showInitialLayout) {
             if (showInitialLayout) {
                 binding.conversationPrivacyToggle.initialRelativeLayout.visibility = View.VISIBLE
                 binding.conversationPrivacyToggle.initialRelativeLayout.visibility = View.VISIBLE
                 binding.conversationPrivacyToggle.secondaryRelativeLayout.visibility = View.GONE
                 binding.conversationPrivacyToggle.secondaryRelativeLayout.visibility = View.GONE
@@ -943,26 +924,17 @@ class ContactsController(args: Bundle) :
                 binding.conversationPrivacyToggle.initialRelativeLayout.visibility = View.GONE
                 binding.conversationPrivacyToggle.initialRelativeLayout.visibility = View.GONE
                 binding.conversationPrivacyToggle.secondaryRelativeLayout.visibility = View.VISIBLE
                 binding.conversationPrivacyToggle.secondaryRelativeLayout.visibility = View.VISIBLE
             }
             }
-        } catch (npe: NullPointerException) {
-            // view binding can be null
-            // since this is called asynchronously and UI might have been destroyed in the meantime
-            Log.i(TAG, "UI destroyed - view binding already gone")
         }
         }
     }
     }
 
 
-    @Suppress("Detekt.TooGenericExceptionCaught")
     private fun toggleConversationViaLinkVisibility(isPublicCall: Boolean) {
     private fun toggleConversationViaLinkVisibility(isPublicCall: Boolean) {
-        try {
+        withNullableControllerViewBinding {
             if (isPublicCall) {
             if (isPublicCall) {
                 binding.joinConversationViaLink.joinConversationViaLinkRelativeLayout.visibility = View.GONE
                 binding.joinConversationViaLink.joinConversationViaLinkRelativeLayout.visibility = View.GONE
                 updateGroupParticipantSelection()
                 updateGroupParticipantSelection()
             } else {
             } else {
                 binding.joinConversationViaLink.joinConversationViaLinkRelativeLayout.visibility = View.VISIBLE
                 binding.joinConversationViaLink.joinConversationViaLinkRelativeLayout.visibility = View.VISIBLE
             }
             }
-        } catch (npe: NullPointerException) {
-            // view binding can be null
-            // since this is called asynchronously and UI might have been destroyed in the meantime
-            Log.i(TAG, "UI destroyed - view binding already gone")
         }
         }
     }
     }
 
 

+ 22 - 1
app/src/main/java/com/nextcloud/talk/controllers/base/NewBaseController.kt

@@ -54,10 +54,10 @@ import com.nextcloud.talk.controllers.ServerSelectionController
 import com.nextcloud.talk.controllers.SwitchAccountController
 import com.nextcloud.talk.controllers.SwitchAccountController
 import com.nextcloud.talk.controllers.WebViewLoginController
 import com.nextcloud.talk.controllers.WebViewLoginController
 import com.nextcloud.talk.controllers.base.providers.ActionBarProvider
 import com.nextcloud.talk.controllers.base.providers.ActionBarProvider
+import com.nextcloud.talk.controllers.util.ControllerViewBindingDelegate
 import com.nextcloud.talk.databinding.ActivityMainBinding
 import com.nextcloud.talk.databinding.ActivityMainBinding
 import com.nextcloud.talk.utils.DisplayUtils
 import com.nextcloud.talk.utils.DisplayUtils
 import com.nextcloud.talk.utils.preferences.AppPreferences
 import com.nextcloud.talk.utils.preferences.AppPreferences
-import java.util.ArrayList
 import javax.inject.Inject
 import javax.inject.Inject
 import kotlin.jvm.internal.Intrinsics
 import kotlin.jvm.internal.Intrinsics
 
 
@@ -296,6 +296,27 @@ abstract class NewBaseController(@LayoutRes var layoutRes: Int, args: Bundle? =
         }
         }
     }
     }
 
 
+    /**
+     * Mainly intended to be used in async listeners that may be called after the controller has been destroyed.
+     *
+     * If you need to use this function to patch a NPE crash, something is wrong in the way that the async calls are
+     * handled, they should have been cancelled when the controller UI was destroyed (if their only purpose was
+     * updating UI).
+     */
+    @Suppress("Detekt.TooGenericExceptionCaught")
+    inline fun withNullableControllerViewBinding(block: () -> Unit) {
+        try {
+            block()
+        } catch (e: NullPointerException) {
+            // Handle only the exceptions we know about, let everything else pass through
+            if (e.stackTrace.firstOrNull()?.className == ControllerViewBindingDelegate::class.qualifiedName) {
+                Log.w("ControllerViewBinding", "Trying to update UI on a null ViewBinding.", e)
+            } else {
+                throw e
+            }
+        }
+    }
+
     open val appBarLayoutType: AppBarLayoutType
     open val appBarLayoutType: AppBarLayoutType
         get() = AppBarLayoutType.TOOLBAR
         get() = AppBarLayoutType.TOOLBAR
     val searchHint: String
     val searchHint: String