fix: Normalize js_refresh_rate against actual screen fps

packages/core/android/src/main/kotlin/com/datadog/reactnative/DdSdkImplementation.kt

@@ -331,7 +331,14 @@ class DdSdkImplementation(
         ddSdkConfiguration: DdSdkConfiguration
     ): FrameRateProvider? {
         val frameTimeCallback = buildFrameTimeCallback(ddSdkConfiguration) ?: return null
-        val frameRateProvider = FrameRateProvider(frameTimeCallback, jsThreadExecutor)
+        // Before API 30 every Display.refreshRate read is an IPC, so use the max refresh rate there
+        val display = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
+            (appContext.getSystemService(Context.DISPLAY_SERVICE) as? DisplayManager)
+                ?.getDisplay(Display.DEFAULT_DISPLAY)
+        } else {
+            null
+        }
+        val frameRateProvider = FrameRateProvider(frameTimeCallback, jsThreadExecutor, display)
         frameRateProvider.start()
 
         return frameRateProvider
@@ -340,7 +347,7 @@ class DdSdkImplementation(
     @Suppress("CyclomaticComplexMethod")
     private fun buildFrameTimeCallback(
         ddSdkConfiguration: DdSdkConfiguration
-    ): ((Double) -> Unit)? {
+    ): ((Double, Double?) -> Unit)? {
         val jsRefreshRateMonitoringEnabled =
             ddSdkConfiguration.rumConfiguration != null &&
             buildVitalUpdateFrequency(ddSdkConfiguration.rumConfiguration.vitalsUpdateFrequency) !=
@@ -351,20 +358,21 @@ class DdSdkImplementation(
             return null
         }
 
-        return {
-            if (jsRefreshRateMonitoringEnabled && it > 0.0) {
-                val normalizedFrameTimeSeconds = normalizeFrameTime(it, appContext)
+        return { frameTime, displayFps ->
+            if (jsRefreshRateMonitoringEnabled && frameTime > 0.0) {
+                val normalizedFrameTimeSeconds =
+                    normalizeFrameTime(frameTime, appContext, deviceDisplayFps = displayFps)
                 datadog.getRumMonitor()
                     ._getInternal()
                     ?.updatePerformanceMetric(RumPerformanceMetric.JS_FRAME_TIME, normalizedFrameTimeSeconds)
             }
             if (jsLongTasksMonitoringEnabled &&
-                it >
+                frameTime >
                 TimeUnit.MILLISECONDS.toNanos(
                     ddSdkConfiguration.rumConfiguration?.longTaskThresholdMs?.toLong() ?: 0L
                 )
             ) {
-                datadog.getRumMonitor()._getInternal()?.addLongTask(it.toLong(), "javascript")
+                datadog.getRumMonitor()._getInternal()?.addLongTask(frameTime.toLong(), "javascript")
             }
         }
     }
@@ -374,7 +382,7 @@ class DdSdkImplementation(
      * @param frameTimeSeconds: the frame time to normalize. In seconds.
      * @param context: The current app context
      * @param fpsBudget: The maximum fps under which the frame Time will be normalized [0-fpsBudget]. Defaults to 60Hz.
-     * @param deviceDisplayFps: The maximum fps supported by the device. If not provided it will be set from the value obtained from the app context.
+     * @param deviceDisplayFps: The refresh rate the display is currently running at. If not provided it will be set from the maximum value obtained from the app context.
      */
     @Suppress("CyclomaticComplexMethod")
     fun normalizeFrameTime(

packages/core/android/src/main/kotlin/com/datadog/reactnative/FrameRateProvider.kt

@@ -7,14 +7,17 @@
 package com.datadog.reactnative
 
 import android.view.Choreographer
+import android.view.Display
 
 internal class FrameRateProvider(
-    reactFrameRateCallback: ((Double) -> Unit),
-    jsThreadExecutor: JsThreadExecutor
+    reactFrameRateCallback: ((Double, Double?) -> Unit),
+    jsThreadExecutor: JsThreadExecutor,
+    display: Display?
 ) {
     private val frameCallback: FpsFrameCallback = FpsFrameCallback(
         reactFrameRateCallback,
-        jsThreadExecutor
+        jsThreadExecutor,
+        display
     )
 
     fun start() {
@@ -28,16 +31,18 @@ internal class FrameRateProvider(
 }
 
 internal class FpsFrameCallback(
-    private val reactFrameRateCallback: ((Double) -> Unit),
-    private val jsThreadExecutor: JsThreadExecutor
+    private val reactFrameRateCallback: ((Double, Double?) -> Unit),
+    private val jsThreadExecutor: JsThreadExecutor,
+    private val display: Display?
 ) : Choreographer.FrameCallback {
 
     private var choreographer: Choreographer? = null
     private var lastFrameTime = -1L
 
     override fun doFrame(time: Long) {
         if (lastFrameTime != -1L) {
-            reactFrameRateCallback((time - lastFrameTime).toDouble())
+            // Refresh rate the display is currently running at
+            reactFrameRateCallback((time - lastFrameTime).toDouble(), display?.refreshRate?.toDouble())
         }
         lastFrameTime = time
         choreographer?.postFrameCallback(this)

packages/core/android/src/test/kotlin/com/datadog/reactnative/DdSdkTest.kt

@@ -9,6 +9,7 @@ package com.datadog.reactnative
 import android.content.Context
 import android.content.pm.PackageInfo
 import android.hardware.display.DisplayManager
+import android.os.Build
 import android.os.Looper
 import android.util.Log
 import android.view.Choreographer
@@ -3670,6 +3671,136 @@ internal class DdSdkTest {
         verify(mockDisplay, times(1)).supportedModes
     }
 
+    @Test
+    fun `𝕄 normalize JS frame time with the display's current refresh rate 𝕎 doFrame()`(
+        @Forgery configuration: DdSdkConfiguration
+    ) {
+        // Given
+        val rumConfiguration = configuration.rumConfiguration?.copy(
+            vitalsUpdateFrequency = "AVERAGE",
+            longTaskThresholdMs = 0.0
+        )
+        val bridgeConfiguration = configuration.copy(rumConfiguration = rumConfiguration)
+        val mockDisplayManager = mock<DisplayManager>()
+        val mockDisplay = mock<Display>()
+        whenever(mockContext.getSystemService(Context.DISPLAY_SERVICE)) doReturn mockDisplayManager
+        whenever(mockDisplayManager.getDisplay(Display.DEFAULT_DISPLAY)) doReturn mockDisplay
+        val originalSdkInt = Build.VERSION.SDK_INT
+        Build.VERSION::class.java.setStaticValue("SDK_INT", Build.VERSION_CODES.R)
+
+        val rumMock = org.mockito.Mockito.mockStatic(Rum::class.java)
+        val traceMock = org.mockito.Mockito.mockStatic(Trace::class.java)
+        val logsMock = org.mockito.Mockito.mockStatic(Logs::class.java)
+
+        try {
+            rumMock.`when`<Unit> { Rum.enable(any(), any()) }.then { }
+            logsMock.`when`<Unit> { Logs.enable(any(), any()) }.then { }
+            traceMock.`when`<Unit> { Trace.enable(any(), any()) }.then { }
+
+            // When
+            testedBridgeSdk.initialize(bridgeConfiguration.toReadableJavaOnlyMap(), mockPromise)
+
+            // Then
+            argumentCaptor<Choreographer.FrameCallback> {
+                verify(mockChoreographer).postFrameCallback(capture())
+                var timestampNs = 0L
+                firstValue.doFrame(timestampNs)
+
+                // 120 fps, display running at 120Hz -> Normalized to 60fps
+                whenever(mockDisplay.refreshRate) doReturn 120f
+                timestampNs += 8_333_333L
+                firstValue.doFrame(timestampNs)
+                verify(mockRumInternalProxy).updatePerformanceMetric(
+                    RumPerformanceMetric.JS_FRAME_TIME,
+                    16_666_666.0
+                )
+
+                // 60 fps, display running at 120Hz -> Normalized to 30fps
+                timestampNs += 16_666_667L
+                firstValue.doFrame(timestampNs)
+                verify(mockRumInternalProxy).updatePerformanceMetric(
+                    RumPerformanceMetric.JS_FRAME_TIME,
+                    33_333_334.0
+                )
+
+                // 60 fps, display running at 60Hz -> Normalized to 60fps
+                whenever(mockDisplay.refreshRate) doReturn 60f
+                timestampNs += 16_666_667L
+                firstValue.doFrame(timestampNs)
+                verify(mockRumInternalProxy).updatePerformanceMetric(
+                    RumPerformanceMetric.JS_FRAME_TIME,
+                    16_666_667.0
+                )
+
+                // 30 fps, display running at 60Hz -> Normalized to 30fps
+                timestampNs += 33_333_333L
+                firstValue.doFrame(timestampNs)
+                verify(mockRumInternalProxy).updatePerformanceMetric(
+                    RumPerformanceMetric.JS_FRAME_TIME,
+                    33_333_333.0
+                )
+            }
+        } finally {
+            Build.VERSION::class.java.setStaticValue("SDK_INT", originalSdkInt)
+            rumMock.close()
+            logsMock.close()
+            traceMock.close()
+        }
+    }
+
+    @Test
+    fun `𝕄 normalize JS frame time with the max refresh rate 𝕎 doFrame() {API below 30}`(
+        @Forgery configuration: DdSdkConfiguration
+    ) {
+        // Given
+        val rumConfiguration = configuration.rumConfiguration?.copy(
+            vitalsUpdateFrequency = "AVERAGE",
+            longTaskThresholdMs = 0.0
+        )
+        val bridgeConfiguration = configuration.copy(rumConfiguration = rumConfiguration)
+        val mockDisplayManager = mock<DisplayManager>()
+        val mockDisplay = mock<Display>()
+        val mockDisplayMode = mock<Display.Mode>()
+        whenever(mockContext.getSystemService(Context.DISPLAY_SERVICE)) doReturn mockDisplayManager
+        whenever(mockDisplayManager.getDisplay(Display.DEFAULT_DISPLAY)) doReturn mockDisplay
+        whenever(mockDisplayMode.refreshRate) doReturn 120f
+        whenever(mockDisplay.supportedModes) doReturn arrayOf(mockDisplayMode)
+        val originalSdkInt = Build.VERSION.SDK_INT
+        Build.VERSION::class.java.setStaticValue("SDK_INT", Build.VERSION_CODES.Q)
+
+        val rumMock = org.mockito.Mockito.mockStatic(Rum::class.java)
+        val traceMock = org.mockito.Mockito.mockStatic(Trace::class.java)
+        val logsMock = org.mockito.Mockito.mockStatic(Logs::class.java)
+
+        try {
+            rumMock.`when`<Unit> { Rum.enable(any(), any()) }.then { }
+            logsMock.`when`<Unit> { Logs.enable(any(), any()) }.then { }
+            traceMock.`when`<Unit> { Trace.enable(any(), any()) }.then { }
+
+            // When
+            testedBridgeSdk.initialize(bridgeConfiguration.toReadableJavaOnlyMap(), mockPromise)
+
+            // Then
+            argumentCaptor<Choreographer.FrameCallback> {
+                verify(mockChoreographer).postFrameCallback(capture())
+                firstValue.doFrame(0L)
+
+                // 60 fps, 120Hz capable display -> Normalized with the max refresh rate to 30fps
+                firstValue.doFrame(16_666_667L)
+                verify(mockRumInternalProxy).updatePerformanceMetric(
+                    RumPerformanceMetric.JS_FRAME_TIME,
+                    33_333_334.0
+                )
+                verify(mockDisplay, never()).refreshRate
+            }
+        } finally {
+            Build.VERSION::class.java.setStaticValue("SDK_INT", originalSdkInt)
+            rumMock.close()
+            logsMock.close()
+            traceMock.close()
+        }
+    }
+
     // endregion
 
     // region Internal

packages/core/ios/Sources/DdSdkImplementation.swift

@@ -309,7 +309,7 @@ public class DdSdkImplementation: NSObject {
         }
     }
 
-    func buildFrameTimeCallback(sdkConfiguration: DdSdkConfiguration) -> ((Double) -> Void)? {
+    func buildFrameTimeCallback(sdkConfiguration: DdSdkConfiguration) -> ((Double, Double) -> Void)? {
         let jsRefreshRateMonitoringEnabled =
             sdkConfiguration.rumConfiguration != nil
             && sdkConfiguration.rumConfiguration?.vitalsUpdateFrequency != nil
@@ -321,7 +321,7 @@ public class DdSdkImplementation: NSObject {
             return nil
         }
 
-        func frameTimeCallback(frameTime: Double) {
+        func frameTimeCallback(frameTime: Double, displayFps: Double) {
             // These checks happen before dispatching because they are quick and less overhead than the dispatch itself.
             let shouldRecordFrameTime = jsRefreshRateMonitoringEnabled && frameTime > 0
             let shouldRecordLongTask =
@@ -338,7 +338,8 @@ public class DdSdkImplementation: NSObject {
             sharedQueue.async {
                 if shouldRecordFrameTime {
                     let normalizedFrameTimeSeconds =
-                        DdSdkImplementation.normalizeFrameTimeForDeviceRefreshRate(frameTime)
+                        DdSdkImplementation.normalizeFrameTimeForDeviceRefreshRate(
+                            frameTime, deviceDisplayFps: displayFps)
                     rumMonitorInternal.updatePerformanceMetric(
                         at: now, metric: .jsFrameTimeSeconds, value: normalizedFrameTimeSeconds,
                         attributes: [:])

packages/core/ios/Sources/JSRefreshRateListener.swift

@@ -6,7 +6,7 @@
 
 import Foundation
 
-typealias frame_time_callback = (Double) -> Void
+typealias frame_time_callback = (_ frameTime: Double, _ displayFps: Double) -> Void
 
 internal protocol RefreshRateListener {
     func start()
@@ -54,6 +54,7 @@ private final class JSRefreshRateListener: RefreshRateListener {
     private var jsQueue: DispatchQueueType
     private var frameTimeCallback: frame_time_callback
     private var lastFrameTimestamp: TimeInterval = -1
+    private var displayFps: Double = -1
     private var jsDisplayLink: CADisplayLink?
 
     init(jsQueue: DispatchQueueType, frameTimeCallback: @escaping frame_time_callback) {
@@ -80,8 +81,11 @@ private final class JSRefreshRateListener: RefreshRateListener {
         let frameTimestamp = displayLink.timestamp
         if lastFrameTimestamp != -1 {
             let frameDuration = frameTimestamp - lastFrameTimestamp
-            frameTimeCallback(frameDuration)
+            frameTimeCallback(frameDuration, displayFps)
         }
         lastFrameTimestamp = frameTimestamp
+
+        // Refresh rate the display is currently running at
+        displayFps = 1.0 / (displayLink.targetTimestamp - frameTimestamp)
     }
 }

packages/core/ios/Tests/DdSdkTests.swift

@@ -1332,6 +1332,34 @@ class DdSdkTests: XCTestCase {
         XCTAssertEqual(
             rumMonitorMock.lastReceivedPerformanceMetrics[.jsFrameTimeSeconds]!, 0.016,
             accuracy: 0.001)
+
+        // 120 fps, display running at 120Hz -> Normalized to 60fps
+        mockRefreshRateMonitor.executeFrameCallback(frameTime: 0.0083, displayFps: 120.0)
+        sharedQueue.sync {}
+        XCTAssertEqual(
+            rumMonitorMock.lastReceivedPerformanceMetrics[.jsFrameTimeSeconds]!, 0.016,
+            accuracy: 0.001)
+
+        // 60 fps, display running at 120Hz -> Normalized to 30fps
+        mockRefreshRateMonitor.executeFrameCallback(frameTime: 0.016, displayFps: 120.0)
+        sharedQueue.sync {}
+        XCTAssertEqual(
+            rumMonitorMock.lastReceivedPerformanceMetrics[.jsFrameTimeSeconds]!, 0.032,
+            accuracy: 0.001)
+
+        // 60 fps, display running at 60Hz -> Normalized to 60fps
+        mockRefreshRateMonitor.executeFrameCallback(frameTime: 0.016, displayFps: 60.0)
+        sharedQueue.sync {}
+        XCTAssertEqual(
+            rumMonitorMock.lastReceivedPerformanceMetrics[.jsFrameTimeSeconds]!, 0.016,
+            accuracy: 0.001)
+
+        // 30 fps, display running at 60Hz -> Normalized to 30fps
+        mockRefreshRateMonitor.executeFrameCallback(frameTime: 0.03, displayFps: 60.0)
+        sharedQueue.sync {}
+        XCTAssertEqual(
+            rumMonitorMock.lastReceivedPerformanceMetrics[.jsFrameTimeSeconds]!, 0.03,
+            accuracy: 0.001)
     }
 
     func testFrameTimeNormalizationUtilityFunction() {
@@ -1757,8 +1785,8 @@ private final class MockJSRefreshRateMonitor: RefreshRateMonitor {
         isStarted = true
     }
 
-    func executeFrameCallback(frameTime: TimeInterval) {
-        frameTimeCallback?(frameTime)
+    func executeFrameCallback(frameTime: TimeInterval, displayFps: Double = 60.0) {
+        frameTimeCallback?(frameTime, displayFps)
     }
 }