Fix live timers flickering when pausing

This change adds a flag which uses a stable id as the key for timers,
instead of using the smartspace target id. The stable id stays the same
when pausing/playing timers, which allows us to re-use the existing
AndroidView of the timer instead of creating a new one.

Bug: 353801573
Test: atest SystemUiRoboTests
Flag: com.android.systemui.communal_timer_flicker_fix
Change-Id: I1124a9148ddd635899eb5d9f6780cd49add9f232
diff --git a/packages/SystemUI/aconfig/systemui.aconfig b/packages/SystemUI/aconfig/systemui.aconfig
index 27f4305..1f1495a 100644
--- a/packages/SystemUI/aconfig/systemui.aconfig
+++ b/packages/SystemUI/aconfig/systemui.aconfig
@@ -1005,6 +1005,16 @@
 }
 
 flag {
+  name: "communal_timer_flicker_fix"
+  namespace: "systemui"
+  description: "fixes timers on the hub flickering when pausing"
+  bug: "353801573"
+  metadata {
+    purpose: PURPOSE_BUGFIX
+  }
+}
+
+flag {
   name: "app_clips_backlinks"
   namespace: "systemui"
   description: "Enables Backlinks improvement feature in App Clips"
diff --git a/packages/SystemUI/compose/features/src/com/android/systemui/communal/ui/compose/CommunalHub.kt b/packages/SystemUI/compose/features/src/com/android/systemui/communal/ui/compose/CommunalHub.kt
index e29e0fd..2d81f3c 100644
--- a/packages/SystemUI/compose/features/src/com/android/systemui/communal/ui/compose/CommunalHub.kt
+++ b/packages/SystemUI/compose/features/src/com/android/systemui/communal/ui/compose/CommunalHub.kt
@@ -152,6 +152,7 @@
 import androidx.compose.ui.unit.times
 import androidx.compose.ui.util.fastAll
 import androidx.compose.ui.viewinterop.AndroidView
+import androidx.compose.ui.viewinterop.NoOpUpdate
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import androidx.window.layout.WindowMetricsCalculator
 import com.android.compose.animation.Easings.Emphasized
@@ -159,6 +160,7 @@
 import com.android.compose.theme.LocalAndroidColorScheme
 import com.android.compose.ui.graphics.painter.rememberDrawablePainter
 import com.android.internal.R.dimen.system_app_widget_background_radius
+import com.android.systemui.Flags.communalTimerFlickerFix
 import com.android.systemui.communal.domain.model.CommunalContentModel
 import com.android.systemui.communal.shared.model.CommunalContentSize
 import com.android.systemui.communal.shared.model.CommunalScenes
@@ -1336,9 +1338,15 @@
         factory = { context ->
             SmartspaceAppWidgetHostView(context).apply {
                 interactionHandler?.let { setInteractionHandler(it) }
-                updateAppWidget(model.remoteViews)
+                if (!communalTimerFlickerFix()) {
+                    updateAppWidget(model.remoteViews)
+                }
             }
         },
+        update =
+            if (communalTimerFlickerFix()) {
+                { view: SmartspaceAppWidgetHostView -> view.updateAppWidget(model.remoteViews) }
+            } else NoOpUpdate,
         // For reusing composition in lazy lists.
         onReset = {},
     )
diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt
index c1816ed..d251585 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepositoryImplTest.kt
@@ -22,6 +22,7 @@
 import android.platform.test.annotations.EnableFlags
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.SmallTest
+import com.android.systemui.Flags.FLAG_COMMUNAL_TIMER_FLICKER_FIX
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.communal.smartspace.CommunalSmartspaceController
 import com.android.systemui.concurrency.fakeExecutor
@@ -97,6 +98,7 @@
         }
 
     @EnableFlags(FLAG_REMOTE_VIEWS)
+    @DisableFlags(FLAG_COMMUNAL_TIMER_FLICKER_FIX)
     @Test
     fun communalTimers_onlyShowTimersWithRemoteViews() =
         testScope.runTest {
@@ -138,6 +140,48 @@
             assertThat(communalTimers?.first()?.smartspaceTargetId).isEqualTo("timer-1-started")
         }
 
+    @EnableFlags(FLAG_REMOTE_VIEWS, FLAG_COMMUNAL_TIMER_FLICKER_FIX)
+    @Test
+    fun communalTimers_onlyShowTimersWithRemoteViews_timerFlickerFix() =
+        testScope.runTest {
+            underTest.startListening()
+
+            val communalTimers by collectLastValue(underTest.timers)
+            runCurrent()
+            fakeExecutor.runAllReady()
+
+            with(captureSmartspaceTargetListener()) {
+                onSmartspaceTargetsUpdated(
+                    listOf(
+                        // Invalid. Not a timer
+                        mock<SmartspaceTarget> {
+                            on { smartspaceTargetId }.doReturn("weather")
+                            on { featureType }.doReturn(SmartspaceTarget.FEATURE_WEATHER)
+                        },
+                        // Invalid. RemoteViews absent
+                        mock<SmartspaceTarget> {
+                            on { smartspaceTargetId }.doReturn("timer-0-started")
+                            on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
+                            on { remoteViews }.doReturn(null)
+                            on { creationTimeMillis }.doReturn(1000)
+                        },
+                        // Valid
+                        mock<SmartspaceTarget> {
+                            on { smartspaceTargetId }.doReturn("timer-1-started")
+                            on { featureType }.doReturn(SmartspaceTarget.FEATURE_TIMER)
+                            on { remoteViews }.doReturn(mock())
+                            on { creationTimeMillis }.doReturn(2000)
+                        },
+                    )
+                )
+            }
+            runCurrent()
+
+            // Verify that only the valid target is listed
+            assertThat(communalTimers).hasSize(1)
+            assertThat(communalTimers?.first()?.smartspaceTargetId).isEqualTo("timer-1")
+        }
+
     @EnableFlags(FLAG_REMOTE_VIEWS)
     @Test
     fun communalTimers_cacheCreationTime() =
diff --git a/packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt b/packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt
index e1d9bef..86241a5 100644
--- a/packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalSmartspaceRepository.kt
@@ -19,6 +19,7 @@
 import android.app.smartspace.SmartspaceTarget
 import android.os.Parcelable
 import androidx.annotation.VisibleForTesting
+import com.android.systemui.Flags.communalTimerFlickerFix
 import com.android.systemui.communal.data.model.CommunalSmartspaceTimer
 import com.android.systemui.communal.smartspace.CommunalSmartspaceController
 import com.android.systemui.dagger.SysUISingleton
@@ -80,7 +81,8 @@
                     // The view layer should have the instance based smartspaceTargetId instead of
                     // stable id, so that when a new instance of the timer is created, for example,
                     // when it is paused, the view should re-render its remote views.
-                    smartspaceTargetId = target.smartspaceTargetId,
+                    smartspaceTargetId =
+                        if (communalTimerFlickerFix()) stableId else target.smartspaceTargetId,
                     createdTimestampMillis = targetCreationTimes[stableId]!!,
                     remoteViews = target.remoteViews!!,
                 )