Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8262228a authored by Alex Shabalin's avatar Alex Shabalin
Browse files

Don't change the suggestion during connection or displaying an error. 

If the user initiated a connection and that connection is in progress
(STATE_CONNECTING) or failed (STATE_CONNECTING_FAILED), don't replace or
clear the suggested device metadata exposed to the UI when the
underlying suggested device information in the Media Router got changed.

This ensures that the suggested device chip UI is stable during the
connection attempt.

Fix: 441546017
Test: atest SuggestedDeviceManagerTest
Flag: EXEMPT BUGFIX
Change-Id: Ic5004743a5907075070ca7316113608244d590c4
parent dcb9a0bf

packages/SettingsLib/src/com/android/settingslib/media/SuggestedDeviceManager.kt

+27 −26
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import android.media.SuggestedDeviceInfo 
import android.os.Handler

import android.util.Log

import androidx.annotation.GuardedBy

import androidx.annotation.VisibleForTesting

import com.android.media.flags.Flags.useSuggestedDeviceConnectionManager

import com.android.settingslib.media.LocalMediaManager.MediaDeviceState

import com.android.settingslib.media.LocalMediaManager.MediaDeviceState.STATE_CONNECTED
@@ -32,8 +33,10 @@ import java.util.concurrent.CopyOnWriteArraySet 


private const val TAG = "SuggestedDeviceManager"



private const val CONNECTING_TIMEOUT_MS = 20_000L

private const val CONNECTING_FAILED_TIMEOUT_MS = 10_000L

@VisibleForTesting

const val CONNECTING_TIMEOUT_MS = 30_000L

@VisibleForTesting

const val CONNECTING_FAILED_TIMEOUT_MS = 10_000L



