Merge pull request #1164 from DataDog/mconstantin/rumm-2769/fix-npe-i…

…n-sr-screen-recorder

RUMM-2769 Fix NPE in ScreenRecorder when wrapping a null window.callback

library/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/recorder/NoOpWindowCallback.kt

@@ -0,0 +1,135 @@
+/*
+ * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
+ * This product includes software developed at Datadog (https://www.datadoghq.com/).
+ * Copyright 2016-Present Datadog, Inc.
+ */
+
+package com.datadog.android.sessionreplay.recorder
+
+import android.view.ActionMode
+import android.view.KeyEvent
+import android.view.Menu
+import android.view.MenuItem
+import android.view.MotionEvent
+import android.view.SearchEvent
+import android.view.View
+import android.view.Window
+import android.view.WindowManager
+import android.view.accessibility.AccessibilityEvent
+
+internal class NoOpWindowCallback : Window.Callback {
+
+    // region Window.Callback
+    override fun onActionModeFinished(mode: ActionMode?) {
+        // No Op
+    }
+
+    override fun onCreatePanelView(featureId: Int): View? {
+        // No Op
+        return null
+    }
+
+    override fun dispatchTouchEvent(event: MotionEvent?): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun onCreatePanelMenu(featureId: Int, menu: Menu): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun onWindowStartingActionMode(callback: ActionMode.Callback?): ActionMode? {
+        // No Op
+        return null
+    }
+
+    override fun onWindowStartingActionMode(
+        callback: ActionMode.Callback?,
+        type: Int
+    ): ActionMode? {
+        // No Op
+        return null
+    }
+
+    override fun onAttachedToWindow() {
+        // No Op
+    }
+
+    override fun dispatchGenericMotionEvent(event: MotionEvent?): Boolean {
+        // No Op
+        return false
+    }
+
+    @Suppress("FunctionMaxLength")
+    override fun dispatchPopulateAccessibilityEvent(event: AccessibilityEvent?): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun dispatchTrackballEvent(event: MotionEvent?): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun dispatchKeyShortcutEvent(event: KeyEvent?): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun dispatchKeyEvent(event: KeyEvent?): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun onMenuOpened(featureId: Int, menu: Menu): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun onPanelClosed(featureId: Int, menu: Menu) {
+        // No Op
+    }
+
+    override fun onMenuItemSelected(featureId: Int, item: MenuItem): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun onDetachedFromWindow() {
+        // No Op
+    }
+
+    override fun onPreparePanel(featureId: Int, view: View?, menu: Menu): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun onWindowAttributesChanged(attrs: WindowManager.LayoutParams?) {
+        // No Op
+    }
+
+    override fun onWindowFocusChanged(hasFocus: Boolean) {
+        // No Op
+    }
+
+    override fun onContentChanged() {
+        // No Op
+    }
+
+    override fun onSearchRequested(): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun onSearchRequested(searchEvent: SearchEvent?): Boolean {
+        // No Op
+        return false
+    }
+
+    override fun onActionModeStarted(mode: ActionMode?) {
+        // No Op
+    }
+
+    // endregion
+}

library/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/recorder/ScreenRecorder.kt

@@ -8,6 +8,7 @@ package com.datadog.android.sessionreplay.recorder
 
 import android.app.Activity
 import android.view.ViewTreeObserver
+import android.view.Window
 import com.datadog.android.sessionreplay.processor.Processor
 import com.datadog.android.sessionreplay.utils.TimeProvider
 
@@ -31,11 +32,10 @@ internal class ScreenRecorder(
                 drawListeners[activity.hashCode()] = this
                 activity.window.decorView.viewTreeObserver?.addOnDrawListener(this)
             }
-            activity.window.callback = RecorderWindowCallback(
-                processor,
-                activity.resources.displayMetrics.density,
-                activity.window.callback,
-                timeProvider
+
+            wrapWindowCallback(
+                activity.window,
+                activity.resources.displayMetrics.density
             )
         }
     }
@@ -45,8 +45,32 @@ internal class ScreenRecorder(
             drawListeners.remove(windowHashCode)?.let {
                 activity.window.decorView.viewTreeObserver.removeOnDrawListener(it)
             }
-            activity.window.callback =
-                (activity.window.callback as? RecorderWindowCallback)?.wrappedCallback
+        }
+
+        if (activity.window != null) {
+            unwrapWindowCallback(activity.window)
+        }
+    }
+
+    private fun wrapWindowCallback(window: Window, screenDensity: Float) {
+        val toWrap = window.callback ?: NoOpWindowCallback()
+        window.callback = RecorderWindowCallback(
+            processor,
+            screenDensity,
+            toWrap,
+            timeProvider
+        )
+    }
+
+    private fun unwrapWindowCallback(window: Window) {
+        val callback = window.callback
+        if (callback is RecorderWindowCallback) {
+            val wrappedCallback = callback.wrappedCallback
+            if (wrappedCallback !is NoOpWindowCallback) {
+                window.callback = wrappedCallback
+            } else {
+                window.callback = null
+            }
         }
     }
 }

