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

Commit 26fd9a0f authored by Matt Pietal's avatar Matt Pietal
Browse files

Controls UI - Better structure/app switching

Implement one subscriber per subscription request. Follow publishers
specs for cancellation/completion. Make sure subscriptions get cleaned
up properly in all cases.

Bug: 151145089
Test: atest ControlsControllerImplTest ControlsBindingControllerImplTest ControlsProviderLifecycleManagerTest
Change-Id: I558a7cbcf34c12dab686a65c5d269408c4f8b6f7
parent 6e2d2206
Loading
Loading
Loading
Loading
+31 −86
Original line number Diff line number Diff line
@@ -31,7 +31,6 @@ import com.android.internal.annotations.VisibleForTesting
import com.android.systemui.dagger.qualifiers.Background
import com.android.systemui.util.concurrency.DelayableExecutor
import dagger.Lazy
import java.util.concurrent.atomic.AtomicBoolean
import javax.inject.Inject
import javax.inject.Singleton

@@ -47,8 +46,6 @@ open class ControlsBindingControllerImpl @Inject constructor(
        private const val TAG = "ControlsBindingControllerImpl"
    }

    private val refreshing = AtomicBoolean(false)

    private var currentUser = UserHandle.of(ActivityManager.getCurrentUser())

    override val currentUserId: Int