/**

 * Provides data to render and handles user interactions for the suggested device chip within the
@@ -57,10 +60,10 @@ class SuggestedDeviceManager( 
  @GuardedBy("lock") private var mediaDevices: List<MediaDevice> = listOf()

  @GuardedBy("lock") private var topSuggestion: SuggestedDeviceInfo? = null

  @GuardedBy("lock") private var suggestedDeviceState: SuggestedDeviceState? = null

  // Overrides the connection state obtained from the [MediaDevice] that matches the

  // [topSuggestion]. This is necessary to prevent UI state jumps during connection attempts or

  // when displaying error messages.

  @GuardedBy("lock") @MediaDeviceState private var connectionStateOverride: Int? = null

  // Overrides the suggested device state obtained from the [MediaDevice] that matches the

  // [topSuggestion]. This is necessary to prevent hiding or changing the title of the suggested

  // device chip during connection attempts or when displaying error messages.

  @GuardedBy("lock") private var suggestedStateOverride: SuggestedDeviceState? = null



  init {

    if (useSuggestedDeviceConnectionManager()) {
@@ -71,14 +74,14 @@ class SuggestedDeviceManager( 
    }

  }



  private val onConnectionStateOverrideExpiredRunnable = Runnable {

  private val onSuggestedStateOverrideExpiredRunnable = Runnable {

    synchronized(lock) {

      if (connectionStateOverride == STATE_CONNECTING_FAILED) {

      if (suggestedStateOverride?.connectionState == STATE_CONNECTING_FAILED) {

        // After the connection error, hide the suggestion chip until the new suggestion is

        // requested.

        topSuggestion = null

      }

      connectionStateOverride = null

      suggestedStateOverride = null

      updateSuggestedDeviceStateLocked(topSuggestion, mediaDevices)

    }

    dispatchOnSuggestedDeviceUpdated()
@@ -164,7 +167,7 @@ class SuggestedDeviceManager( 
      Log.w(TAG, "Suggestion got changed, aborting connection.")

      return

    }

    overrideConnectionStateWithExpiration(

    overrideSuggestedStateWithExpiration(

      connectionState = STATE_CONNECTING,

      timeoutMs = CONNECTING_TIMEOUT_MS,

    )
@@ -187,7 +190,7 @@ class SuggestedDeviceManager( 
      return

    }

    if (!success) {

      overrideConnectionStateWithExpiration(

      overrideSuggestedStateWithExpiration(

        connectionState = STATE_CONNECTING_FAILED,

        timeoutMs = CONNECTING_FAILED_TIMEOUT_MS,

      )
@@ -222,21 +225,20 @@ class SuggestedDeviceManager( 
    newMediaDevices: List<MediaDevice>,

  ): SuggestedDeviceState? {

    if (newTopSuggestion == null) {

      clearConnectionStateOverrideLocked()

      return null

      return suggestedStateOverride ?: null

    }



    val newConnectionState =

      getConnectionStateFromMatchedDeviceLocked(newTopSuggestion, newMediaDevices)

    if (shouldClearStateOverride(newTopSuggestion, newConnectionState)) {

      clearConnectionStateOverrideLocked()

    if (shouldClearStateOverride(newConnectionState)) {

      clearSuggestedStateOverrideLocked()

    }



    return if (isConnectedState(newConnectionState)) {

      // Don't display a suggestion if the MediaDevice that matches the suggestion is connected.

      null

    } else {

      SuggestedDeviceState(newTopSuggestion, connectionStateOverride ?: newConnectionState)

      suggestedStateOverride ?: SuggestedDeviceState(newTopSuggestion, newConnectionState)

    }

  }
@@ -254,12 +256,11 @@ class SuggestedDeviceManager( 
  }



  private fun shouldClearStateOverride(

    newTopSuggestion: SuggestedDeviceInfo,

    @MediaDeviceState newConnectionState: Int,

  ): Boolean {

    // Don't clear the state override if a matched device is in DISCONNECTED state. Currently, the

    // DISCONNECTED state can be reported during connection that can lead to UI flicker.

    return !isCurrentSuggestion(newTopSuggestion) || newConnectionState != STATE_DISCONNECTED

    return newConnectionState != STATE_DISCONNECTED

  }



  private fun isConnectedState(@MediaDeviceState state: Int): Boolean =
@@ -273,20 +274,20 @@ class SuggestedDeviceManager( 
      suggestedDeviceState?.suggestedDeviceInfo?.routeId == suggestedDeviceInfo.routeId

    }



  private fun overrideConnectionStateWithExpiration(connectionState: Int, timeoutMs: Long) {

  private fun overrideSuggestedStateWithExpiration(connectionState: Int, timeoutMs: Long) {

    synchronized(lock) {

      connectionStateOverride = connectionState

      suggestedDeviceState = suggestedDeviceState?.copy(connectionState = connectionState)

      handler.removeCallbacks(onConnectionStateOverrideExpiredRunnable)

      handler.postDelayed(onConnectionStateOverrideExpiredRunnable, timeoutMs)

      suggestedStateOverride = suggestedDeviceState?.copy(connectionState = connectionState)

      suggestedDeviceState = suggestedStateOverride

      handler.removeCallbacks(onSuggestedStateOverrideExpiredRunnable)

      handler.postDelayed(onSuggestedStateOverrideExpiredRunnable, timeoutMs)

    }

    dispatchOnSuggestedDeviceUpdated()

  }



  @GuardedBy("lock")

  private fun clearConnectionStateOverrideLocked() {

    connectionStateOverride = null

    handler.removeCallbacks(onConnectionStateOverrideExpiredRunnable)

  private fun clearSuggestedStateOverrideLocked() {

    suggestedStateOverride = null

    handler.removeCallbacks(onSuggestedStateOverrideExpiredRunnable)

  }



  private fun dispatchOnSuggestedDeviceUpdated() {

packages/SettingsLib/tests/robotests/src/com/android/settingslib/media/SuggestedDeviceManagerTest.kt

+64 −4
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ import org.junit.Rule 
import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.kotlin.any

import org.mockito.kotlin.anyOrNull

import org.mockito.kotlin.argumentCaptor

import org.mockito.kotlin.clearInvocations

import org.mockito.kotlin.doReturn
@@ -244,6 +245,67 @@ class SuggestedDeviceManagerTest { 
    verify(listener).onSuggestedDeviceStateUpdated(expectedState2)

  }



  @Test

  fun onDeviceSuggestionsUpdated_hasStateOverrideAndNewSuggestionDifferent_keepsOverriddenState() {

    onDeviceSuggestionsUpdated_hasStateOverride_keepsOverriddenState(

      initialSuggestedDeviceInfo = suggestedDeviceInfo1,

      updatedSuggestedDeviceInfo = suggestedDeviceInfo2,

    )

  }



  @Test

  fun onDeviceSuggestionsUpdated_hasStateOverrideAndNewSuggestionNull_keepsOverriddenState() {

    onDeviceSuggestionsUpdated_hasStateOverride_keepsOverriddenState(

      initialSuggestedDeviceInfo = suggestedDeviceInfo1,

      updatedSuggestedDeviceInfo = null,

    )

  }



  fun onDeviceSuggestionsUpdated_hasStateOverride_keepsOverriddenState(

    initialSuggestedDeviceInfo: SuggestedDeviceInfo,

    updatedSuggestedDeviceInfo: SuggestedDeviceInfo?,

  ) {

    val deviceCallback = addListenerAndCaptureCallback(listener)

    deviceCallback.onDeviceListUpdate(listOf(mediaDevice1, mediaDevice2))



    // Initial suggested device is set.

    deviceCallback.onDeviceSuggestionsUpdated(listOf(initialSuggestedDeviceInfo))

    val initialSuggestedDeviceState =

      SuggestedDeviceState(initialSuggestedDeviceInfo, mediaDevice1.state)

    verify(listener).onSuggestedDeviceStateUpdated(initialSuggestedDeviceState)



    // Emulate starting connection and subsequently setting the override.

    mSuggestedDeviceManager.connectSuggestedDevice(initialSuggestedDeviceState, routingChangeInfo)

    val connectingSuggestedState =

      initialSuggestedDeviceState.copy(connectionState = STATE_CONNECTING)

    verify(listener).onSuggestedDeviceStateUpdated(connectingSuggestedState)

    clearInvocations(listener)



    // A different suggested device is set.

    deviceCallback.onDeviceSuggestionsUpdated(listOf(updatedSuggestedDeviceInfo))



    // The overridden state hasn't changed

    verify(listener, never()).onSuggestedDeviceStateUpdated(anyOrNull())

    assertThat(mSuggestedDeviceManager.getSuggestedDevice()).isEqualTo(connectingSuggestedState)



    // Emulate connection failure and subsequently setting the override.

    deviceCallback.onConnectSuggestedDeviceFinished(

      initialSuggestedDeviceState,

      false,

    )

    connectionFinishedCallback?.invoke(initialSuggestedDeviceState, false)

    val failedSuggestedState = initialSuggestedDeviceState.copy(connectionState = STATE_CONNECTING_FAILED)

    verify(listener).onSuggestedDeviceStateUpdated(failedSuggestedState)

    clearInvocations(listener)



    // A different suggested device is set.

    deviceCallback.onDeviceSuggestionsUpdated(listOf(updatedSuggestedDeviceInfo))



    // The overridden state hasn't changed

    verify(listener, never()).onSuggestedDeviceStateUpdated(anyOrNull())

    assertThat(mSuggestedDeviceManager.getSuggestedDevice()).isEqualTo(failedSuggestedState)

  }



  @Test

  fun onDeviceSuggestionsUpdated_suggestionCleared_dispatchesNull() {

    val deviceCallback = addListenerAndCaptureCallback(listener)
@@ -326,7 +388,6 @@ class SuggestedDeviceManagerTest { 


  @Test

  fun onTimeout_fromConnectingOverride_dispatchesDisconnectedState() {

    val expectedConnectingTimeoutMs = 20_000L

    val deviceCallback = addListenerAndCaptureCallback(listener)



    deviceCallback.onDeviceListUpdate(listOf(mediaDevice1))
@@ -344,7 +405,7 @@ class SuggestedDeviceManagerTest { 


    clearInvocations(listener)

    // Check the state one second before the timeout is reached.

    ShadowLooper.idleMainLooper(expectedConnectingTimeoutMs - 1_000, TimeUnit.MILLISECONDS)

    ShadowLooper.idleMainLooper(CONNECTING_TIMEOUT_MS - 1_000, TimeUnit.MILLISECONDS)

    verify(listener, never()).onSuggestedDeviceStateUpdated(any())



    clearInvocations(listener)
@@ -404,7 +465,6 @@ class SuggestedDeviceManagerTest { 
  }



  fun onTimeout_fromConnectingFailedOverride_dispatchesNullState() {

    val expectedConnectingFailedTimeoutMs = 10_000L

    val deviceCallback = addListenerAndCaptureCallback(listener)



    deviceCallback.onDeviceListUpdate(listOf(mediaDevice1))
@@ -425,7 +485,7 @@ class SuggestedDeviceManagerTest { 


    clearInvocations(listener)

    // One second before the timeout is reached - no events are dispatched.

    ShadowLooper.idleMainLooper(expectedConnectingFailedTimeoutMs - 1_000, TimeUnit.MILLISECONDS)

    ShadowLooper.idleMainLooper(CONNECTING_FAILED_TIMEOUT_MS - 1_000, TimeUnit.MILLISECONDS)

    verify(listener, never()).onSuggestedDeviceStateUpdated(any())



    clearInvocations(listener)