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

Commit a20527d1 authored by Miranda Kephart's avatar Miranda Kephart Committed by Matt Casey
Browse files

Switch to using withContext in ScreenshotSoundController 

Avoid passing Deferred objects between the SoundController and
ScreenshotController, especially since we're not using the return value
anyway.

Bug: 326441239
Bug: 329659738
Fix: 326441239
Test: atest ScreenshotSoundControllerTest
Flag: NONE
Change-Id: I7d8e2e85f10eaaa75800b8e37a0269b4e63b8ca7
Merged-In: I7d8e2e85f10eaaa75800b8e37a0269b4e63b8ca7
parent dc577846

packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotController.java

+2 −2
Original line number Diff line number Diff line
@@ -576,7 +576,7 @@ public class ScreenshotController { 


    private void releaseMediaPlayer() {

        if (mScreenshotSoundController == null) return;

        mScreenshotSoundController.releaseScreenshotSound();

        mScreenshotSoundController.releaseScreenshotSoundAsync();

    }



    private void respondToKeyDismissal() {
@@ -891,7 +891,7 @@ public class ScreenshotController { 
    private void playCameraSoundIfNeeded() {

        if (mScreenshotSoundController == null) return;

        // the controller is not-null only on the default display controller

        mScreenshotSoundController.playCameraSound();

        mScreenshotSoundController.playScreenshotSoundAsync();

    }



    /**

packages/SystemUI/src/com/android/systemui/screenshot/ScreenshotSoundController.kt

+30 −10
Original line number Diff line number Diff line
@@ -21,22 +21,34 @@ import android.util.Log 
import com.android.app.tracing.coroutines.async

import com.android.systemui.dagger.qualifiers.Application

import com.android.systemui.dagger.qualifiers.Background

import com.google.errorprone.annotations.CanIgnoreReturnValue

import javax.inject.Inject

import kotlin.time.Duration.Companion.seconds

import kotlinx.coroutines.CoroutineDispatcher

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.Deferred

import kotlinx.coroutines.TimeoutCancellationException

import kotlinx.coroutines.launch

import kotlinx.coroutines.withContext

import kotlinx.coroutines.withTimeout



/** Controls sound reproduction after a screenshot is taken. */

interface ScreenshotSoundController {

    /** Reproduces the camera sound. */

    @CanIgnoreReturnValue fun playCameraSound(): Deferred<Unit>

    suspend fun playScreenshotSound()



    /** Releases the sound. [playCameraSound] behaviour is undefined after this has been called. */

    @CanIgnoreReturnValue fun releaseScreenshotSound(): Deferred<Unit>

    /**

     * Releases the sound. [playScreenshotSound] behaviour is undefined after this has been called.

     */

    suspend fun releaseScreenshotSound()



    /** Reproduces the camera sound. Used for compatibility with Java code. */

    fun playScreenshotSoundAsync()



    /**

     * Releases the sound. [playScreenshotSound] behaviour is undefined after this has been called.

     * Used for compatibility with Java code.

     */

    fun releaseScreenshotSoundAsync()

}



class ScreenshotSoundControllerImpl
@@ -47,8 +59,8 @@ constructor( 
    @Background private val bgDispatcher: CoroutineDispatcher

) : ScreenshotSoundController {



    val player: Deferred<MediaPlayer?> =

        coroutineScope.async("loadCameraSound", bgDispatcher) {

    private val player: Deferred<MediaPlayer?> =

        coroutineScope.async("loadScreenshotSound", bgDispatcher) {

            try {

                soundProvider.getScreenshotSound()

            } catch (e: IllegalStateException) {
@@ -57,8 +69,8 @@ constructor( 
            }

        }



    override fun playCameraSound(): Deferred<Unit> {

        return coroutineScope.async("playCameraSound", bgDispatcher) {

    override suspend fun playScreenshotSound() {

        withContext(bgDispatcher) {

            try {

                player.await()?.start()

            } catch (e: IllegalStateException) {
@@ -68,8 +80,8 @@ constructor( 
        }

    }



    override fun releaseScreenshotSound(): Deferred<Unit> {

        return coroutineScope.async("releaseScreenshotSound", bgDispatcher) {

    override suspend fun releaseScreenshotSound() {

        withContext(bgDispatcher) {

            try {

                withTimeout(1.seconds) { player.await()?.release() }

            } catch (e: TimeoutCancellationException) {
@@ -79,6 +91,14 @@ constructor( 
        }

    }



    override fun playScreenshotSoundAsync() {

        coroutineScope.launch { playScreenshotSound() }

    }



    override fun releaseScreenshotSoundAsync() {

        coroutineScope.launch { releaseScreenshotSound() }

    }



    private companion object {

        const val TAG = "ScreenshotSoundControllerImpl"

    }

packages/SystemUI/tests/src/com/android/systemui/screenshot/ScreenshotSoundControllerTest.kt

+37 −26
Original line number Diff line number Diff line
@@ -22,8 +22,10 @@ import com.android.systemui.SysuiTestCase 
import com.android.systemui.util.mockito.mock

import com.android.systemui.util.mockito.whenever

import java.lang.IllegalStateException

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.test.TestScope

import kotlinx.coroutines.test.UnconfinedTestDispatcher

import kotlinx.coroutines.test.advanceUntilIdle

import kotlinx.coroutines.test.runTest

import org.junit.Before

import org.junit.Test
@@ -31,12 +33,14 @@ import org.mockito.Mockito.never 
import org.mockito.Mockito.verify



@SmallTest

@OptIn(ExperimentalCoroutinesApi::class)

class ScreenshotSoundControllerTest : SysuiTestCase() {



    private val soundProvider = mock<ScreenshotSoundProvider>()

    private val mediaPlayer = mock<MediaPlayer>()

    private val bgDispatcher = UnconfinedTestDispatcher()

    private val scope = TestScope(bgDispatcher)



    @Before

    fun setup() {

        whenever(soundProvider.getScreenshotSound()).thenReturn(mediaPlayer)
@@ -45,49 +49,56 @@ class ScreenshotSoundControllerTest : SysuiTestCase() { 
    @Test

    fun init_soundLoading() {

        createController()

        bgDispatcher.scheduler.runCurrent()

        scope.advanceUntilIdle()



        verify(soundProvider).getScreenshotSound()

    }



    @Test

    fun init_soundLoadingException_playAndReleaseDoNotThrow() = runTest {

    fun init_soundLoadingException_playAndReleaseDoNotThrow() =

        scope.runTest {

            whenever(soundProvider.getScreenshotSound()).thenThrow(IllegalStateException())



            val controller = createController()



        controller.playCameraSound().await()

        controller.releaseScreenshotSound().await()

            controller.playScreenshotSound()

            advanceUntilIdle()



            verify(mediaPlayer, never()).start()

            verify(mediaPlayer, never()).release()

        }



    @Test

    fun playCameraSound_soundLoadingSuccessful_mediaPlayerPlays() = runTest {

    fun playCameraSound_soundLoadingSuccessful_mediaPlayerPlays() =

        scope.runTest {

            val controller = createController()



        controller.playCameraSound().await()

            controller.playScreenshotSound()

            advanceUntilIdle()



            verify(mediaPlayer).start()

        }



    @Test

    fun playCameraSound_illegalStateException_doesNotThrow() = runTest {

    fun playCameraSound_illegalStateException_doesNotThrow() =

        scope.runTest {

            whenever(mediaPlayer.start()).thenThrow(IllegalStateException())



            val controller = createController()

        controller.playCameraSound().await()

            controller.playScreenshotSound()

            advanceUntilIdle()



            verify(mediaPlayer).start()

            verify(mediaPlayer).release()

        }



    @Test

    fun playCameraSound_soundLoadingSuccessful_mediaPlayerReleases() = runTest {

    fun playCameraSound_soundLoadingSuccessful_mediaPlayerReleases() =

        scope.runTest {

            val controller = createController()



        controller.releaseScreenshotSound().await()

            controller.releaseScreenshotSound()

            advanceUntilIdle()



            verify(mediaPlayer).release()

        }