@@ -56,6 +53,12 @@ open class ControlsBindingControllerImpl @Inject constructor(

    private var currentProvider: ControlsProviderLifecycleManager? = null

    /*
     * Will track any active subscriber for subscribe/unsubscribe requests coming into
     * this controller. Only one can be active at any time
     */
    private var statefulControlSubscriber: StatefulControlSubscriber? = null

    private val actionCallbackService = object : IControlsActionCallback.Stub() {
        override fun accept(
            token: IBinder,
@@ -66,27 +69,6 @@ open class ControlsBindingControllerImpl @Inject constructor(
        }
    }

    private val subscriberService = object : IControlsSubscriber.Stub() {
        override fun onSubscribe(token: IBinder, subs: IControlsSubscription) {
            backgroundExecutor.execute(OnSubscribeRunnable(token, subs))
        }

        override fun onNext(token: IBinder, c: Control) {
            if (!refreshing.get()) {
                Log.d(TAG, "Refresh outside of window for token:$token")
            } else {
                backgroundExecutor.execute(OnNextRunnable(token, c))
            }
        }
        override fun onError(token: IBinder, s: String) {
            backgroundExecutor.execute(OnErrorRunnable(token, s))
        }

        override fun onComplete(token: IBinder) {
            backgroundExecutor.execute(OnCompleteRunnable(token))
        }
    }

    @VisibleForTesting
    internal open fun createProviderManager(component: ComponentName):
            ControlsProviderLifecycleManager {
@@ -94,23 +76,21 @@ open class ControlsBindingControllerImpl @Inject constructor(
                context,
                backgroundExecutor,
                actionCallbackService,
                subscriberService,
                currentUser,
                component
        )
    }

    private fun retrieveLifecycleManager(component: ComponentName):
            ControlsProviderLifecycleManager? {
            ControlsProviderLifecycleManager {
        if (currentProvider != null && currentProvider?.componentName != component) {
            unbind()
        }

        if (currentProvider == null) {
            currentProvider = createProviderManager(component)
        }
        val provider = currentProvider ?: createProviderManager(component)
        currentProvider = provider

        return currentProvider
        return provider
    }

    override fun bindAndLoad(
@@ -118,21 +98,23 @@ open class ControlsBindingControllerImpl @Inject constructor(
        callback: ControlsBindingController.LoadCallback
    ): Runnable {
        val subscriber = LoadSubscriber(callback)
        retrieveLifecycleManager(component)?.maybeBindAndLoad(subscriber)
        retrieveLifecycleManager(component).maybeBindAndLoad(subscriber)
        return subscriber.loadCancel()
    }

    override fun subscribe(structureInfo: StructureInfo) {
        if (refreshing.compareAndSet(false, true)) {
        // make sure this has happened. only allow one active subscription
        unsubscribe()

        statefulControlSubscriber = null
        val provider = retrieveLifecycleManager(structureInfo.componentName)
            provider?.maybeBindAndSubscribe(structureInfo.controls.map { it.controlId })
        }
        val scs = StatefulControlSubscriber(lazyController.get(), provider, backgroundExecutor)
        statefulControlSubscriber = scs
        provider.maybeBindAndSubscribe(structureInfo.controls.map { it.controlId }, scs)
    }

    override fun unsubscribe() {
        if (refreshing.compareAndSet(true, false)) {
            currentProvider?.unsubscribe()
        }
        statefulControlSubscriber?.cancel()
    }

    override fun action(
@@ -140,20 +122,24 @@ open class ControlsBindingControllerImpl @Inject constructor(
        controlInfo: ControlInfo,
        action: ControlAction
    ) {
        if (statefulControlSubscriber == null) {
            Log.w(TAG, "No actions can occur outside of an active subscription. Ignoring.")
        } else {
            retrieveLifecycleManager(componentName)
            ?.maybeBindAndSendAction(controlInfo.controlId, action)
                .maybeBindAndSendAction(controlInfo.controlId, action)
        }
    }

    override fun bindService(component: ComponentName) {
        retrieveLifecycleManager(component)?.bindService()
        retrieveLifecycleManager(component).bindService()
    }

    override fun changeUser(newUser: UserHandle) {
        if (newUser == currentUser) return

        unsubscribe()
        unbind()

        refreshing.set(false)
        currentProvider = null
        currentUser = newUser
    }

@@ -174,8 +160,8 @@ open class ControlsBindingControllerImpl @Inject constructor(

    override fun toString(): String {
        return StringBuilder("  ControlsBindingController:\n").apply {
            append("    refreshing=${refreshing.get()}\n")
            append("    currentUser=$currentUser\n")
            append("    StatefulControlSubscriber=$statefulControlSubscriber")
            append("    Providers=$currentProvider\n")
        }.toString()
    }
@@ -213,53 +199,12 @@ open class ControlsBindingControllerImpl @Inject constructor(
        }
    }

    private inner class OnNextRunnable(
        token: IBinder,
        val control: Control
    ) : CallbackRunnable(token) {
        override fun doRun() {
            if (!refreshing.get()) {
                Log.d(TAG, "onRefresh outside of window from:${provider?.componentName}")
            }

            provider?.let {
                lazyController.get().refreshStatus(it.componentName, control)
            }
        }
    }

    private inner class OnSubscribeRunnable(
        token: IBinder,
        val subscription: IControlsSubscription
    ) : CallbackRunnable(token) {
        override fun doRun() {
            if (!refreshing.get()) {
                Log.d(TAG, "onRefresh outside of window from '${provider?.componentName}'")
            }
            provider?.let {
                it.startSubscription(subscription)
            }
        }
    }

    private inner class OnCompleteRunnable(
        token: IBinder
    ) : CallbackRunnable(token) {
        override fun doRun() {
            provider?.let {
                Log.i(TAG, "onComplete receive from '${it.componentName}'")
            }
        }
    }

    private inner class OnErrorRunnable(
        token: IBinder,
        val error: String
    ) : CallbackRunnable(token) {
        override fun doRun() {
            provider?.let {
                Log.e(TAG, "onError receive from '${it.componentName}': $error")
            }
            provider?.startSubscription(subscription)
        }
    }

+40 −31
Original line number Diff line number Diff line
@@ -58,7 +58,6 @@ class ControlsProviderLifecycleManager(
    private val context: Context,
    private val executor: DelayableExecutor,
    private val actionCallbackService: IControlsActionCallback.Stub,
    private val subscriberService: IControlsSubscriber.Stub,
    val user: UserHandle,
    val componentName: ComponentName
) : IBinder.DeathRecipient {
@@ -157,10 +156,9 @@ class ControlsProviderLifecycleManager(
            load(msg.subscriber)
        }

        queue.filter { it is Message.Subscribe }.flatMap { (it as Message.Subscribe).list }.run {
            if (this.isNotEmpty()) {
                subscribe(this)
            }
        queue.filter { it is Message.Subscribe }.forEach {
            val msg = it as Message.Subscribe
            subscribe(msg.list, msg.subscriber)
        }
        queue.filter { it is Message.Action }.forEach {
            val msg = it as Message.Action
@@ -185,9 +183,9 @@ class ControlsProviderLifecycleManager(
        }
    }

    private fun unqueueMessage(message: Message) {
    private fun unqueueMessageType(type: Int) {
        synchronized(queuedMessages) {
            queuedMessages.removeIf { it.type == message.type }
            queuedMessages.removeIf { it.type == type }
        }
    }

@@ -219,7 +217,7 @@ class ControlsProviderLifecycleManager(
     * @param subscriber the subscriber that manages coordination for loading controls
     */
    fun maybeBindAndLoad(subscriber: IControlsSubscriber.Stub) {
        unqueueMessage(Message.Unbind)
        unqueueMessageType(MSG_UNBIND)
        onLoadCanceller = executor.executeDelayed({
            // Didn't receive a response in time, log and send back error
            Log.d(TAG, "Timeout waiting onLoad for $componentName")
@@ -237,16 +235,20 @@ class ControlsProviderLifecycleManager(
     *
     * @param controlIds a list of the ids of controls to send status back.
     */
    fun maybeBindAndSubscribe(controlIds: List<String>) {
        invokeOrQueue({ subscribe(controlIds) }, Message.Subscribe(controlIds))
    fun maybeBindAndSubscribe(controlIds: List<String>, subscriber: IControlsSubscriber) {
        invokeOrQueue(
            { subscribe(controlIds, subscriber) },
            Message.Subscribe(controlIds, subscriber)
        )
    }

    private fun subscribe(controlIds: List<String>) {
    private fun subscribe(controlIds: List<String>, subscriber: IControlsSubscriber) {
        if (DEBUG) {
            Log.d(TAG, "subscribe $componentName - $controlIds")
        }
        if (!(wrapper?.subscribe(controlIds, subscriberService) ?: false)) {
            queueMessage(Message.Subscribe(controlIds))

        if (!(wrapper?.subscribe(controlIds, subscriber) ?: false)) {
            queueMessage(Message.Subscribe(controlIds, subscriber))
            binderDied()
        }
    }
@@ -276,10 +278,13 @@ class ControlsProviderLifecycleManager(
    /**
     * Starts the subscription to the [ControlsProviderService] and requests status of controls.
     *
     * @param subscription the subscriber to use to request controls
     * @param subscription the subscription to use to request controls
     * @see maybeBindAndLoad
     */
    fun startSubscription(subscription: IControlsSubscription) {
        if (DEBUG) {
            Log.d(TAG, "startSubscription: $subscription")
        }
        synchronized(subscriptions) {
            subscriptions.add(subscription)
        }
@@ -287,30 +292,26 @@ class ControlsProviderLifecycleManager(
    }

    /**
     * Unsubscribe from this service, cancelling all status requests.
     * Cancels the subscription to the [ControlsProviderService].
     *
     * @param subscription the subscription to cancel
     * @see maybeBindAndLoad
     */
    fun unsubscribe() {
    fun cancelSubscription(subscription: IControlsSubscription) {
        if (DEBUG) {
            Log.d(TAG, "unsubscribe $componentName")
            Log.d(TAG, "cancelSubscription: $subscription")
        }
        unqueueMessage(Message.Subscribe(emptyList())) // Removes all subscribe messages

        val subs = synchronized(subscriptions) {
            ArrayList(subscriptions).also {
                subscriptions.clear()
            }
        }

        subs.forEach {
            wrapper?.cancel(it)
        synchronized(subscriptions) {
            subscriptions.remove(subscription)
        }
        wrapper?.cancel(subscription)
    }

    /**
     * Request bind to the service.
     */
    fun bindService() {
        unqueueMessage(Message.Unbind)
        unqueueMessageType(MSG_UNBIND)
        bindService(true)
    }

@@ -321,8 +322,16 @@ class ControlsProviderLifecycleManager(
        onLoadCanceller?.run()
        onLoadCanceller = null

        // just in case this wasn't called already
        unsubscribe()
        // be sure to cancel all subscriptions
        val subs = synchronized(subscriptions) {
            ArrayList(subscriptions).also {
                subscriptions.clear()
            }
        }

        subs.forEach {
            wrapper?.cancel(it)
        }

        bindService(false)
    }
@@ -346,7 +355,7 @@ class ControlsProviderLifecycleManager(
        object Unbind : Message() {
            override val type = MSG_UNBIND
        }
        class Subscribe(val list: List<String>) : Message() {
        class Subscribe(val list: List<String>, val subscriber: IControlsSubscriber) : Message() {
            override val type = MSG_SUBSCRIBE
        }
        class Action(val id: String, val action: ControlAction) : Message() {
+96 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2020 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.systemui.controls.controller

import android.os.IBinder
import android.service.controls.Control
import android.service.controls.IControlsSubscriber
import android.service.controls.IControlsSubscription
import android.util.Log
import com.android.systemui.util.concurrency.DelayableExecutor

/**
 * A single subscriber, supporting stateful controls for publishers created by
 * {@link ControlsProviderService#createPublisherFor}. In general, this subscription will remain
 * active until the SysUi chooses to cancel it.
 */
class StatefulControlSubscriber(
    private val controller: ControlsController,
    private val provider: ControlsProviderLifecycleManager,
    private val bgExecutor: DelayableExecutor
) : IControlsSubscriber.Stub() {
    private var subscriptionOpen = false
    private var subscription: IControlsSubscription? = null

    companion object {
        private const val TAG = "StatefulControlSubscriber"
    }

    private fun run(token: IBinder, f: () -> Unit) {
        if (provider.token == token) {
            bgExecutor.execute { f() }
        }
    }

    override fun onSubscribe(token: IBinder, subs: IControlsSubscription) {
        run(token) {
            subscriptionOpen = true
            subscription = subs
            provider.startSubscription(subs)
        }
    }

    override fun onNext(token: IBinder, control: Control) {
        run(token) {
            if (!subscriptionOpen) {
                Log.w(TAG, "Refresh outside of window for token:$token")
            } else {
                controller.refreshStatus(provider.componentName, control)
            }
        }
    }
    override fun onError(token: IBinder, error: String) {
        run(token) {
            if (subscriptionOpen) {
                subscriptionOpen = false
                Log.e(TAG, "onError receive from '${provider.componentName}': $error")
            }
        }
    }

    override fun onComplete(token: IBinder) {
        run(token) {
            if (subscriptionOpen) {
                subscriptionOpen = false
                Log.i(TAG, "onComplete receive from '${provider.componentName}'")
            }
        }
    }

    fun cancel() {
        if (!subscriptionOpen) return
        bgExecutor.execute {
            if (subscriptionOpen) {
                subscriptionOpen = false
                subscription?.let {
                    provider.cancelSubscription(it)
                }
                subscription = null
            }
        }
    }
}
+34 −13
Original line number Diff line number Diff line
@@ -52,8 +52,8 @@ import org.mockito.MockitoAnnotations
class ControlsBindingControllerImplTest : SysuiTestCase() {

    companion object {
        fun <T> any(): T = Mockito.any<T>()
        fun <T> capture(argumentCaptor: ArgumentCaptor<T>): T = argumentCaptor.capture()
        fun <T> any(): T = Mockito.any<T>()
        private val TEST_COMPONENT_NAME_1 = ComponentName("TEST_PKG", "TEST_CLS_1")
        private val TEST_COMPONENT_NAME_2 = ComponentName("TEST_PKG", "TEST_CLS_2")
        private val TEST_COMPONENT_NAME_3 = ComponentName("TEST_PKG", "TEST_CLS_3")
@@ -61,8 +61,15 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {

    @Mock
    private lateinit var mockControlsController: ControlsController

    @Captor
    private lateinit var subscriberCaptor: ArgumentCaptor<IControlsSubscriber>

    @Captor
    private lateinit var loadSubscriberCaptor: ArgumentCaptor<IControlsSubscriber.Stub>

    @Captor
    private lateinit var subscriberCaptor: ArgumentCaptor<IControlsSubscriber.Stub>
    private lateinit var listStringCaptor: ArgumentCaptor<List<String>>

    private val user = UserHandle.of(mContext.userId)
    private val otherUser = UserHandle.of(user.identifier + 1)
@@ -114,8 +121,8 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {

        val canceller = controller.bindAndLoad(TEST_COMPONENT_NAME_1, callback)

        verify(providers[0]).maybeBindAndLoad(capture(subscriberCaptor))
        subscriberCaptor.value.onSubscribe(Binder(), subscription)
        verify(providers[0]).maybeBindAndLoad(capture(loadSubscriberCaptor))
        loadSubscriberCaptor.value.onSubscribe(Binder(), subscription)

        canceller.run()
        verify(subscription).cancel()
@@ -132,11 +139,11 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {

        val canceller = controller.bindAndLoad(TEST_COMPONENT_NAME_1, callback)

        verify(providers[0]).maybeBindAndLoad(capture(subscriberCaptor))
        verify(providers[0]).maybeBindAndLoad(capture(loadSubscriberCaptor))
        val b = Binder()
        subscriberCaptor.value.onSubscribe(b, subscription)
        loadSubscriberCaptor.value.onSubscribe(b, subscription)

        subscriberCaptor.value.onComplete(b)
        loadSubscriberCaptor.value.onComplete(b)
        canceller.run()
        verify(subscription, never()).cancel()
    }
@@ -152,11 +159,11 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {

        val canceller = controller.bindAndLoad(TEST_COMPONENT_NAME_1, callback)

        verify(providers[0]).maybeBindAndLoad(capture(subscriberCaptor))
        verify(providers[0]).maybeBindAndLoad(capture(loadSubscriberCaptor))
        val b = Binder()
        subscriberCaptor.value.onSubscribe(b, subscription)
        loadSubscriberCaptor.value.onSubscribe(b, subscription)

        subscriberCaptor.value.onError(b, "")
        loadSubscriberCaptor.value.onError(b, "")
        canceller.run()
        verify(subscription, never()).cancel()
    }
@@ -180,8 +187,13 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {

        executor.runAllReady()

        val subs = mock(IControlsSubscription::class.java)
        verify(providers[0]).maybeBindAndSubscribe(
            capture(listStringCaptor), capture(subscriberCaptor))
        assertEquals(listStringCaptor.value,
            listOf(controlInfo1.controlId, controlInfo2.controlId))

        subscriberCaptor.value.onSubscribe(providers[0].token, subs)
    }

    @Test
@@ -191,7 +203,7 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {

        executor.runAllReady()

        verify(providers[0], never()).unsubscribe()
        verify(providers[0], never()).cancelSubscription(any())
    }

    @Test
@@ -202,12 +214,21 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {
            StructureInfo(TEST_COMPONENT_NAME_1, "Home", listOf(controlInfo1, controlInfo2))

        controller.subscribe(structure)
        executor.runAllReady()

        controller.unsubscribe()
        val subs = mock(IControlsSubscription::class.java)
        verify(providers[0]).maybeBindAndSubscribe(
            capture(listStringCaptor), capture(subscriberCaptor))
        assertEquals(listStringCaptor.value,
            listOf(controlInfo1.controlId, controlInfo2.controlId))

        subscriberCaptor.value.onSubscribe(providers[0].token, subs)
        executor.runAllReady()

        controller.unsubscribe()
        executor.runAllReady()

        verify(providers[0]).unsubscribe()
        verify(providers[0]).cancelSubscription(subs)
    }

    @Test
+2 −3
Original line number Diff line number Diff line
/*
 * Copyright (C) 2020 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * Licensed under the Apache License, Version 2.0 (149the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
@@ -83,7 +83,6 @@ class ControlsProviderLifecycleManagerTest : SysuiTestCase() {
                context,
                executor,
                actionCallbackService,
                subscriberService,
                UserHandle.of(0),
                componentName
        )
@@ -146,7 +145,7 @@ class ControlsProviderLifecycleManagerTest : SysuiTestCase() {
    @Test
    fun testMaybeBindAndSubscribe() {
        val list = listOf("TEST_ID")
        manager.maybeBindAndSubscribe(list)
        manager.maybeBindAndSubscribe(list, subscriberService)
        executor.runAllReady()

        assertTrue(mContext.isBound(componentName))
Loading