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

Commit 727682d0 authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Make MainThreadUnfoldTransitionProgressProvider callback registration thread safe 

This fixes a potential crash from UnfoldTransitionRepositoryImpl. When the transitionStatus flow was being collected from the background, it could have called the addCallback method from that bg thread. The "assertMainThread" is not needed anymore now.

Flag: ACONFIG unfold_animation_background_progress TRUNKFOOD
Test: MainThreadUnfoldTransitionProgressProviderTest
Fixes: 318958512
Change-Id: I41cb9ec5612a931b804f703993c324a2a2512575
parent a19a33f1

packages/SystemUI/tests/src/com/android/systemui/unfold/progress/MainThreadUnfoldTransitionProgressProviderTest.kt

+12 −0
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package com.android.systemui.unfold.progress



import android.os.Handler

import android.os.HandlerThread

import android.os.Looper

import android.testing.AndroidTestingRunner

import android.testing.TestableLooper.RunWithLooper
@@ -24,6 +26,7 @@ import com.android.systemui.SysuiTestCase 
import com.android.systemui.unfold.TestUnfoldTransitionProvider

import com.android.systemui.utils.os.FakeHandler

import kotlin.test.Test

import kotlinx.coroutines.test.runTest

import org.junit.runner.RunWith



@RunWith(AndroidTestingRunner::class)
@@ -93,4 +96,13 @@ class MainThreadUnfoldTransitionProgressProviderTest : SysuiTestCase() { 


        listener.assertNotStarted()

    }



    @Test

    fun addCallback_fromBackgroundThread_succeeds() = runTest {

        val bgHandler = Handler(HandlerThread("TestBgThread").apply { start() }.looper)

        bgHandler.runWithScissors({ progressProvider.addCallback(listener) }, 5000L)



        wrappedProgressProvider.onTransitionStarted()

        listener.assertStarted()

    }

}

packages/SystemUI/unfold/src/com/android/systemui/unfold/progress/MainThreadUnfoldTransitionProgressProvider.kt

+6 −9
Original line number Diff line number Diff line
@@ -24,12 +24,16 @@ import com.android.systemui.unfold.dagger.UnfoldMain 
import dagger.assisted.Assisted

import dagger.assisted.AssistedFactory

import dagger.assisted.AssistedInject

import java.util.Collections.synchronizedMap



/**

 * [UnfoldTransitionProgressProvider] that forwards all progress to the main thread handler.

 *

 * This is needed when progress are calculated in the background, but some listeners need the

 * callbacks in the main thread.

 *

 * Note that this class assumes that the root provider has thread safe callback registration, as

 * they might be called from any thread.

 */

class MainThreadUnfoldTransitionProgressProvider

@AssistedInject
@@ -38,27 +42,20 @@ constructor( 
    @Assisted private val rootProvider: UnfoldTransitionProgressProvider

) : UnfoldTransitionProgressProvider {



    private val listenerMap = mutableMapOf<TransitionProgressListener, TransitionProgressListener>()

    private val listenerMap: MutableMap<TransitionProgressListener, TransitionProgressListener> =

        synchronizedMap(mutableMapOf())



    override fun addCallback(listener: TransitionProgressListener) {

        assertMainThread()

        val proxy = TransitionProgressListerProxy(listener)

        rootProvider.addCallback(proxy)

        listenerMap[listener] = proxy

    }



    override fun removeCallback(listener: TransitionProgressListener) {

        assertMainThread()

        val proxy = listenerMap.remove(listener) ?: return

        rootProvider.removeCallback(proxy)

    }



    private fun assertMainThread() {

        check(mainHandler.looper.isCurrentThread) {

            "Should be called from the main thread, but this is ${Thread.currentThread()}"

        }

    }



    override fun destroy() {

        rootProvider.destroy()

    }