Controls UI - Motion for activities

Transition back to global actions with animations. Delay
pager/recyclerview loading until later in the activity cycle for
smoother animations. Fix race conditions discovered while testing,
around canceling load subscriptions too early.

Bug: 154158092
Bug: 154509798
Test: atest ControlsControllerImplTest
Change-Id: I019c269fc6a54dce6e762cb710b0a254a58d9a2d
This commit is contained in:
Matt Pietal
2020-04-21 15:00:15 -04:00
parent 41eb96833f
commit d8a6ca68eb
8 changed files with 148 additions and 99 deletions

View File

@@ -31,6 +31,7 @@ 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
@@ -284,13 +285,19 @@ open class ControlsBindingControllerImpl @Inject constructor(
val requestLimit: Long
) : IControlsSubscriber.Stub() {
val loadedControls = ArrayList<Control>()
private var isTerminated = false
private var isTerminated = AtomicBoolean(false)
private var _loadCancelInternal: (() -> Unit)? = null
private lateinit var subscription: IControlsSubscription
/**
* Potentially cancel a subscriber. The subscriber may also have terminated, in which case
* the request is ignored.
*/
fun loadCancel() = Runnable {
Log.d(TAG, "Cancel load requested")
_loadCancelInternal?.invoke()
_loadCancelInternal?.let {
Log.d(TAG, "Canceling loadSubscribtion")
it.invoke()
}
}
override fun onSubscribe(token: IBinder, subs: IControlsSubscription) {
@@ -301,7 +308,7 @@ open class ControlsBindingControllerImpl @Inject constructor(
override fun onNext(token: IBinder, c: Control) {
backgroundExecutor.execute {
if (isTerminated) return@execute
if (isTerminated.get()) return@execute
loadedControls.add(c)
@@ -325,13 +332,15 @@ open class ControlsBindingControllerImpl @Inject constructor(
}
private fun maybeTerminateAndRun(postTerminateFn: Runnable) {
if (isTerminated) return
if (isTerminated.get()) return
isTerminated = true
_loadCancelInternal = {}
currentProvider?.cancelLoadTimeout()
backgroundExecutor.execute(postTerminateFn)
backgroundExecutor.execute {
isTerminated.compareAndSet(false, true)
postTerminateFn.run()
}
}
}
}

View File

@@ -52,18 +52,16 @@ interface ControlsController : UserAwareController {
* Load all available [Control] for a given service.
*
* @param componentName the [ComponentName] of the [ControlsProviderService] to load from
* @param dataCallback a callback in which to retrieve the result.
* @param dataCallback a callback in which to retrieve the result
* @param cancelWrapper a callback to receive a [Runnable] that can be run to cancel the
* request
*/
fun loadForComponent(
componentName: ComponentName,
dataCallback: Consumer<LoadData>
dataCallback: Consumer<LoadData>,
cancelWrapper: Consumer<Runnable>
)
/**
* Cancels a pending load call
*/
fun cancelLoad()
/**
* Request to subscribe for favorited controls per structure
*

View File

@@ -77,8 +77,6 @@ class ControlsControllerImpl @Inject constructor (
private var userChanging: Boolean = true
private var loadCanceller: Runnable? = null
private var seedingInProgress = false
private val seedingCallbacks = mutableListOf<Consumer<Boolean>>()
@@ -276,28 +274,29 @@ class ControlsControllerImpl @Inject constructor (
override fun loadForComponent(
componentName: ComponentName,
dataCallback: Consumer<ControlsController.LoadData>
dataCallback: Consumer<ControlsController.LoadData>,
cancelWrapper: Consumer<Runnable>
) {
if (!confirmAvailability()) {
if (userChanging) {
// Try again later, userChanging should not last forever. If so, we have bigger
// problems. This will return a runnable that allows to cancel the delayed version,
// it will not be able to cancel the load if
loadCanceller = executor.executeDelayed(
{ loadForComponent(componentName, dataCallback) },
USER_CHANGE_RETRY_DELAY,
TimeUnit.MILLISECONDS
executor.executeDelayed(
{ loadForComponent(componentName, dataCallback, cancelWrapper) },
USER_CHANGE_RETRY_DELAY,
TimeUnit.MILLISECONDS
)
} else {
dataCallback.accept(createLoadDataObject(emptyList(), emptyList(), true))
}
return
dataCallback.accept(createLoadDataObject(emptyList(), emptyList(), true))
}
loadCanceller = bindingController.bindAndLoad(
cancelWrapper.accept(
bindingController.bindAndLoad(
componentName,
object : ControlsBindingController.LoadCallback {
override fun accept(controls: List<Control>) {
loadCanceller = null
executor.execute {
val favoritesForComponentKeys = Favorites
.getControlsForComponent(componentName).map { it.controlId }
@@ -333,7 +332,6 @@ class ControlsControllerImpl @Inject constructor (
}
override fun error(message: String) {
loadCanceller = null
executor.execute {
val controls = Favorites.getStructuresForComponent(componentName)
.flatMap { st ->
@@ -348,6 +346,7 @@ class ControlsControllerImpl @Inject constructor (
}
}
}
)
)
}
@@ -432,12 +431,6 @@ class ControlsControllerImpl @Inject constructor (
seedingCallbacks.clear()
}
override fun cancelLoad() {
loadCanceller?.let {
executor.execute(it)
}
}
private fun createRemovedStatus(
componentName: ComponentName,
controlInfo: ControlInfo,

View File

@@ -32,6 +32,7 @@ import com.android.systemui.R
import com.android.systemui.broadcast.BroadcastDispatcher
import com.android.systemui.controls.controller.ControlsControllerImpl
import com.android.systemui.controls.controller.StructureInfo
import com.android.systemui.globalactions.GlobalActionsComponent
import com.android.systemui.settings.CurrentUserTracker
import com.android.systemui.util.LifecycleActivity
import javax.inject.Inject
@@ -41,7 +42,8 @@ import javax.inject.Inject
*/
class ControlsEditingActivity @Inject constructor(
private val controller: ControlsControllerImpl,
broadcastDispatcher: BroadcastDispatcher
broadcastDispatcher: BroadcastDispatcher,
private val globalActionsComponent: GlobalActionsComponent
) : LifecycleActivity() {
companion object {
@@ -86,12 +88,20 @@ class ControlsEditingActivity @Inject constructor(
bindViews()
bindButtons()
}
override fun onStart() {
super.onStart()
setUpList()
currentUserTracker.startTracking()
}
override fun onStop() {
super.onStop()
currentUserTracker.stopTracking()
}
private fun bindViews() {
setContentView(R.layout.controls_management)
@@ -129,12 +139,21 @@ class ControlsEditingActivity @Inject constructor(
}
}
val rootView = requireViewById<ViewGroup>(R.id.controls_management_root)
saveButton = requireViewById<Button>(R.id.done).apply {
isEnabled = false
setText(R.string.save)
setOnClickListener {
saveFavorites()
finishAffinity()
ControlsAnimations.exitAnimation(
rootView,
object : Runnable {
override fun run() {
finish()
}
}
).start()
globalActionsComponent.handleShowGlobalActionsMenu()
}
}
}

View File

@@ -40,6 +40,7 @@ import com.android.systemui.controls.TooltipManager
import com.android.systemui.controls.controller.ControlsControllerImpl
import com.android.systemui.controls.controller.StructureInfo
import com.android.systemui.dagger.qualifiers.Main
import com.android.systemui.globalactions.GlobalActionsComponent
import com.android.systemui.settings.CurrentUserTracker
import com.android.systemui.util.LifecycleActivity
import java.text.Collator
@@ -51,7 +52,8 @@ class ControlsFavoritingActivity @Inject constructor(
@Main private val executor: Executor,
private val controller: ControlsControllerImpl,
private val listingController: ControlsListingController,
broadcastDispatcher: BroadcastDispatcher
broadcastDispatcher: BroadcastDispatcher,
private val globalActionsComponent: GlobalActionsComponent
) : LifecycleActivity() {
companion object {
@@ -82,6 +84,8 @@ class ControlsFavoritingActivity @Inject constructor(
private var listOfStructures = emptyList<StructureContainer>()
private lateinit var comparator: Comparator<StructureContainer>
private var cancelLoadRunnable: Runnable? = null
private var isPagerLoaded = false
private val currentUserTracker = object : CurrentUserTracker(broadcastDispatcher) {
private val startingUser = controller.currentUserId
@@ -124,14 +128,6 @@ class ControlsFavoritingActivity @Inject constructor(
component = intent.getParcelableExtra<ComponentName>(Intent.EXTRA_COMPONENT_NAME)
bindViews()
setUpPager()
loadControls()
listingController.addCallback(listingCallback)
currentUserTracker.startTracking()
}
private val controlsModelCallback = object : ControlsModel.ControlsModelCallback {
@@ -180,7 +176,7 @@ class ControlsFavoritingActivity @Inject constructor(
ControlsAnimations.enterAnimation(pageIndicator).start()
ControlsAnimations.enterAnimation(structurePager).start()
}
})
}, Consumer { runnable -> cancelLoadRunnable = runnable })
}
}
@@ -299,6 +295,7 @@ class ControlsFavoritingActivity @Inject constructor(
}
}
val rootView = requireViewById<ViewGroup>(R.id.controls_management_root)
doneButton = requireViewById<Button>(R.id.done).apply {
isEnabled = false
setOnClickListener {
@@ -309,7 +306,16 @@ class ControlsFavoritingActivity @Inject constructor(
StructureInfo(component!!, it.structureName, favoritesForStorage)
)
}
finishAffinity()
ControlsAnimations.exitAnimation(
rootView,
object : Runnable {
override fun run() {
finish()
}
}
).start()
globalActionsComponent.handleShowGlobalActionsMenu()
}
}
}
@@ -319,15 +325,39 @@ class ControlsFavoritingActivity @Inject constructor(
mTooltipManager?.hide(false)
}
override fun onStart() {
super.onStart()
listingController.addCallback(listingCallback)
currentUserTracker.startTracking()
}
override fun onResume() {
super.onResume()
// only do once, to make sure that any user changes do not get replaces if resume is called
// more than once
if (!isPagerLoaded) {
setUpPager()
loadControls()
isPagerLoaded = true
}
}
override fun onStop() {
super.onStop()
listingController.removeCallback(listingCallback)
currentUserTracker.stopTracking()
}
override fun onConfigurationChanged(newConfig: Configuration) {
super.onConfigurationChanged(newConfig)
mTooltipManager?.hide(false)
}
override fun onDestroy() {
currentUserTracker.stopTracking()
listingController.removeCallback(listingCallback)
controller.cancelLoad()
cancelLoadRunnable?.run()
super.onDestroy()
}

View File

@@ -84,8 +84,22 @@ class ControlsProviderSelectorActivity @Inject constructor(
}
recyclerView = requireViewById(R.id.list)
recyclerView.alpha = 0.0f
recyclerView.layoutManager = LinearLayoutManager(applicationContext)
requireViewById<TextView>(R.id.title).apply {
text = resources.getText(R.string.controls_providers_title)
}
requireViewById<Button>(R.id.done).setOnClickListener {
this@ControlsProviderSelectorActivity.finishAffinity()
}
}
override fun onStart() {
super.onStart()
currentUserTracker.startTracking()
recyclerView.alpha = 0.0f
recyclerView.adapter = AppAdapter(
backExecutor,
executor,
@@ -105,16 +119,11 @@ class ControlsProviderSelectorActivity @Inject constructor(
}
})
}
}
requireViewById<TextView>(R.id.title).apply {
text = resources.getText(R.string.controls_providers_title)
}
requireViewById<Button>(R.id.done).setOnClickListener {
this@ControlsProviderSelectorActivity.finishAffinity()
}
currentUserTracker.startTracking()
override fun onStop() {
super.onStop()
currentUserTracker.stopTracking()
}
/**

View File

@@ -2234,7 +2234,7 @@ public class GlobalActionsDialog implements DialogInterface.OnDismissListener,
private void dismissForControlsActivity() {
dismissWithAnimation(() -> {
ViewGroup root = (ViewGroup) mGlobalActionsLayout.getRootView();
ViewGroup root = (ViewGroup) mGlobalActionsLayout.getParent();
ControlsAnimations.exitAnimation(root, this::completeDismiss).start();
});
}

View File

@@ -89,6 +89,7 @@ class ControlsControllerImplTest : SysuiTestCase() {
@Captor
private lateinit var controlLoadCallbackCaptor:
ArgumentCaptor<ControlsBindingController.LoadCallback>
@Captor
private lateinit var broadcastReceiverCaptor: ArgumentCaptor<BroadcastReceiver>
@Captor
@@ -97,6 +98,7 @@ class ControlsControllerImplTest : SysuiTestCase() {
private lateinit var delayableExecutor: FakeExecutor
private lateinit var controller: ControlsControllerImpl
private lateinit var canceller: DidRunRunnable
companion object {
fun <T> capture(argumentCaptor: ArgumentCaptor<T>): T = argumentCaptor.capture()
@@ -146,6 +148,9 @@ class ControlsControllerImplTest : SysuiTestCase() {
}
}
canceller = DidRunRunnable()
`when`(bindingController.bindAndLoad(any(), any())).thenReturn(canceller)
controller = ControlsControllerImpl(
wrapper,
delayableExecutor,
@@ -266,7 +271,7 @@ class ControlsControllerImplTest : SysuiTestCase() {
assertTrue(favorites.isEmpty())
assertFalse(data.errorOnLoad)
})
}, Consumer {})
verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
capture(controlLoadCallbackCaptor))
@@ -301,7 +306,7 @@ class ControlsControllerImplTest : SysuiTestCase() {
assertEquals(1, favorites.size)
assertEquals(TEST_CONTROL_ID, favorites[0])
assertFalse(data.errorOnLoad)
})
}, Consumer {})
verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
capture(controlLoadCallbackCaptor))
@@ -332,7 +337,7 @@ class ControlsControllerImplTest : SysuiTestCase() {
assertEquals(1, favorites.size)
assertEquals(TEST_CONTROL_ID, favorites[0])
assertFalse(data.errorOnLoad)
})
}, Consumer {})
verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
capture(controlLoadCallbackCaptor))
@@ -363,7 +368,7 @@ class ControlsControllerImplTest : SysuiTestCase() {
assertEquals(1, favorites.size)
assertEquals(TEST_CONTROL_ID, favorites[0])
assertTrue(data.errorOnLoad)
})
}, Consumer {})
verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
capture(controlLoadCallbackCaptor))
@@ -377,22 +382,15 @@ class ControlsControllerImplTest : SysuiTestCase() {
@Test
fun testCancelLoad() {
val canceller = object : Runnable {
var ran = false
override fun run() {
ran = true
}
}
`when`(bindingController.bindAndLoad(any(), any())).thenReturn(canceller)
var loaded = false
var cancelRunnable: Runnable? = null
controller.replaceFavoritesForStructure(TEST_STRUCTURE_INFO)
delayableExecutor.runAllReady()
controller.loadForComponent(TEST_COMPONENT, Consumer {
loaded = true
})
}, Consumer { runnable -> cancelRunnable = runnable })
controller.cancelLoad()
cancelRunnable?.run()
delayableExecutor.runAllReady()
assertFalse(loaded)
@@ -400,61 +398,47 @@ class ControlsControllerImplTest : SysuiTestCase() {
}
@Test
fun testCancelLoad_noCancelAfterSuccessfulLoad() {
val canceller = object : Runnable {
var ran = false
override fun run() {
ran = true
}
}
`when`(bindingController.bindAndLoad(any(), any())).thenReturn(canceller)
fun testCancelLoad_afterSuccessfulLoad() {
var loaded = false
var cancelRunnable: Runnable? = null
controller.replaceFavoritesForStructure(TEST_STRUCTURE_INFO)
delayableExecutor.runAllReady()
controller.loadForComponent(TEST_COMPONENT, Consumer {
loaded = true
})
}, Consumer { runnable -> cancelRunnable = runnable })
verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
capture(controlLoadCallbackCaptor))
controlLoadCallbackCaptor.value.accept(emptyList())
controller.cancelLoad()
cancelRunnable?.run()
delayableExecutor.runAllReady()
assertTrue(loaded)
assertFalse(canceller.ran)
assertTrue(canceller.ran)
}
@Test
fun testCancelLoad_noCancelAfterErrorLoad() {
val canceller = object : Runnable {
var ran = false
override fun run() {
ran = true
}
}
`when`(bindingController.bindAndLoad(any(), any())).thenReturn(canceller)
fun testCancelLoad_afterErrorLoad() {
var loaded = false
var cancelRunnable: Runnable? = null
controller.replaceFavoritesForStructure(TEST_STRUCTURE_INFO)
delayableExecutor.runAllReady()
controller.loadForComponent(TEST_COMPONENT, Consumer {
loaded = true
})
}, Consumer { runnable -> cancelRunnable = runnable })
verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
capture(controlLoadCallbackCaptor))
controlLoadCallbackCaptor.value.error("")
controller.cancelLoad()
cancelRunnable?.run()
delayableExecutor.runAllReady()
assertTrue(loaded)
assertFalse(canceller.ran)
assertTrue(canceller.ran)
}
@Test
@@ -465,7 +449,7 @@ class ControlsControllerImplTest : SysuiTestCase() {
val newControlInfo = TEST_CONTROL_INFO.copy(controlTitle = TEST_CONTROL_TITLE_2)
val control = statelessBuilderFromInfo(newControlInfo).build()
controller.loadForComponent(TEST_COMPONENT, Consumer {})
controller.loadForComponent(TEST_COMPONENT, Consumer {}, Consumer {})
verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
capture(controlLoadCallbackCaptor))
@@ -963,3 +947,10 @@ class ControlsControllerImplTest : SysuiTestCase() {
assertTrue(controller.getFavoritesForStructure(TEST_COMPONENT_2, TEST_STRUCTURE).isEmpty())
}
}
private class DidRunRunnable() : Runnable {
var ran = false
override fun run() {
ran = true
}
}