Skip to content

Commit 530128c

Browse files
satweensatween
authored
Merge pull request #2558 from DataDog/tvaleev/RUM-9154/fix-NPE-in-metrics
[RUM-9154]: reverting back os version constraints to S for registerMetricsListener and unregisterMetricsListener
2 parents 52d70bb + bfe7e1f commit 530128c

File tree

3 files changed

+103
-5
lines changed

3 files changed

+103
-5
lines changed

‎features/dd-sdk-android-rum/api/dd-sdk-android-rum.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,8 @@ public abstract interface class com/datadog/android/rum/RumSessionListener {
234234
public final class com/datadog/android/rum/_RumInternalProxy {
235235
public static final field Companion Lcom/datadog/android/rum/_RumInternalProxy$Companion;
236236
public final fun addLongTask (JLjava/lang/String;)V
237-
public final fun setInternalViewAttribute (Ljava/lang/String;Ljava/lang/Object;)V
238237
public final fun enableJankStatsTracking (Landroid/app/Activity;)V
238+
public final fun setInternalViewAttribute (Ljava/lang/String;Ljava/lang/Object;)V
239239
public final fun setSyntheticsAttribute (Ljava/lang/String;Ljava/lang/String;)V
240240
public final fun updatePerformanceMetric (Lcom/datadog/android/rum/RumPerformanceMetric;D)V
241241
}

‎features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListener.kt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ internal class JankStatsActivityLifecycleListener(
143143
if (activeActivities[activity.window].isNullOrEmpty()) {
144144
activeWindowsListener.remove(activity.window)
145145
activeActivities.remove(activity.window)
146-
if (buildSdkVersionProvider.version >= Build.VERSION_CODES.N) {
146+
if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S) {
147147
unregisterMetricListener(activity.window)
148148
}
149149
}
@@ -204,7 +204,7 @@ internal class JankStatsActivityLifecycleListener(
204204
@SuppressLint("NewApi")
205205
@MainThread
206206
private fun trackWindowMetrics(isKnownWindow: Boolean, window: Window, activity: Activity) {
207-
if (buildSdkVersionProvider.version >= Build.VERSION_CODES.N && !isKnownWindow) {
207+
if (buildSdkVersionProvider.version >= Build.VERSION_CODES.S && !isKnownWindow) {
208208
registerMetricListener(window)
209209
} else if (display == null && buildSdkVersionProvider.version == Build.VERSION_CODES.R) {
210210
// Fallback - Android 30 allows apps to not run at a fixed 60hz, but didn't yet have
@@ -214,7 +214,9 @@ internal class JankStatsActivityLifecycleListener(
214214
}
215215
}
216216

217-
@RequiresApi(Build.VERSION_CODES.N)
217+
// There is a bug in AndroidFramework that could throw NPE, so we subscribing to the FrameMetrics only since S
218+
// https://github.com/DataDog/dd-sdk-android/issues/2556
219+
@RequiresApi(Build.VERSION_CODES.S)
218220
private fun registerMetricListener(window: Window) {
219221
if (frameMetricsListener == null) {
220222
frameMetricsListener = DDFrameMetricsListener()
@@ -261,7 +263,9 @@ internal class JankStatsActivityLifecycleListener(
261263
}
262264
}
263265

264-
@RequiresApi(Build.VERSION_CODES.N)
266+
// There is a bug in AndroidFramework that could throw NPE, so we subscribing to the FrameMetrics only since S
267+
// https://github.com/DataDog/dd-sdk-android/issues/2556
268+
@RequiresApi(Build.VERSION_CODES.S)
265269
private fun unregisterMetricListener(window: Window) {
266270
try {
267271
window.removeOnFrameMetricsAvailableListener(frameMetricsListener)

‎features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/vitals/JankStatsActivityLifecycleListenerTest.kt

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
package com.datadog.android.rum.internal.vitals
88

99
import android.app.Activity
10+
import android.content.Context
11+
import android.hardware.display.DisplayManager
1012
import android.os.Build
1113
import android.os.Bundle
1214
import android.view.Display
@@ -25,6 +27,7 @@ import com.datadog.tools.unit.annotations.TestConfigurationsProvider
2527
import com.datadog.tools.unit.extensions.TestConfigurationExtension
2628
import com.datadog.tools.unit.extensions.config.TestConfiguration
2729
import fr.xgouchet.elmyr.Forge
30+
import fr.xgouchet.elmyr.annotation.IntForgery
2831
import fr.xgouchet.elmyr.junit5.ForgeConfiguration
2932
import fr.xgouchet.elmyr.junit5.ForgeExtension
3033
import org.assertj.core.api.Assertions.assertThat
@@ -43,6 +46,7 @@ import org.mockito.kotlin.doThrow
4346
import org.mockito.kotlin.inOrder
4447
import org.mockito.kotlin.mock
4548
import org.mockito.kotlin.reset
49+
import org.mockito.kotlin.times
4650
import org.mockito.kotlin.verify
4751
import org.mockito.kotlin.verifyNoInteractions
4852
import org.mockito.kotlin.whenever
@@ -77,6 +81,9 @@ internal class JankStatsActivityLifecycleListenerTest {
7781
@Mock
7882
lateinit var mockDecorView: View
7983

84+
@Mock
85+
lateinit var mockDisplayManager: DisplayManager
86+
8087
@Mock
8188
lateinit var mockJankStatsProvider: JankStatsProvider
8289

@@ -93,6 +100,8 @@ internal class JankStatsActivityLifecycleListenerTest {
93100
whenever(mockWindow.peekDecorView()) doReturn mockDecorView
94101
whenever(mockActivity.window) doReturn mockWindow
95102
whenever(mockActivity.display) doReturn mockDisplay
103+
whenever(mockDisplayManager.getDisplay(Display.DEFAULT_DISPLAY)) doReturn mockDisplay
104+
whenever(mockActivity.getSystemService(Context.DISPLAY_SERVICE)) doReturn mockDisplayManager
96105
whenever(mockJankStatsProvider.createJankStatsAndTrack(any(), any(), any())) doReturn mockJankStats
97106
whenever(mockJankStats.isTrackingEnabled) doReturn true
98107

@@ -383,6 +392,91 @@ internal class JankStatsActivityLifecycleListenerTest {
383392
}
384393
}
385394

395+
@Test
396+
fun `M not call addOnFrameMetricsAvailableListener() W onActivityStarted { ANDROID_SDK less than S } `(
397+
@IntForgery(min = Build.VERSION_CODES.LOLLIPOP, max = Build.VERSION_CODES.S) apiVersion: Int
398+
) {
399+
// There is a bug in AndroidFramework that could throw NPE, so we subscribing to the FrameMetrics only since S
400+
// https://github.com/DataDog/dd-sdk-android/issues/2556
401+
402+
// Given
403+
whenever(mockDecorView.isHardwareAccelerated) doReturn true
404+
whenever(mockBuildSdkVersionProvider.version) doReturn apiVersion
405+
406+
// When
407+
testedJankListener.onActivityStarted(mockActivity)
408+
409+
// Then
410+
verify(mockDecorView, times(0)).post(any())
411+
verify(mockWindow, times(0)).addOnFrameMetricsAvailableListener(any(), any())
412+
}
413+
414+
@Test
415+
fun `M not call addOnFrameMetricsAvailableListener() W onActivityStarted { ANDROID_SDK grater or equal S } `(
416+
@IntForgery(min = Build.VERSION_CODES.S) apiVersion: Int
417+
) {
418+
// There is a bug in AndroidFramework that could throw NPE, so we subscribing to the FrameMetrics only since S
419+
// https://github.com/DataDog/dd-sdk-android/issues/2556
420+
421+
// Given
422+
whenever(mockDecorView.isHardwareAccelerated) doReturn true
423+
whenever(mockBuildSdkVersionProvider.version) doReturn apiVersion
424+
// When
425+
testedJankListener.onActivityStarted(mockActivity)
426+
argumentCaptor<Runnable> {
427+
verify(mockDecorView).post(capture())
428+
firstValue.run()
429+
}
430+
431+
// Then
432+
verify(mockWindow).addOnFrameMetricsAvailableListener(any(), any())
433+
}
434+
435+
@Test
436+
fun `M not call addOnFrameMetricsAvailableListener() W onActivityDestroyed { ANDROID_SDK less than S } `(
437+
@IntForgery(min = Build.VERSION_CODES.LOLLIPOP, max = Build.VERSION_CODES.S) apiVersion: Int
438+
) {
439+
// There is a bug in AndroidFramework that could throw NPE, so we subscribing to the FrameMetrics only since S
440+
// https://github.com/DataDog/dd-sdk-android/issues/2556
441+
442+
// Given
443+
whenever(mockDecorView.isHardwareAccelerated) doReturn true
444+
whenever(mockBuildSdkVersionProvider.version) doReturn apiVersion
445+
testedJankListener.onActivityStarted(mockActivity)
446+
testedJankListener.activeActivities.clear()
447+
448+
// When
449+
testedJankListener.onActivityDestroyed(mockActivity)
450+
451+
// Then
452+
verify(mockDecorView, times(0)).post(any())
453+
verify(mockWindow, times(0)).removeOnFrameMetricsAvailableListener(any())
454+
}
455+
456+
@Test
457+
fun `M not call addOnFrameMetricsAvailableListener() W onActivityDestroyed { ANDROID_SDK grater or equal S } `(
458+
@IntForgery(min = Build.VERSION_CODES.S) apiVersion: Int
459+
) {
460+
// There is a bug in AndroidFramework that could throw NPE, so we subscribing to the FrameMetrics only since S
461+
// https://github.com/DataDog/dd-sdk-android/issues/2556
462+
463+
// Given
464+
whenever(mockDecorView.isHardwareAccelerated) doReturn true
465+
whenever(mockBuildSdkVersionProvider.version) doReturn apiVersion
466+
testedJankListener.onActivityStarted(mockActivity)
467+
argumentCaptor<Runnable> {
468+
verify(mockDecorView).post(capture())
469+
firstValue.run()
470+
}
471+
testedJankListener.activeActivities.clear()
472+
473+
// When
474+
testedJankListener.onActivityDestroyed(mockActivity)
475+
476+
// Then
477+
verify(mockWindow).removeOnFrameMetricsAvailableListener(any())
478+
}
479+
386480
@Test
387481
fun `M forward FrameData W onFrame`(forge: Forge) {
388482
val frameData = forge.getForgery<FrameData>()

0 commit comments

Comments
 (0)