library/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/recorder/ScreenRecorderTest.kt

@@ -14,8 +14,10 @@ import android.view.ViewTreeObserver
 import android.view.Window
 import com.datadog.android.sessionreplay.processor.Processor
 import com.datadog.android.sessionreplay.utils.TimeProvider
+import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.argumentCaptor
 import com.nhaarman.mockitokotlin2.mock
+import com.nhaarman.mockitokotlin2.never
 import com.nhaarman.mockitokotlin2.times
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
@@ -93,6 +95,29 @@ internal class ScreenRecorderTest {
             .isEqualTo(mockDefaultCallback)
     }
 
+    @Test
+    fun `M register the RecorderWindowCallback W startRecording{default callback is null}`(
+        forge: Forge
+    ) {
+        // Given
+        val mockWindow: Window = mock()
+        val mockActivity = mockActivity(
+            forge,
+            window = mockWindow,
+            defaultWindowCallback = null
+        )
+
+        // When
+        testedRecorder.startRecording(mockActivity)
+
+        // Then
+        val captor = argumentCaptor<Window.Callback>()
+        verify(mockWindow).callback = captor.capture()
+        assertThat(captor.firstValue).isInstanceOf(RecorderWindowCallback::class.java)
+        assertThat((captor.firstValue as RecorderWindowCallback).wrappedCallback)
+            .isInstanceOf(NoOpWindowCallback::class.java)
+    }
+
     @Test
     fun `M do nothing W startRecording() { activity window is null }`() {
         // Given
@@ -122,7 +147,9 @@ internal class ScreenRecorderTest {
     }
 
     @Test
-    fun `M remove the RecorderWindowCallback W stopRecording()`(forge: Forge) {
+    fun `M remove the RecorderWindowCallback W stopRecording(){default callback is not null}`(
+        forge: Forge
+    ) {
         // Given
         val mockWindow: Window = mock()
         val mockDefaultCallback: Window.Callback = mock()
@@ -147,6 +174,72 @@ internal class ScreenRecorderTest {
         assertThat(stopRecordingCaptureTarget.secondValue).isSameAs(mockDefaultCallback)
     }
 
+    @Test
+    fun `M remove the RecorderWindowCallback W stopRecording(){default callback was null}`(
+        forge: Forge
+    ) {
+        // Given
+        val mockWindow: Window = mock()
+        val mockActivity = mockActivity(
+            forge,
+            window = mockWindow,
+            defaultWindowCallback = null
+        )
+        testedRecorder.startRecording(mockActivity)
+        val startRecordingCapture = argumentCaptor<Window.Callback>()
+        verify(mockWindow).callback = startRecordingCapture.capture()
+        assertThat(startRecordingCapture.firstValue)
+            .isInstanceOf(RecorderWindowCallback::class.java)
+        whenever(mockWindow.callback).thenReturn(startRecordingCapture.firstValue)
+
+        // When
+        testedRecorder.stopRecording(mockActivity)
+
+        // Then
+        val stopRecordingCaptureTarget = argumentCaptor<Window.Callback>()
+        verify(mockWindow, times(2)).callback = stopRecordingCaptureTarget.capture()
+        assertThat(stopRecordingCaptureTarget.secondValue).isNull()
+    }
+
+    @Test
+    fun `M do nothing W stopRecording(){window callback was not wrapped}`(
+        forge: Forge
+    ) {
+        // Given
+        val mockWindow: Window = mock()
+        val mockDefaultCallback: Window.Callback = mock()
+        val mockActivity = mockActivity(
+            forge,
+            window = mockWindow,
+            defaultWindowCallback = mockDefaultCallback
+        )
+
+        // When
+        testedRecorder.stopRecording(mockActivity)
+
+        // Then
+        verify(mockWindow, never()).callback = any()
+    }
+
+    @Test
+    fun `M do nothing W stopRecording(){window callback was not wrapped and was null}`(
+        forge: Forge
+    ) {
+        // Given
+        val mockWindow: Window = mock()
+        val mockActivity = mockActivity(
+            forge,
+            window = mockWindow,
+            defaultWindowCallback = null
+        )
+
+        // When
+        testedRecorder.stopRecording(mockActivity)
+
+        // Then
+        verify(mockWindow, never()).callback = any()
+    }
+
     @Test
     fun `M clean the listeners the RecorderOnDrawListener W stopRecording()`(forge: Forge) {
         // Given
@@ -169,7 +262,7 @@ internal class ScreenRecorderTest {
         forge: Forge,
         viewTreeObserver: ViewTreeObserver = mock(),
         window: Window = mock(),
-        defaultWindowCallback: Window.Callback = mock()
+        defaultWindowCallback: Window.Callback? = null
     ): Activity {
         val fakeDensity = forge.aFloat()
         val displayMetrics = DisplayMetrics().apply { density = fakeDensity }