Skip to content

Commit

Permalink
RUMM-3500 Delay the stop view to prevent gap between fragments/activity
Browse files Browse the repository at this point in the history
  • Loading branch information
xgouchet committed Aug 17, 2023
1 parent f67b2e1 commit 07777fb
Show file tree
Hide file tree
Showing 12 changed files with 167 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.doThrow
import org.mockito.kotlin.eq
import org.mockito.kotlin.isA
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.verify
Expand Down
2 changes: 2 additions & 0 deletions detekt_custom.yml
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ datadog:
- "android.app.FragmentManager.FragmentLifecycleCallbacks.onFragmentPaused(android.app.FragmentManager, android.app.Fragment)"
- "android.app.FragmentManager.FragmentLifecycleCallbacks.onFragmentResumed(android.app.FragmentManager, android.app.Fragment)"
- "android.app.FragmentManager.FragmentLifecycleCallbacks.onFragmentStarted(android.app.FragmentManager, android.app.Fragment)"
- "android.app.FragmentManager.FragmentLifecycleCallbacks.onFragmentStopped(android.app.FragmentManager, android.app.Fragment)"
- "android.app.FragmentManager.registerFragmentLifecycleCallbacks(android.app.FragmentManager.FragmentLifecycleCallbacks, kotlin.Boolean)"
- "android.app.FragmentManager.unregisterFragmentLifecycleCallbacks(android.app.FragmentManager.FragmentLifecycleCallbacks)"
- "android.content.Context.createDeviceProtectedStorageContext()"
Expand Down Expand Up @@ -398,6 +399,7 @@ datadog:
- "androidx.fragment.app.FragmentManager.FragmentLifecycleCallbacks.onFragmentPaused(androidx.fragment.app.FragmentManager, androidx.fragment.app.Fragment)"
- "androidx.fragment.app.FragmentManager.FragmentLifecycleCallbacks.onFragmentResumed(androidx.fragment.app.FragmentManager, androidx.fragment.app.Fragment)"
- "androidx.fragment.app.FragmentManager.FragmentLifecycleCallbacks.onFragmentStarted(androidx.fragment.app.FragmentManager, androidx.fragment.app.Fragment)"
- "androidx.fragment.app.FragmentManager.FragmentLifecycleCallbacks.onFragmentStopped(androidx.fragment.app.FragmentManager, androidx.fragment.app.Fragment)"
- "androidx.fragment.app.FragmentManager.findFragmentById(kotlin.Int)"
- "androidx.fragment.app.FragmentManager.registerFragmentLifecycleCallbacks(androidx.fragment.app.FragmentManager.FragmentLifecycleCallbacks, kotlin.Boolean)"
- "androidx.fragment.app.FragmentManager.unregisterFragmentLifecycleCallbacks(androidx.fragment.app.FragmentManager.FragmentLifecycleCallbacks)"
Expand Down
2 changes: 1 addition & 1 deletion features/dd-sdk-android-rum/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ abstract class com.datadog.android.rum.tracking.ActivityLifecycleTrackingStrateg
class com.datadog.android.rum.tracking.ActivityViewTrackingStrategy : ActivityLifecycleTrackingStrategy, ViewTrackingStrategy
constructor(Boolean, ComponentPredicate<android.app.Activity> = AcceptAllActivities())
override fun onActivityResumed(android.app.Activity)
override fun onActivityPaused(android.app.Activity)
override fun onActivityPostStopped(android.app.Activity)
override fun equals(Any?): Boolean
override fun hashCode(): Int
interface com.datadog.android.rum.tracking.ComponentPredicate<T>
Expand Down
2 changes: 1 addition & 1 deletion features/dd-sdk-android-rum/api/dd-sdk-android-rum.api
Original file line number Diff line number Diff line change
Expand Up @@ -4485,7 +4485,7 @@ public final class com/datadog/android/rum/tracking/ActivityViewTrackingStrategy
public synthetic fun <init> (ZLcom/datadog/android/rum/tracking/ComponentPredicate;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
public fun hashCode ()I
public fun onActivityPaused (Landroid/app/Activity;)V
public fun onActivityPostStopped (Landroid/app/Activity;)V
public fun onActivityResumed (Landroid/app/Activity;)V
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import java.lang.ref.WeakReference
import java.util.Locale
import java.util.concurrent.TimeUnit



internal class RumViewManagerScope(
private val parentScope: RumScope,
private val sdkCore: InternalSdkCore,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,23 @@
package com.datadog.android.rum.internal.tracking

import android.os.Bundle
import androidx.annotation.MainThread
import androidx.fragment.app.DialogFragment
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentActivity
import androidx.fragment.app.FragmentManager
import com.datadog.android.api.InternalLogger
import com.datadog.android.api.SdkCore
import com.datadog.android.api.feature.FeatureSdkCore
import com.datadog.android.core.internal.thread.LoggingScheduledThreadPoolExecutor
import com.datadog.android.core.internal.utils.scheduleSafe
import com.datadog.android.rum.RumMonitor
import com.datadog.android.rum.internal.RumFeature
import com.datadog.android.rum.tracking.ComponentPredicate
import com.datadog.android.rum.utils.resolveViewName
import com.datadog.android.rum.utils.runIfValid
import java.util.concurrent.ScheduledExecutorService
import java.util.concurrent.TimeUnit

internal open class AndroidXFragmentLifecycleCallbacks(
internal val argumentsProvider: (Fragment) -> Map<String, Any?>,
Expand All @@ -28,6 +33,9 @@ internal open class AndroidXFragmentLifecycleCallbacks(
) : FragmentLifecycleCallbacks<FragmentActivity>, FragmentManager.FragmentLifecycleCallbacks() {

protected lateinit var sdkCore: FeatureSdkCore
private val executor: ScheduledExecutorService by lazy {
LoggingScheduledThreadPoolExecutor(1, internalLogger)
}

private val internalLogger: InternalLogger
get() = if (this::sdkCore.isInitialized) {
Expand All @@ -51,6 +59,7 @@ internal open class AndroidXFragmentLifecycleCallbacks(

// TODO: RUMM-0000 Update Androidx packages and handle deprecated APIs
@Suppress("DEPRECATION")
@MainThread
override fun onFragmentActivityCreated(
fm: FragmentManager,
f: Fragment,
Expand All @@ -67,6 +76,7 @@ internal open class AndroidXFragmentLifecycleCallbacks(
}
}

@MainThread
override fun onFragmentResumed(fm: FragmentManager, f: Fragment) {
super.onFragmentResumed(fm, f)
componentPredicate.runIfValid(f, internalLogger) {
Expand All @@ -77,11 +87,19 @@ internal open class AndroidXFragmentLifecycleCallbacks(
}
}

override fun onFragmentPaused(fm: FragmentManager, f: Fragment) {
super.onFragmentPaused(fm, f)
componentPredicate.runIfValid(f, internalLogger) {
val key = resolveKey(it)
rumMonitor.stopView(key)
@MainThread
override fun onFragmentStopped(fm: FragmentManager, f: Fragment) {
super.onFragmentStopped(fm, f)
executor.scheduleSafe(
"Delayed view stop",
STOP_VIEW_DELAY_MS,
TimeUnit.MILLISECONDS,
sdkCore.internalLogger
) {
componentPredicate.runIfValid(f, internalLogger) {
val key = resolveKey(it)
rumMonitor.stopView(key)
}
}
}

Expand All @@ -94,4 +112,8 @@ internal open class AndroidXFragmentLifecycleCallbacks(
}

// endregion

companion object {
private const val STOP_VIEW_DELAY_MS = 150L
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,15 @@ import com.datadog.android.api.SdkCore
import com.datadog.android.api.feature.FeatureSdkCore
import com.datadog.android.core.internal.system.BuildSdkVersionProvider
import com.datadog.android.core.internal.system.DefaultBuildSdkVersionProvider
import com.datadog.android.core.internal.thread.LoggingScheduledThreadPoolExecutor
import com.datadog.android.core.internal.utils.scheduleSafe
import com.datadog.android.rum.RumMonitor
import com.datadog.android.rum.internal.RumFeature
import com.datadog.android.rum.tracking.ComponentPredicate
import com.datadog.android.rum.utils.resolveViewName
import com.datadog.android.rum.utils.runIfValid
import java.util.concurrent.ScheduledExecutorService
import java.util.concurrent.TimeUnit

@Suppress("DEPRECATION")
@RequiresApi(Build.VERSION_CODES.O)
Expand All @@ -31,6 +35,9 @@ internal class OreoFragmentLifecycleCallbacks(
) : FragmentLifecycleCallbacks<Activity>, FragmentManager.FragmentLifecycleCallbacks() {

private lateinit var sdkCore: FeatureSdkCore
private val executor: ScheduledExecutorService by lazy {
LoggingScheduledThreadPoolExecutor(1, internalLogger)
}

private val internalLogger: InternalLogger
get() = if (this::sdkCore.isInitialized) {
Expand Down Expand Up @@ -85,12 +92,18 @@ internal class OreoFragmentLifecycleCallbacks(
}
}

override fun onFragmentPaused(fm: FragmentManager, f: Fragment) {
super.onFragmentPaused(fm, f)
override fun onFragmentStopped(fm: FragmentManager, f: Fragment) {
super.onFragmentStopped(fm, f)
if (isNotAViewFragment(f)) return

componentPredicate.runIfValid(f, internalLogger) {
rumMonitor.stopView(it)
executor.scheduleSafe(
"Delayed view stop",
STOP_VIEW_DELAY_MS,
TimeUnit.MILLISECONDS,
sdkCore.internalLogger
) {
componentPredicate.runIfValid(f, internalLogger) {
rumMonitor.stopView(it)
}
}
}

Expand All @@ -104,6 +117,7 @@ internal class OreoFragmentLifecycleCallbacks(

private companion object {
private const val REPORT_FRAGMENT_NAME = "androidx.lifecycle.ReportFragment"
private const val STOP_VIEW_DELAY_MS = 150L
}

// endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ package com.datadog.android.rum.tracking

import android.app.Activity
import androidx.annotation.MainThread
import com.datadog.android.core.internal.thread.LoggingScheduledThreadPoolExecutor
import com.datadog.android.core.internal.utils.scheduleSafe
import com.datadog.android.rum.GlobalRumMonitor
import com.datadog.android.rum.RumMonitor
import com.datadog.android.rum.utils.resolveViewName
import com.datadog.android.rum.utils.runIfValid
import java.util.concurrent.ScheduledExecutorService
import java.util.concurrent.TimeUnit

/**
* A [ViewTrackingStrategy] that will track [Activity] as RUM Views.
Expand All @@ -29,6 +33,10 @@ class ActivityViewTrackingStrategy @JvmOverloads constructor(
ActivityLifecycleTrackingStrategy(),
ViewTrackingStrategy {

private val executor: ScheduledExecutorService by lazy {
LoggingScheduledThreadPoolExecutor(1, internalLogger)
}

// region ActivityLifecycleTrackingStrategy

@MainThread
Expand All @@ -46,10 +54,17 @@ class ActivityViewTrackingStrategy @JvmOverloads constructor(
}

@MainThread
override fun onActivityPaused(activity: Activity) {
super.onActivityPaused(activity)
componentPredicate.runIfValid(activity, internalLogger) {
getRumMonitor()?.stopView(it)
override fun onActivityPostStopped(activity: Activity) {
super.onActivityPostStopped(activity)
executor.scheduleSafe(
"Delayed view stop",
STOP_VIEW_DELAY_MS,
TimeUnit.MILLISECONDS,
internalLogger
) {
componentPredicate.runIfValid(activity, internalLogger) {
getRumMonitor()?.stopView(it)
}
}
}

Expand Down Expand Up @@ -84,4 +99,8 @@ class ActivityViewTrackingStrategy @JvmOverloads constructor(
}

// endregion

internal companion object {
private const val STOP_VIEW_DELAY_MS = 150L
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import org.mockito.Mock
import org.mockito.junit.jupiter.MockitoExtension
import org.mockito.junit.jupiter.MockitoSettings
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.never
import org.mockito.kotlin.verify
import org.mockito.kotlin.verifyNoInteractions
import org.mockito.kotlin.whenever
Expand Down Expand Up @@ -212,28 +213,60 @@ internal class ActivityViewTrackingStrategyTest :
}

@Test
fun `𝕄 stop RUM View 𝕎 onActivityPaused() { first display }`() {
fun `𝕄 not stop RUM View 𝕎 onActivityPaused() { first display }`() {
// Given
testedStrategy.register(rumMonitor.mockSdkCore, mockAppContext)
whenever(mockPredicate.accept(mockActivity)) doReturn true
testedStrategy.register(rumMonitor.mockSdkCore, mockActivity)

// When
testedStrategy.onActivityPaused(mockActivity)
Thread.sleep(250)

// Then
verify(rumMonitor.mockInstance).stopView(mockActivity, emptyMap())
verify(rumMonitor.mockInstance, never()).stopView(mockActivity, emptyMap())
}

@Test
fun `𝕄 stop RUM View 𝕎 onActivityPaused() { redisplay }`() {
fun `𝕄 not stop RUM View 𝕎 onActivityPaused() { redisplay }`() {
// Given
testedStrategy.register(rumMonitor.mockSdkCore, mockAppContext)
whenever(mockPredicate.accept(mockActivity)) doReturn true
testedStrategy.register(rumMonitor.mockSdkCore, mockActivity)

// When
testedStrategy.onActivityPaused(mockActivity)
Thread.sleep(250)

// Then
verify(rumMonitor.mockInstance, never()).stopView(mockActivity, emptyMap())
}

@Test
fun `𝕄 stop RUM View 𝕎 onActivityPostStopped() { first display }`() {
// Given
testedStrategy.register(rumMonitor.mockSdkCore, mockAppContext)
whenever(mockPredicate.accept(mockActivity)) doReturn true
testedStrategy.register(rumMonitor.mockSdkCore, mockActivity)

// When
testedStrategy.onActivityPostStopped(mockActivity)
Thread.sleep(250)

// Then
verify(rumMonitor.mockInstance).stopView(mockActivity, emptyMap())
}

@Test
fun `𝕄 stop RUM View 𝕎 onActivityPostStopped() { redisplay }`() {
// Given
testedStrategy.register(rumMonitor.mockSdkCore, mockAppContext)
whenever(mockPredicate.accept(mockActivity)) doReturn true
testedStrategy.register(rumMonitor.mockSdkCore, mockActivity)

// When
testedStrategy.onActivityPostStopped(mockActivity)
Thread.sleep(250)

// Then
verify(rumMonitor.mockInstance).stopView(mockActivity, emptyMap())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import androidx.fragment.app.DialogFragment
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentActivity
import androidx.fragment.app.FragmentManager
import com.datadog.android.api.InternalLogger
import com.datadog.android.api.feature.FeatureSdkCore
import com.datadog.android.rum.RumMonitor
import com.datadog.android.rum.internal.RumFeature
Expand All @@ -33,6 +34,7 @@ import org.mockito.junit.jupiter.MockitoExtension
import org.mockito.junit.jupiter.MockitoSettings
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.verify
import org.mockito.kotlin.verifyNoInteractions
import org.mockito.kotlin.whenever
Expand Down Expand Up @@ -77,6 +79,9 @@ internal class AndroidXFragmentLifecycleCallbacksTest {
@Mock
lateinit var mockSdkCore: FeatureSdkCore

@Mock
lateinit var mockInternalLogger: InternalLogger

@Mock
lateinit var mockAdvancedRumMonitor: AdvancedRumMonitor

Expand All @@ -87,6 +92,8 @@ internal class AndroidXFragmentLifecycleCallbacksTest {

@BeforeEach
fun `set up`(forge: Forge) {
whenever(mockSdkCore.internalLogger) doReturn mockInternalLogger

val mockRumFeature = mock<RumFeature>()
whenever(mockRumFeature.actionTrackingStrategy) doReturn mockUserActionTrackingStrategy
whenever(mockUserActionTrackingStrategy.getGesturesTracker()) doReturn mockGesturesTracker
Expand Down Expand Up @@ -190,12 +197,27 @@ internal class AndroidXFragmentLifecycleCallbacksTest {
}

@Test
fun `𝕄 stop RUM View 𝕎 onActivityPaused()`() {
fun `𝕄 not stop RUM View 𝕎 onFragmentPaused()`() {
// Given
whenever(mockPredicate.accept(mockFragment)) doReturn true

// When
testedLifecycleCallbacks.onFragmentPaused(mockFragmentManager, mockFragment)
Thread.sleep(250)

// Then
verify(mockRumMonitor, never()).stopView(mockFragment, emptyMap())
}

@Test
fun `𝕄 stop RUM View 𝕎 onFragmentStopped()`() {
// Given
testedLifecycleCallbacks.register(mockFragmentActivity, mockSdkCore)
whenever(mockPredicate.accept(mockFragment)) doReturn true

// When
testedLifecycleCallbacks.onFragmentStopped(mockFragmentManager, mockFragment)
Thread.sleep(250)

// Then
verify(mockRumMonitor).stopView(mockFragment, emptyMap())
Expand Down
Loading

0 comments on commit 07777fb

Please sign in to comment.