fix(breadcrumbs): report the correct "previous" state

bugsnag · Mar 15, 2023 · 35bcc63 · 35bcc63
1 parent 75eca1c
commit 35bcc63
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,11 @@
 * Small improvements to the root detection overhead
   [#1815](https://github.com/bugsnag/bugsnag-android/pull/1815)
 
+### Bug fixes
+
+* Activity breadcrumbs now report the correct "previous" state
+  [#1818](https://github.com/bugsnag/bugsnag-android/pull/1818)
+
 ## 5.28.4 (2023-02-08)
 
 ### Bug fixes

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt
@@ -3,47 +3,56 @@ package com.bugsnag.android
 import android.app.Activity
 import android.app.Application
 import android.os.Bundle
+import java.util.WeakHashMap
 
 internal class ActivityBreadcrumbCollector(
     private val cb: (message: String, method: Map<String, Any>) -> Unit
 ) : Application.ActivityLifecycleCallbacks {
 
-    var prevState: String? = null
+    private val prevState = WeakHashMap<Activity, String>()
 
     override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) =
-        leaveBreadcrumb(getActivityName(activity), "onCreate()", savedInstanceState != null)
+        leaveBreadcrumb(activity, "onCreate()", savedInstanceState != null)
 
     override fun onActivityStarted(activity: Activity) =
-        leaveBreadcrumb(getActivityName(activity), "onStart()")
+        leaveBreadcrumb(activity, "onStart()")
 
     override fun onActivityResumed(activity: Activity) =
-        leaveBreadcrumb(getActivityName(activity), "onResume()")
+        leaveBreadcrumb(activity, "onResume()")
 
     override fun onActivityPaused(activity: Activity) =
-        leaveBreadcrumb(getActivityName(activity), "onPause()")
+        leaveBreadcrumb(activity, "onPause()")
 
     override fun onActivityStopped(activity: Activity) =
-        leaveBreadcrumb(getActivityName(activity), "onStop()")
+        leaveBreadcrumb(activity, "onStop()")
 
     override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) =
-        leaveBreadcrumb(getActivityName(activity), "onSaveInstanceState()")
+        leaveBreadcrumb(activity, "onSaveInstanceState()")
 
-    override fun onActivityDestroyed(activity: Activity) =
-        leaveBreadcrumb(getActivityName(activity), "onDestroy()")
+    override fun onActivityDestroyed(activity: Activity) {
+        leaveBreadcrumb(activity, "onDestroy()")
+        prevState.remove(activity)
+    }
 
     private fun getActivityName(activity: Activity) = activity.javaClass.simpleName
 
-    private fun leaveBreadcrumb(activityName: String, lifecycleCallback: String, hasBundle: Boolean? = null) {
+    private fun leaveBreadcrumb(
+        activity: Activity,
+        lifecycleCallback: String,
+        hasBundle: Boolean? = null
+    ) {
         val metadata = mutableMapOf<String, Any>()
         if (hasBundle != null) {
             metadata["hasBundle"] = hasBundle
         }
-        val previousVal = prevState
+        val previousVal = prevState[activity]
 
         if (previousVal != null) {
             metadata["previous"] = previousVal
         }
+
+        val activityName = getActivityName(activity)
         cb("$activityName#$lifecycleCallback", metadata)
-        prevState = lifecycleCallback
+        prevState[activity] = lifecycleCallback
     }
 }
diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ActivityLifecycleBreadcrumbTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ActivityLifecycleBreadcrumbTest.kt
@@ -20,6 +20,9 @@ internal class ActivityLifecycleBreadcrumbTest {
     @Mock
     lateinit var activity: Activity
 
+    @Mock
+    lateinit var activity2: Activity
+
     @Mock
     lateinit var bundle: Bundle
 
@@ -101,4 +104,20 @@ internal class ActivityLifecycleBreadcrumbTest {
         tracker.onActivityStarted(activity)
         assertEquals("onCreate()", resultMetadata!!["previous"])
     }
+
+    @Test
+    fun interleavedStateChanges() {
+        tracker.onActivityCreated(activity, null)
+        assertNull(resultMetadata!!["previous"])
+        tracker.onActivityCreated(activity2, null)
+        assertNull(resultMetadata!!["previous"])
+
+        tracker.onActivityStarted(activity)
+        assertEquals("onCreate()", resultMetadata!!["previous"])
+        tracker.onActivityResumed(activity)
+        assertEquals("onStart()", resultMetadata!!["previous"])
+
+        tracker.onActivityStarted(activity2)
+        assertEquals("onCreate()", resultMetadata!!["previous"])
+    }
 }