Merge "Avoid reentrant callback when setting up listeners" into rvc-dev am:... (2bf3dea2) · Commits · e / os / android_frameworks_base

packages/SystemUI/src/com/android/systemui/media/MediaTimeoutListener.kt

+40 −5

Original line number	Original line	Diff line number	Diff line
	@@ -54,7 +54,32 @@ class MediaTimeoutListener @Inject constructor(
	if (mediaListeners.containsKey(key)) {		if (mediaListeners.containsKey(key)) {
	return		return
	}		}
			// Having an old key means that we're migrating from/to resumption. We should invalidate
			// the old listener and create a new one.
			val migrating = oldKey != null && key != oldKey
			var wasPlaying = false
			if (migrating) {
			if (mediaListeners.containsKey(oldKey)) {
			val oldListener = mediaListeners.remove(oldKey)
			wasPlaying = oldListener?.playing ?: false
			oldListener?.destroy()
			if (DEBUG) Log.d(TAG, "migrating key $oldKey to $key, for resumption")
			} else {
			Log.w(TAG, "Old key $oldKey for player $key doesn't exist. Continuing...")
			}
			}
	mediaListeners[key] = PlaybackStateListener(key, data)		mediaListeners[key] = PlaybackStateListener(key, data)

			// If a player becomes active because of a migration, we'll need to broadcast its state.
			// Doing it now would lead to reentrant callbacks, so let's wait until we're done.
			if (migrating && mediaListeners[key]?.playing != wasPlaying) {
			mainExecutor.execute {
			if (mediaListeners[key]?.playing == true) {
			if (DEBUG) Log.d(TAG, "deliver delayed playback state for $key")
			timeoutCallback.invoke(key, false /* timedOut */)
			}
			}
			}
	}		}

	override fun onMediaDataRemoved(key: String) {		override fun onMediaDataRemoved(key: String) {
	@@ -71,7 +96,7 @@ class MediaTimeoutListener @Inject constructor(
	) : MediaController.Callback() {		) : MediaController.Callback() {

	var timedOut = false		var timedOut = false
	private var playing: Boolean? = null		var playing: Boolean? = null

	// Resume controls may have null token		// Resume controls may have null token
	private val mediaController = if (data.token != null) {		private val mediaController = if (data.token != null) {
	@@ -83,7 +108,9 @@ class MediaTimeoutListener @Inject constructor(

	init {		init {
	mediaController?.registerCallback(this)		mediaController?.registerCallback(this)
	onPlaybackStateChanged(mediaController?.playbackState)		// Let's register the cancellations, but not dispatch events now.
			// Timeouts didn't happen yet and reentrant events are troublesome.
			processState(mediaController?.playbackState, dispatchEvents = false)
	}		}

	fun destroy() {		fun destroy() {
	@@ -91,8 +118,12 @@ class MediaTimeoutListener @Inject constructor(
	}		}

	override fun onPlaybackStateChanged(state: PlaybackState?) {		override fun onPlaybackStateChanged(state: PlaybackState?) {
			processState(state, dispatchEvents = true)
			}

			private fun processState(state: PlaybackState?, dispatchEvents: Boolean) {
	if (DEBUG) {		if (DEBUG) {
	Log.v(TAG, "onPlaybackStateChanged: $state")		Log.v(TAG, "processState: $state")
	}		}

	val isPlaying = state != null && isPlayingState(state.state)		val isPlaying = state != null && isPlayingState(state.state)
	@@ -116,14 +147,18 @@ class MediaTimeoutListener @Inject constructor(
	Log.v(TAG, "Execute timeout for $key")		Log.v(TAG, "Execute timeout for $key")
	}		}
	timedOut = true		timedOut = true
			if (dispatchEvents) {
	timeoutCallback(key, timedOut)		timeoutCallback(key, timedOut)
			}
	}, PAUSED_MEDIA_TIMEOUT)		}, PAUSED_MEDIA_TIMEOUT)
	} else {		} else {
	expireMediaTimeout(key, "playback started - $state, $key")		expireMediaTimeout(key, "playback started - $state, $key")
	timedOut = false		timedOut = false
			if (dispatchEvents) {
	timeoutCallback(key, timedOut)		timeoutCallback(key, timedOut)
	}		}
	}		}
			}

	private fun expireMediaTimeout(mediaKey: String, reason: String) {		private fun expireMediaTimeout(mediaKey: String, reason: String) {
	cancellation?.apply {		cancellation?.apply {

packages/SystemUI/tests/src/com/android/systemui/media/MediaTimeoutListenerTest.kt

+21 −0

Original line number	Original line	Diff line number	Diff line
	@@ -32,7 +32,9 @@ import org.junit.Test
	import org.junit.runner.RunWith		import org.junit.runner.RunWith
	import org.mockito.ArgumentCaptor		import org.mockito.ArgumentCaptor
	import org.mockito.ArgumentMatchers.any		import org.mockito.ArgumentMatchers.any
			import org.mockito.ArgumentMatchers.anyBoolean
	import org.mockito.ArgumentMatchers.anyLong		import org.mockito.ArgumentMatchers.anyLong
			import org.mockito.ArgumentMatchers.anyString
	import org.mockito.Captor		import org.mockito.Captor
	import org.mockito.Mock		import org.mockito.Mock
	import org.mockito.Mockito		import org.mockito.Mockito
	@@ -118,6 +120,7 @@ class MediaTimeoutListenerTest : SysuiTestCase() {
	mediaTimeoutListener.onMediaDataLoaded(KEY, null, mediaData)		mediaTimeoutListener.onMediaDataLoaded(KEY, null, mediaData)
	verify(mediaController).registerCallback(capture(mediaCallbackCaptor))		verify(mediaController).registerCallback(capture(mediaCallbackCaptor))
	verify(executor).executeDelayed(capture(timeoutCaptor), anyLong())		verify(executor).executeDelayed(capture(timeoutCaptor), anyLong())
			verify(timeoutCallback, never()).invoke(anyString(), anyBoolean())
	}		}

	@Test		@Test
	@@ -132,6 +135,24 @@ class MediaTimeoutListenerTest : SysuiTestCase() {
	verify(mediaController, never()).unregisterCallback(anyObject())		verify(mediaController, never()).unregisterCallback(anyObject())
	}		}

			@Test
			fun testOnMediaDataLoaded_migratesKeys() {
			// From not playing
			mediaTimeoutListener.onMediaDataLoaded(KEY, null, mediaData)
			clearInvocations(mediaController)

			// To playing
			val playingState = mock(android.media.session.PlaybackState::class.java)
			`when`(playingState.state).thenReturn(PlaybackState.STATE_PLAYING)
			`when`(mediaController.playbackState).thenReturn(playingState)
			mediaTimeoutListener.onMediaDataLoaded("NEWKEY", KEY, mediaData)
			verify(mediaController).unregisterCallback(anyObject())
			verify(mediaController).registerCallback(anyObject())

			// Enqueues callback
			verify(executor).execute(anyObject())
			}

	@Test		@Test
	fun testOnPlaybackStateChanged_schedulesTimeout_whenPaused() {		fun testOnPlaybackStateChanged_schedulesTimeout_whenPaused() {
	// Assuming we're registered		// Assuming we're registered