From bbd794fd58ee5118087b174f2041cc0f913d713d Mon Sep 17 00:00:00 2001 From: Marius Constantin Date: Tue, 29 Nov 2022 13:50:13 +0100 Subject: [PATCH] RUMM-2769 Fix NPE in ScreenRecorder when wrapping a null window.callback --- .../recorder/NoOpWindowCallback.kt | 135 ++++++++++++++++++ .../sessionreplay/recorder/ScreenRecorder.kt | 38 ++++- .../recorder/ScreenRecorderTest.kt | 97 ++++++++++++- 3 files changed, 261 insertions(+), 9 deletions(-) create mode 100644 library/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/recorder/NoOpWindowCallback.kt diff --git a/library/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/recorder/NoOpWindowCallback.kt b/library/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/recorder/NoOpWindowCallback.kt new file mode 100644 index 0000000000..85754ddfab --- /dev/null +++ b/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 +} diff --git a/library/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/recorder/ScreenRecorder.kt b/library/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/recorder/ScreenRecorder.kt index 2915d4fa83..e77742ad78 100644 --- a/library/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/recorder/ScreenRecorder.kt +++ b/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 + } } } } diff --git a/library/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/recorder/ScreenRecorderTest.kt b/library/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/recorder/ScreenRecorderTest.kt index e653b329c4..056ba744da 100644 --- a/library/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/recorder/ScreenRecorderTest.kt +++ b/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() + 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() + 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() + 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 }