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

Commit 8ef954df authored by Nicolo' Mazzucato's avatar Nicolo' Mazzucato
Browse files

Directly propagate progress if setReadyToHandleTransition is called from the correct thread

Before this change, setReadyToHandleTransition was always posting hte callback to propagate progress to the progressHandler. This caused some reordering of callbacks that led to some transition progress to be received out of order.

Now if setReadyToHandle transition is called from the correct thread (the same as progressHandler), the posting is skipped.

Flag: None
Bug: 311084698
Test: ScopedUnfoldTransitionProgressProviderTest
Change-Id: Ib1c596838b7582cef0cee4b4363f74edc77f8c0e
parent 8087ef78
Loading
Loading
Loading
Loading
+1 −4
Original line number Diff line number Diff line
@@ -16,8 +16,6 @@

package com.android.wm.shell.unfold;

import static android.app.WindowConfiguration.WINDOWING_MODE_PINNED;

import android.annotation.NonNull;
import android.app.ActivityManager.RunningTaskInfo;
import android.app.TaskInfo;
@@ -230,8 +228,7 @@ public class UnfoldAnimationController implements UnfoldListener {
    }

    private void maybeResetTask(UnfoldTaskAnimator animator, TaskInfo taskInfo) {
        // TODO(b/311084698): the windowing mode check is added here as a work around.
        if (!mIsInStageChange || taskInfo.getWindowingMode() == WINDOWING_MODE_PINNED) {
        if (!mIsInStageChange) {
            // No need to resetTask if there is no ongoing state change.
            return;
        }
+24 −0
Original line number Diff line number Diff line
@@ -87,6 +87,30 @@ class ScopedUnfoldTransitionProgressProviderTest : SysuiTestCase() {
        }
    }

    @Test
    fun setReadyToHandleTransition_whileTransitionRunning_fromBgThread_propagatesCallbacks() =
        testScope.runTest {
            runBlockingInBg { rootProvider.onTransitionStarted() }

            runBlockingInBg {
                // This causes the transition started callback to be propagated immediately, without
                // the need to switch thread (as we're already in the correct one). We don't need a
                // sync barrier on the bg thread as in
                // setReadyToHandleTransition_whileTransitionRunning_propagatesCallbacks here.
                scopedProvider.setReadyToHandleTransition(true)
            }

            listener.assertStarted()

            runBlockingInBg { rootProvider.onTransitionProgress(1f) }

            listener.assertLastProgress(1f)

            runBlockingInBg { rootProvider.onTransitionFinished() }

            listener.assertNotStarted()
        }

    @Test
    fun setReadyToHandleTransition_beforeAnyCallback_doesNotCrash() {
        testScope.runTest { scopedProvider.setReadyToHandleTransition(true) }
+62 −28
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ package com.android.systemui.unfold.util

import android.os.Handler
import android.os.Looper
import androidx.annotation.GuardedBy
import com.android.systemui.unfold.UnfoldTransitionProgressProvider
import com.android.systemui.unfold.UnfoldTransitionProgressProvider.TransitionProgressListener
import java.util.concurrent.CopyOnWriteArrayList
@@ -41,8 +42,12 @@ constructor(source: UnfoldTransitionProgressProvider? = null) :

    private val listeners = CopyOnWriteArrayList<TransitionProgressListener>()

    @Volatile private var isReadyToHandleTransition = false
    @Volatile private var isTransitionRunning = false
    private val lock = Object()

    @GuardedBy("lock") private var isReadyToHandleTransition = false
    // Accessed only from progress thread
    private var isTransitionRunning = false
    // Accessed only from progress thread
    private var lastTransitionProgress = PROGRESS_UNSET

    init {
@@ -72,11 +77,34 @@ constructor(source: UnfoldTransitionProgressProvider? = null) :
     * the transition progress events.
     *
     * Call it with readyToHandleTransition = false when listeners can't process the events.
     *
     * Note that this could be called by any thread.
     */
    fun setReadyToHandleTransition(isReadyToHandleTransition: Boolean) {
        val progressHandler = this.progressHandler
        if (isTransitionRunning && progressHandler != null) {
            progressHandler.post {
        synchronized(lock) {
            this.isReadyToHandleTransition = isReadyToHandleTransition
            val progressHandlerLocal = this.progressHandler
            if (progressHandlerLocal != null) {
                ensureInHandler(progressHandlerLocal) { reportLastProgressIfNeeded() }
            }
        }
    }

    /** Runs directly if called from the handler thread. Posts otherwise. */
    private fun ensureInHandler(handler: Handler, f: () -> Unit) {
        if (handler.looper.isCurrentThread) {
            f()
        } else {
            handler.post(f)
        }
    }

    private fun reportLastProgressIfNeeded() {
        assertInProgressThread()
        synchronized(lock) {
            if (!isTransitionRunning) {
                return
            }
            if (isReadyToHandleTransition) {
                listeners.forEach { it.onTransitionStarted() }
                if (lastTransitionProgress != PROGRESS_UNSET) {
@@ -88,8 +116,6 @@ constructor(source: UnfoldTransitionProgressProvider? = null) :
            }
        }
    }
        this.isReadyToHandleTransition = isReadyToHandleTransition
    }

    override fun addCallback(listener: TransitionProgressListener) {
        listeners += listener
@@ -106,35 +132,43 @@ constructor(source: UnfoldTransitionProgressProvider? = null) :

    override fun onTransitionStarted() {
        assertInProgressThread()
        synchronized(lock) {
            isTransitionRunning = true
            if (isReadyToHandleTransition) {
                listeners.forEach { it.onTransitionStarted() }
            }
        }
    }

    override fun onTransitionProgress(progress: Float) {
        assertInProgressThread()
        synchronized(lock) {
            if (isReadyToHandleTransition) {
                listeners.forEach { it.onTransitionProgress(progress) }
            }
            lastTransitionProgress = progress
        }
    }

    override fun onTransitionFinishing() {
        assertInProgressThread()
        synchronized(lock) {
            if (isReadyToHandleTransition) {
                listeners.forEach { it.onTransitionFinishing() }
            }
        }
    }

    override fun onTransitionFinished() {
        assertInProgressThread()
        synchronized(lock) {
            if (isReadyToHandleTransition) {
                listeners.forEach { it.onTransitionFinished() }
            }
            isTransitionRunning = false
            lastTransitionProgress = PROGRESS_UNSET
        }
    }

    private fun assertInProgressThread() {
        val cachedProgressHandler = progressHandler
@@ -151,7 +185,7 @@ constructor(source: UnfoldTransitionProgressProvider? = null) :
        }
    }

    companion object {
        private const val PROGRESS_UNSET = -1f
    private companion object {
        const val PROGRESS_UNSET = -1f
    }
}