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

Commit 815600d8 authored by Kshitij Gupta's avatar Kshitij Gupta
Browse files

Media3ActionFactory: Drop usages of runBlocking

- This CL refactors Media3ActionFactory to remove blocking runBlocking
  calls, leveraging asynchronous coroutines for Media3 operations.
- Previous use of runBlocking in getMedia3Actions for Media3 controller
  interactions could block the calling thread, potentially causing UI
  jank or ANRs.
- getMedia3Actions is now a suspend fun, allowing its callers to suspend
  instead of blocking. Custom actions are fetched and mapped
  asynchronously
- The suspendCancellableCoroutine block for createMediaAction now
  directly launches a coroutine on bgScope, removing reliance on
  Handler.post for dispatching controller calls, for structured
  concurrency and cancellation.

Bug: 423462317
Flag: com.android.systemui.do_not_use_run_blocking
Test: atest Media3ActionFactoryTest
Change-Id: Ie9adcb5cd8caaaf0a42c7400e890f5bfcd26dc4c
parent 8a9ab13d
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -24222,7 +24222,7 @@
        errorLine2="~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
        <location
            file="frameworks/base/packages/SystemUI/src/com/android/systemui/media/controls/domain/pipeline/Media3ActionFactory.kt"
            line="34"
            line="33"
            column="1"/>
    </issue>
+19 −3
Original line number Diff line number Diff line
@@ -20,6 +20,8 @@ import android.media.session.MediaSession
import android.os.Bundle
import android.os.Handler
import android.os.looper
import android.platform.test.flag.junit.FlagsParameterization
import android.platform.test.flag.junit.FlagsParameterization.allCombinationsOf
import android.testing.TestableLooper.RunWithLooper
import androidx.media.utils.MediaConstants
import androidx.media3.common.Player
@@ -28,8 +30,8 @@ import androidx.media3.session.MediaController as Media3Controller
import androidx.media3.session.SessionCommand
import androidx.media3.session.SessionResult
import androidx.media3.session.SessionToken
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.Flags.FLAG_DO_NOT_USE_RUN_BLOCKING
import com.android.systemui.SysuiTestCase
import com.android.systemui.graphics.imageLoader
import com.android.systemui.kosmos.testScope
@@ -57,6 +59,8 @@ import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import platform.test.runner.parameterized.ParameterizedAndroidJunit4
import platform.test.runner.parameterized.Parameters

private const val PACKAGE_NAME = "package_name"
private const val CUSTOM_ACTION_NAME = "Custom Action"
@@ -64,8 +68,8 @@ private const val CUSTOM_ACTION_COMMAND = "custom-action"

@SmallTest
@RunWithLooper
@RunWith(AndroidJUnit4::class)
class Media3ActionFactoryTest : SysuiTestCase() {
@RunWith(ParameterizedAndroidJunit4::class)
class Media3ActionFactoryTest(flags: FlagsParameterization) : SysuiTestCase() {

