Merge "Fix dialog flicker when onDisplayChanged is received" into main
diff --git a/packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayRepository.kt b/packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayRepository.kt
index 9e54b5c..26c5ea6 100644
--- a/packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/display/data/repository/DisplayRepository.kt
@@ -23,9 +23,9 @@
import android.hardware.display.DisplayManager.EVENT_FLAG_DISPLAY_CHANGED
import android.hardware.display.DisplayManager.EVENT_FLAG_DISPLAY_REMOVED
import android.os.Handler
-import android.os.Trace
import android.util.Log
import android.view.Display
+import com.android.app.tracing.FlowTracing.traceEach
import com.android.app.tracing.traceSection
import com.android.systemui.common.coroutine.ConflatedCallbackFlow.conflatedCallbackFlow
import com.android.systemui.dagger.SysUISingleton
@@ -43,10 +43,9 @@
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
-import kotlinx.coroutines.flow.filter
+import kotlinx.coroutines.flow.filterIsInstance
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.flow.shareIn
import kotlinx.coroutines.flow.stateIn
@@ -97,7 +96,6 @@
@Application applicationScope: CoroutineScope,
@Background backgroundCoroutineDispatcher: CoroutineDispatcher
) : DisplayRepository {
- // Displays are enabled only after receiving them in [onDisplayAdded]
private val allDisplayEvents: Flow<DisplayEvent> =
conflatedCallbackFlow {
val callback =
@@ -124,16 +122,22 @@
awaitClose { displayManager.unregisterDisplayListener(callback) }
}
.onStart { emit(DisplayEvent.Changed(Display.DEFAULT_DISPLAY)) }
+ .debugLog("allDisplayEvents")
.flowOn(backgroundCoroutineDispatcher)
override val displayChangeEvent: Flow<Int> =
- allDisplayEvents.filter { it is DisplayEvent.Changed }.map { it.displayId }
+ allDisplayEvents.filterIsInstance<DisplayEvent.Changed>().map { event -> event.displayId }
override val displayAdditionEvent: Flow<Display?> =
- allDisplayEvents
- .filter { it is DisplayEvent.Added }
- .map { displayManager.getDisplay(it.displayId) }
+ allDisplayEvents.filterIsInstance<DisplayEvent.Added>().map {
+ displayManager.getDisplay(it.displayId)
+ }
+ /**
+ * Represents displays that went though the [DisplayListener.onDisplayAdded] callback.
+ *
+ * Those are commonly the ones provided by [DisplayManager.getDisplays] by default.
+ */
private val enabledDisplays =
allDisplayEvents
.map { getDisplays() }
@@ -143,9 +147,7 @@
override val displays: Flow<Set<Display>> = enabledDisplays
private fun getDisplays(): Set<Display> =
- traceSection("DisplayRepository#getDisplays()") {
- displayManager.displays?.toSet() ?: emptySet()
- }
+ traceSection("$TAG#getDisplays()") { displayManager.displays?.toSet() ?: emptySet() }
/** Propagate to the listeners only enabled displays */
private val enabledDisplayIds: Flow<Set<Int>> =
@@ -153,7 +155,8 @@
.map { enabledDisplaysSet -> enabledDisplaysSet.map { it.displayId }.toSet() }
.debugLog("enabledDisplayIds")
- private val ignoredDisplayIds = MutableStateFlow<Set<Int>>(emptySet())
+ private val _ignoredDisplayIds = MutableStateFlow<Set<Int>>(emptySet())
+ private val ignoredDisplayIds: Flow<Set<Int>> = _ignoredDisplayIds.debugLog("ignoredDisplayIds")
private fun getInitialConnectedDisplays(): Set<Int> =
displayManager
@@ -177,7 +180,7 @@
Log.d(TAG, "display with id=$id connected.")
}
connectedIds += id
- ignoredDisplayIds.value -= id
+ _ignoredDisplayIds.value -= id
trySend(connectedIds.toSet())
}
@@ -186,7 +189,7 @@
if (DEBUG) {
Log.d(TAG, "display with id=$id disconnected.")
}
- ignoredDisplayIds.value -= id
+ _ignoredDisplayIds.value -= id
trySend(connectedIds.toSet())
}
}
@@ -214,17 +217,23 @@
private val connectedExternalDisplayIds: Flow<Set<Int>> =
connectedDisplayIds
.map { connectedDisplayIds ->
- connectedDisplayIds
- .filter { id -> displayManager.getDisplay(id)?.type == Display.TYPE_EXTERNAL }
- .toSet()
+ traceSection("$TAG#filteringExternalDisplays") {
+ connectedDisplayIds
+ .filter { id -> getDisplayType(id) == Display.TYPE_EXTERNAL }
+ .toSet()
+ }
}
.flowOn(backgroundCoroutineDispatcher)
.debugLog("connectedExternalDisplayIds")
+ private fun getDisplayType(displayId: Int): Int? =
+ traceSection("$TAG#getDisplayType") { displayManager.getDisplay(displayId)?.type }
+
/**
- * Pending displays are the ones connected, but not enabled and not ignored. A connected display
- * is ignored after the user makes the decision to use it or not. For now, the initial decision
- * from the user is final and not reversible.
+ * Pending displays are the ones connected, but not enabled and not ignored.
+ *
+ * A connected display is ignored after the user makes the decision to use it or not. For now,
+ * the initial decision from the user is final and not reversible.
*/
private val pendingDisplayIds: Flow<Set<Int>> =
combine(enabledDisplayIds, connectedExternalDisplayIds, ignoredDisplayIds) {
@@ -241,12 +250,16 @@
}
connectedExternalDisplayIds - enabledDisplaysIds - ignoredDisplayIds
}
- .debugLog("pendingDisplayIds")
+ .debugLog("allPendingDisplayIds")
+
+ /** Which display id should be enabled among the pending ones. */
+ private val pendingDisplayId: Flow<Int?> =
+ pendingDisplayIds.map { it.maxOrNull() }.distinctUntilChanged().debugLog("pendingDisplayId")
override val pendingDisplay: Flow<DisplayRepository.PendingDisplay?> =
- pendingDisplayIds
- .map { pendingDisplayIds ->
- val id = pendingDisplayIds.maxOrNull() ?: return@map null
+ pendingDisplayId
+ .map { displayId ->
+ val id = displayId ?: return@map null
object : DisplayRepository.PendingDisplay {
override val id = id
@@ -263,7 +276,7 @@
override suspend fun ignore() {
traceSection("DisplayRepository#ignore($id)") {
- ignoredDisplayIds.value += id
+ _ignoredDisplayIds.value += id
}
}
@@ -282,11 +295,7 @@
private fun <T> Flow<T>.debugLog(flowName: String): Flow<T> {
return if (DEBUG) {
- this.onEach {
- Log.d(TAG, "$flowName: $it")
- Trace.asyncTraceForTrackEnd(Trace.TRACE_TAG_APP, "$TAG#$flowName", 0)
- Trace.asyncTraceForTrackBegin(Trace.TRACE_TAG_APP, "$TAG#$flowName", "$it", 0)
- }
+ traceEach(flowName, logcat = true)
} else {
this
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/display/data/repository/DisplayRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/display/data/repository/DisplayRepositoryTest.kt
index d80dd76..806930d 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/display/data/repository/DisplayRepositoryTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/display/data/repository/DisplayRepositoryTest.kt
@@ -380,6 +380,32 @@
}
@Test
+ fun pendingDisplay_afterConfigChanged_doesNotChange() =
+ testScope.runTest {
+ val pendingDisplay by lastPendingDisplay()
+
+ sendOnDisplayConnected(1, TYPE_EXTERNAL)
+ val initialPendingDisplay: DisplayRepository.PendingDisplay? = pendingDisplay
+ assertThat(pendingDisplay).isNotNull()
+ sendOnDisplayChanged(1)
+
+ assertThat(initialPendingDisplay).isEqualTo(pendingDisplay)
+ }
+
+ @Test
+ fun pendingDisplay_afterNewHigherDisplayConnected_changes() =
+ testScope.runTest {
+ val pendingDisplay by lastPendingDisplay()
+
+ sendOnDisplayConnected(1, TYPE_EXTERNAL)
+ val initialPendingDisplay: DisplayRepository.PendingDisplay? = pendingDisplay
+ assertThat(pendingDisplay).isNotNull()
+ sendOnDisplayConnected(2, TYPE_EXTERNAL)
+
+ assertThat(initialPendingDisplay).isNotEqualTo(pendingDisplay)
+ }
+
+ @Test
fun onPendingDisplay_OneInternalAndOneExternalDisplay_internalIgnored() =
testScope.runTest {
val pendingDisplay by lastPendingDisplay()
@@ -466,6 +492,10 @@
connectedDisplayListener.value.onDisplayConnected(id)
}
+ private fun sendOnDisplayChanged(id: Int) {
+ connectedDisplayListener.value.onDisplayChanged(id)
+ }
+
private fun setDisplays(displays: List<Display>) {
whenever(displayManager.displays).thenReturn(displays.toTypedArray())
displays.forEach { display ->