Merge "Move the `isRecording` state in Issuerecording service into a global settings rather than a user-specific variable." into main

8 files changed, 154 insertions, 40 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingDataInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingDataInteractorTest.kt
index 2194c75dba5b..f3fc0ab92c84 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingDataInteractorTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingDataInteractorTest.kt
@@ -16,6 +16,7 @@
 
 package com.android.systemui.qs.tiles.impl.irecording
 
+import android.os.Handler
 import android.os.UserHandle
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.SmallTest
@@ -28,6 +29,7 @@ import com.android.systemui.qs.tiles.base.interactor.DataUpdateTrigger
 import com.android.systemui.recordissue.IssueRecordingState
 import com.android.systemui.settings.fakeUserFileManager
 import com.android.systemui.settings.userTracker
+import com.android.systemui.util.settings.fakeGlobalSettings
 import com.google.common.truth.Truth
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.flowOf
@@ -51,7 +53,14 @@ class IssueRecordingDataInteractorTest : SysuiTestCase() {
 
     @Before
     fun setup() {
-        state = IssueRecordingState(userTracker, userFileManager)
+        state =
+            IssueRecordingState(
+                userTracker,
+                userFileManager,
+                Handler.getMain(),
+                mContext.contentResolver,
+                kosmos.fakeGlobalSettings,
+            )
         underTest = IssueRecordingDataInteractor(state, kosmos.testScope.testScheduler)
     }
 
@@ -67,10 +76,12 @@ class IssueRecordingDataInteractorTest : SysuiTestCase() {
             Truth.assertThat(data?.isRecording).isFalse()
 
             state.isRecording = true
+            state.onRecordingChangeListener.onChange(true)
             runCurrent()
             Truth.assertThat(data?.isRecording).isTrue()
 
             state.isRecording = false
+            state.onRecordingChangeListener.onChange(true)
             runCurrent()
             Truth.assertThat(data?.isRecording).isFalse()
         }
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingUserActionInteractorTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingUserActionInteractorTest.kt
index 99d2da670172..9c2be899b8ca 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingUserActionInteractorTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingUserActionInteractorTest.kt
@@ -16,6 +16,7 @@
 
 package com.android.systemui.qs.tiles.impl.irecording
 
+import android.os.Handler
 import android.os.UserHandle
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.SmallTest
@@ -38,6 +39,7 @@ import com.android.systemui.settings.userFileManager
 import com.android.systemui.settings.userTracker
 import com.android.systemui.statusbar.phone.KeyguardDismissUtil
 import com.android.systemui.statusbar.policy.keyguardStateController
+import com.android.systemui.util.settings.fakeGlobalSettings
 import com.google.common.truth.Truth
 import kotlinx.coroutines.test.runTest
 import org.junit.Before
@@ -83,7 +85,13 @@ class IssueRecordingUserActionInteractorTest : SysuiTestCase() {
             underTest =
                 IssueRecordingUserActionInteractor(
                     testDispatcher,
-                    IssueRecordingState(userTracker, userFileManager),
+                    IssueRecordingState(
+                        userTracker,
+                        userFileManager,
+                        Handler.getMain(),
+                        mContext.contentResolver,
+                        kosmos.fakeGlobalSettings,
+                    ),
                     KeyguardDismissUtil(
                         keyguardStateController,
                         statusBarStateController,
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/IssueRecordingServiceSessionTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/IssueRecordingServiceSessionTest.kt
index aceaab8206a2..f2e658dc3759 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/IssueRecordingServiceSessionTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/IssueRecordingServiceSessionTest.kt
@@ -19,6 +19,7 @@ package com.android.systemui.recordissue
 import android.app.IActivityManager
 import android.app.NotificationManager
 import android.net.Uri
+import android.os.Handler
 import android.os.UserHandle
 import android.testing.TestableLooper
 import androidx.test.ext.junit.runners.AndroidJUnit4
@@ -33,6 +34,7 @@ import com.android.systemui.qs.pipeline.domain.interactor.PanelInteractor
 import com.android.systemui.settings.UserContextProvider
 import com.android.systemui.settings.userFileManager
 import com.android.systemui.settings.userTracker
+import com.android.systemui.util.settings.fakeGlobalSettings
 import com.android.traceur.TraceConfig
 import com.google.common.truth.Truth
 import org.junit.Before
@@ -55,7 +57,13 @@ class IssueRecordingServiceSessionTest : SysuiTestCase() {
     private val dialogTransitionAnimator: DialogTransitionAnimator = kosmos.dialogTransitionAnimator
     private lateinit var traceurConnection: TraceurConnection
     private val issueRecordingState =
-        IssueRecordingState(kosmos.userTracker, kosmos.userFileManager)
+        IssueRecordingState(
+            kosmos.userTracker,
+            kosmos.userFileManager,
+            Handler.getMain(),
+            mContext.contentResolver,
+            kosmos.fakeGlobalSettings,
+        )
 
     private val iActivityManager = mock<IActivityManager>()
     private val notificationManager = mock<NotificationManager>()
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/IssueRecordingStateTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/IssueRecordingStateTest.kt
index 4ab3c7b203c4..83bdcd2161ee 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/IssueRecordingStateTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/recordissue/IssueRecordingStateTest.kt
@@ -16,6 +16,8 @@
 
 package com.android.systemui.recordissue
 
+import android.content.ContentResolver
+import android.os.Handler
 import android.testing.TestableLooper
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.SmallTest
@@ -23,11 +25,15 @@ import com.android.systemui.SysuiTestCase
 import com.android.systemui.kosmos.Kosmos
 import com.android.systemui.settings.userFileManager
 import com.android.systemui.settings.userTracker
+import com.android.systemui.util.settings.fakeGlobalSettings
 import com.google.common.truth.Truth
-import java.util.concurrent.CountDownLatch
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.Mock
+import org.mockito.MockitoAnnotations
+import org.mockito.kotlin.any
+import org.mockito.kotlin.verify
 
 @SmallTest
 @RunWith(AndroidJUnit4::class)
@@ -36,10 +42,19 @@ class IssueRecordingStateTest : SysuiTestCase() {
 
     private val kosmos = Kosmos()
     private lateinit var underTest: IssueRecordingState
+    @Mock private lateinit var resolver: ContentResolver
 
     @Before
     fun setup() {
-        underTest = IssueRecordingState(kosmos.userTracker, kosmos.userFileManager)
+        MockitoAnnotations.initMocks(this)
+        underTest =
+            IssueRecordingState(
+                kosmos.userTracker,
+                kosmos.userFileManager,
+                Handler.getMain(),
+                resolver,
+                kosmos.fakeGlobalSettings,
+            )
     }
 
     @Test
@@ -47,7 +62,14 @@ class IssueRecordingStateTest : SysuiTestCase() {
         val expected = true
 
         underTest.takeBugreport = expected
-        underTest = IssueRecordingState(kosmos.userTracker, kosmos.userFileManager)
+        underTest =
+            IssueRecordingState(
+                kosmos.userTracker,
+                kosmos.userFileManager,
+                Handler.getMain(),
+                resolver,
+                kosmos.fakeGlobalSettings,
+            )
 
         Truth.assertThat(underTest.takeBugreport).isEqualTo(expected)
     }
@@ -57,7 +79,14 @@ class IssueRecordingStateTest : SysuiTestCase() {
         val expected = true
 
         underTest.recordScreen = expected
-        underTest = IssueRecordingState(kosmos.userTracker, kosmos.userFileManager)
+        underTest =
+            IssueRecordingState(
+                kosmos.userTracker,
+                kosmos.userFileManager,
+                Handler.getMain(),
+                resolver,
+                kosmos.fakeGlobalSettings,
+            )
 
         Truth.assertThat(underTest.recordScreen).isEqualTo(expected)
     }
@@ -65,7 +94,14 @@ class IssueRecordingStateTest : SysuiTestCase() {
     @Test
     fun hasUserApprovedScreenRecording_isTrue_afterBeingMarkedAsCompleted() {
         underTest.markUserApprovalForScreenRecording()
-        underTest = IssueRecordingState(kosmos.userTracker, kosmos.userFileManager)
+        underTest =
+            IssueRecordingState(
+                kosmos.userTracker,
+                kosmos.userFileManager,
+                Handler.getMain(),
+                resolver,
+                kosmos.fakeGlobalSettings,
+            )
 
         Truth.assertThat(underTest.hasUserApprovedScreenRecording).isEqualTo(true)
     }
@@ -75,35 +111,35 @@ class IssueRecordingStateTest : SysuiTestCase() {
         val expected = setOf("a", "b", "c")
 
         underTest.tagTitles = expected
-        underTest = IssueRecordingState(kosmos.userTracker, kosmos.userFileManager)
+        underTest =
+            IssueRecordingState(
+                kosmos.userTracker,
+                kosmos.userFileManager,
+                Handler.getMain(),
+                resolver,
+                kosmos.fakeGlobalSettings,
+            )
 
         Truth.assertThat(underTest.tagTitles).isEqualTo(expected)
     }
 
     @Test
-    fun isRecording_callsListeners_onTheValueChanging() {
-        val count = CountDownLatch(1)
-        val listener = Runnable { count.countDown() }
+    fun addListener_registersContentObserver_ifListOfListenersIsNotEmpty() {
+        val listener = Runnable { /* No-op */ }
 
         underTest.addListener(listener)
-        underTest.isRecording = true
 
-        Truth.assertThat(count.count).isEqualTo(0)
+        verify(resolver).registerContentObserver(any(), any(), any())
     }
 
     @Test
-    fun isRecording_callsOnlyListeners_whoHaveNotBeenRemoved() {
-        val count1 = CountDownLatch(1)
-        val count2 = CountDownLatch(1)
-        val listener1 = Runnable { count1.countDown() }
-        val listener2 = Runnable { count2.countDown() }
-
-        underTest.addListener(listener1)
-        underTest.removeListener(listener1)
-        underTest.addListener(listener2)
-        underTest.isRecording = true
-
-        Truth.assertThat(count1.count).isEqualTo(1)
-        Truth.assertThat(count2.count).isEqualTo(0)
+    fun removeListener_unRegistersContentObserver_ifListOfListenersIsEmpty() {
+        val listener = Runnable { /* No-op */ }
+
+        underTest.addListener(listener)
+        underTest.removeListener(listener)
+
+        verify(resolver).registerContentObserver(any(), any(), any())
+        verify(resolver).unregisterContentObserver(any())
     }
 }
diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/RecordIssueTile.kt b/packages/SystemUI/src/com/android/systemui/qs/tiles/RecordIssueTile.kt
index 1792ebd5245d..028ac6f4ac18 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/tiles/RecordIssueTile.kt
+++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/RecordIssueTile.kt
@@ -111,10 +111,12 @@ constructor(
 
     override fun handleSetListening(listening: Boolean) {
         super.handleSetListening(listening)
-        if (listening) {
-            issueRecordingState.addListener(onRecordingChangeListener)
-        } else {
-            issueRecordingState.removeListener(onRecordingChangeListener)
+        bgExecutor.execute {
+            if (listening) {
+                issueRecordingState.addListener(onRecordingChangeListener)
+            } else {
+                issueRecordingState.removeListener(onRecordingChangeListener)
+            }
         }
     }
 
diff --git a/packages/SystemUI/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingDataInteractor.kt b/packages/SystemUI/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingDataInteractor.kt
index 1af328e1e0a8..09a6ce8e5ec0 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingDataInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/qs/tiles/impl/irecording/IssueRecordingDataInteractor.kt
@@ -41,7 +41,7 @@ constructor(
 
     override fun tileData(
         user: UserHandle,
-        triggers: Flow<DataUpdateTrigger>
+        triggers: Flow<DataUpdateTrigger>,
     ): Flow<IssueRecordingModel> =
         conflatedCallbackFlow {
                 val listener = Runnable { trySend(IssueRecordingModel(state.isRecording)) }
diff --git a/packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingServiceSession.kt b/packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingServiceSession.kt
index 43539335d8e4..16ca81a970e0 100644
--- a/packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingServiceSession.kt
+++ b/packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingServiceSession.kt
@@ -55,8 +55,10 @@ class IssueRecordingServiceSession(
     var screenRecord = false
 
     fun start() {
-        bgExecutor.execute { traceurConnection.startTracing(traceConfig) }
-        issueRecordingState.isRecording = true
+        bgExecutor.execute {
+            traceurConnection.startTracing(traceConfig)
+            issueRecordingState.isRecording = true
+        }
     }
 
     fun stop() {
@@ -69,8 +71,8 @@ class IssueRecordingServiceSession(
                 )
             }
             traceurConnection.stopTracing()
+            issueRecordingState.isRecording = false
         }
-        issueRecordingState.isRecording = false
     }
 
     fun share(notificationId: Int, screenRecording: Uri?) {
diff --git a/packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingState.kt b/packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingState.kt
index 51c437dbef24..ffdcf1b8811e 100644
--- a/packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingState.kt
+++ b/packages/SystemUI/src/com/android/systemui/recordissue/IssueRecordingState.kt
@@ -16,12 +16,20 @@
 
 package com.android.systemui.recordissue
 
+import android.annotation.SuppressLint
+import android.content.ContentResolver
 import android.content.Context
+import android.database.ContentObserver
+import android.os.Handler
+import androidx.annotation.VisibleForTesting
+import androidx.annotation.WorkerThread
 import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.dagger.qualifiers.Background
 import com.android.systemui.recordissue.RecordIssueModule.Companion.TILE_SPEC
 import com.android.systemui.res.R
 import com.android.systemui.settings.UserFileManager
 import com.android.systemui.settings.UserTracker
+import com.android.systemui.util.settings.GlobalSettings
 import com.android.traceur.PresetTraceConfigs
 import com.android.traceur.TraceConfig
 import java.util.concurrent.CopyOnWriteArrayList
@@ -31,12 +39,20 @@ import javax.inject.Inject
 class IssueRecordingState
 @Inject
 constructor(
-    userTracker: UserTracker,
-    userFileManager: UserFileManager,
+    private val userTracker: UserTracker,
+    private val userFileManager: UserFileManager,
+    @Background bgHandler: Handler,
+    private val resolver: ContentResolver,
+    private val globalSettings: GlobalSettings,
 ) {
 
-    private val prefs =
-        userFileManager.getSharedPreferences(TILE_SPEC, Context.MODE_PRIVATE, userTracker.userId)
+    private val prefs
+        get() =
+            userFileManager.getSharedPreferences(
+                TILE_SPEC,
+                Context.MODE_PRIVATE,
+                userTracker.userId,
+            )
 
     val customTraceState = CustomTraceState(prefs)
 
@@ -77,22 +93,52 @@ constructor(
 
     private val listeners = CopyOnWriteArrayList<Runnable>()
 
+    @VisibleForTesting
+    val onRecordingChangeListener =
+        object : ContentObserver(bgHandler) {
+            override fun onChange(selfChange: Boolean) {
+                isRecording = globalSettings.getInt(KEY_ONGOING_ISSUE_RECORDING, 0) == 1
+                listeners.forEach(Runnable::run)
+            }
+        }
+
+    /**
+     * isRecording is purposely always set to false at the initialization of the record issue qs
+     * tile. We want to avoid a situation where the System UI crashed / the device was restarted in
+     * the middle of a trace session and the QS tile is in an active state even though no tracing is
+     * ongoing.
+     */
     var isRecording = false
+        @WorkerThread
         set(value) {
+            globalSettings.putInt(KEY_ONGOING_ISSUE_RECORDING, if (value) 1 else 0)
             field = value
-            listeners.forEach(Runnable::run)
         }
 
     fun markUserApprovalForScreenRecording() {
         hasUserApprovedScreenRecording = true
     }
 
+    @WorkerThread
+    @SuppressLint("RegisterContentObserverViaContentResolver")
     fun addListener(listener: Runnable) {
+        if (listeners.isEmpty()) {
+            resolver.registerContentObserver(
+                globalSettings.getUriFor(KEY_ONGOING_ISSUE_RECORDING),
+                false,
+                onRecordingChangeListener,
+            )
+        }
         listeners.add(listener)
     }
 
+    @WorkerThread
+    @SuppressLint("RegisterContentObserverViaContentResolver")
     fun removeListener(listener: Runnable) {
         listeners.remove(listener)
+        if (listeners.isEmpty()) {
+            resolver.unregisterContentObserver(onRecordingChangeListener)
+        }
     }
 
     companion object {
@@ -100,6 +146,7 @@ constructor(
         private const val HAS_APPROVED_SCREEN_RECORDING = "HasApprovedScreenRecord"
         private const val KEY_RECORD_SCREEN = "key_recordScreen"
         private const val KEY_TAG_TITLES = "key_tagTitles"
+        private const val KEY_ONGOING_ISSUE_RECORDING = "issueRecordingOngoing"
         const val KEY_ISSUE_TYPE_INDEX = "key_issueTypeIndex"
         const val ISSUE_TYPE_NOT_SET = -1
         const val TAG_TITLE_DELIMITER = ": "