    private val kosmos = testKosmos()
    private val testScope = kosmos.testScope
@@ -98,6 +102,18 @@ class Media3ActionFactoryTest : SysuiTestCase() {

    private lateinit var underTest: Media3ActionFactory

    companion object {
        @JvmStatic
        @Parameters(name = "{0}")
        fun getParams(): List<FlagsParameterization> {
            return allCombinationsOf(FLAG_DO_NOT_USE_RUN_BLOCKING)
        }
    }

    init {
        mSetFlagsRule.setFlagsParameterization(flags)
    }

    @Before
    fun setup() {
        underTest =
+105 −23
Original line number Diff line number Diff line
@@ -28,7 +28,6 @@ import androidx.annotation.WorkerThread
import androidx.media.utils.MediaConstants
import androidx.media3.common.Player
import androidx.media3.session.CommandButton
import androidx.media3.session.MediaController as Media3Controller
import androidx.media3.session.SessionCommand
import androidx.media3.session.SessionToken
import com.android.app.tracing.coroutines.runBlockingTraced as runBlocking
@@ -45,11 +44,14 @@ import com.android.systemui.media.controls.util.MediaControllerFactory
import com.android.systemui.media.controls.util.SessionTokenFactory
import com.android.systemui.res.R
import com.android.systemui.util.concurrency.Execution
import java.util.concurrent.ExecutionException
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.suspendCancellableCoroutine
import java.util.concurrent.ExecutionException
import javax.inject.Inject
import kotlin.coroutines.resume
import kotlin.coroutines.resumeWithException
import androidx.media3.session.MediaController as Media3Controller

private const val TAG = "Media3ActionFactory"

@@ -89,21 +91,22 @@ constructor(

        // Build button info
        val buttons = suspendCancellableCoroutine { continuation ->
            // Media3Controller methods must always be called from a specific looper
            val runnable = Runnable {
            val job = bgScope.launch {
                try {
                    val result = getMedia3Actions(packageName, m3controller, token)
                    continuation.resumeWith(Result.success(result))
                    continuation.resume(result)
                } catch (e: Exception) {
                    continuation.resumeWithException(e)
                } finally {
                    m3controller.tryRelease(packageName, logger)
                }
            }
            handler.post(runnable)
            continuation.invokeOnCancellation {
                job.cancel()
                // Ensure controller is released, even if loading was cancelled partway through
                val releaseRunnable = Runnable { m3controller.tryRelease(packageName, logger) }
                handler.post(releaseRunnable)
                handler.removeCallbacks(runnable)
                bgScope.launch {
                    m3controller.tryRelease(packageName, logger)
                }
            }
        }
        return buttons
@@ -111,7 +114,7 @@ constructor(

    /** This method must be called on the Media3 looper! */
    @WorkerThread
    private fun getMedia3Actions(
    private suspend fun getMedia3Actions(
        packageName: String,
        m3controller: Media3Controller,
        token: SessionToken,
@@ -159,16 +162,12 @@ constructor(
            )

        // Then, get custom actions
        var customActions =
            m3controller.customLayout
                .asSequence()
                .filter {
                    it.isEnabled &&
                        it.sessionCommand?.commandCode == SessionCommand.COMMAND_CODE_CUSTOM &&
                        m3controller.isSessionCommandAvailable(it.sessionCommand!!)
        val customActions = if (Flags.doNotUseRunBlocking()) {
            createCustomActionsIterator(m3controller, packageName, token)
        } else {
            createCustomActionsIteratorBlocking(m3controller, packageName, token)
        }
                .map { getCustomAction(packageName, token, it) }
                .iterator()

        fun nextCustomAction() = if (customActions.hasNext()) customActions.next() else null

        // Finally, assign the remaining button slots: play/pause A B C D
@@ -213,6 +212,51 @@ constructor(
        )
    }

    /**
     * Creates an [Iterator] of [MediaAction]s from the controller's custom layout.
     * Each [MediaAction] represents a [CommandButton]
     */
    private suspend fun createCustomActionsIterator(
        m3controller: androidx.media3.session.MediaController,
        packageName: String,
        token: SessionToken
    ): Iterator<MediaAction> {
        return m3controller.customLayout
            .asSequence()
            .filter {
                it.isEnabled &&
                        it.sessionCommand?.commandCode == SessionCommand.COMMAND_CODE_CUSTOM &&
                        m3controller.isSessionCommandAvailable(it.sessionCommand!!)
            }
            .toList()
            .map { button -> getCustomAction(packageName, token, button) }
            .iterator()
    }

    /**
     * Creates an [Iterator] of [MediaAction]s from the controller's custom layout.
     * Each [MediaAction] represents a [CommandButton]
     */
    @Deprecated(
        message = "Avoid using runBlocking in production code. Consider asynchronous alternatives.",
        replaceWith = ReplaceWith("createCustomActionsIterator()")
    )
    private fun createCustomActionsIteratorBlocking(
        m3controller: androidx.media3.session.MediaController,
        packageName: String,
        token: SessionToken
    ): Iterator<MediaAction> {
        return m3controller.customLayout
            .asSequence()
            .filter {
                it.isEnabled &&
                        it.sessionCommand?.commandCode == SessionCommand.COMMAND_CODE_CUSTOM &&
                        m3controller.isSessionCommandAvailable(it.sessionCommand!!)
            }
            .map { getCustomActionBlocking(packageName, token, it) }
            .iterator()
    }

    /**
     * Create a [MediaAction] for a given command, if supported
     *
@@ -279,7 +323,7 @@ constructor(
    }

    /** Get a [MediaAction] representing a [CommandButton] */
    private fun getCustomAction(
    private suspend fun getCustomAction(
        packageName: String,
        token: SessionToken,
        customAction: CommandButton,
@@ -292,6 +336,24 @@ constructor(
        )
    }

    /** Get a [MediaAction] representing a [CommandButton] */
    @Deprecated(
        message = "Avoid using runBlocking in production code. Consider asynchronous alternatives.",
        replaceWith = ReplaceWith("getCustomAction()")
    )
    private fun getCustomActionBlocking(
        packageName: String,
        token: SessionToken,
        customAction: CommandButton,
    ): MediaAction {
        return MediaAction(
            getIconForActionBlocking(customAction, packageName),
            { executeAction(packageName, token, Player.COMMAND_INVALID, customAction) },
            customAction.displayName,
            null,
        )
    }

    private fun getIconForAction(command: @Player.Command Int): Drawable? {
        return when (command) {
            Player.COMMAND_SEEK_TO_PREVIOUS -> MediaControlDrawables.getPrevIcon(context)
@@ -305,7 +367,27 @@ constructor(
        }
    }

    private fun getIconForAction(customAction: CommandButton, packageName: String): Drawable? {
    private suspend fun getIconForAction(customAction: CommandButton, packageName: String): Drawable? {
        val size = context.resources.getDimensionPixelSize(R.dimen.min_clickable_item_size)
        // TODO(b/360196209): check customAction.icon field to use platform icons
        if (customAction.iconResId != 0) {
            val packageContext = context.createPackageContext(packageName, 0)
            val source = ImageLoader.Res(customAction.iconResId, packageContext)
            return imageLoader.loadDrawable(source, size, size)
        }

        if (customAction.iconUri != null) {
            val source = ImageLoader.Uri(customAction.iconUri!!)
            return imageLoader.loadDrawable(source, size, size)
        }
        return null
    }

    @Deprecated(
        message = "Avoid using runBlocking in production code. Consider asynchronous alternatives.",
        replaceWith = ReplaceWith("getIconForAction()")
    )
    private fun getIconForActionBlocking(customAction: CommandButton, packageName: String): Drawable? {
        val size = context.resources.getDimensionPixelSize(R.dimen.min_clickable_item_size)
        // TODO(b/360196209): check customAction.icon field to use platform icons
        if (customAction.iconResId != 0) {