From d8a6ca68ebaade1941846fc5d47dcba216760b93 Mon Sep 17 00:00:00 2001 From: Matt Pietal Date: Tue, 21 Apr 2020 15:00:15 -0400 Subject: [PATCH] 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 --- .../ControlsBindingControllerImpl.kt | 23 +++++-- .../controls/controller/ControlsController.kt | 12 ++-- .../controller/ControlsControllerImpl.kt | 31 ++++----- .../management/ControlsEditingActivity.kt | 23 ++++++- .../management/ControlsFavoritingActivity.kt | 58 ++++++++++++---- .../ControlsProviderSelectorActivity.kt | 29 +++++--- .../globalactions/GlobalActionsDialog.java | 2 +- .../controller/ControlsControllerImplTest.kt | 69 ++++++++----------- 8 files changed, 148 insertions(+), 99 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt index 7e8fec716b1f0..e84f439c1fe2a 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt @@ -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() - 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() + } } } } diff --git a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsController.kt b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsController.kt index 79a5b0a1ce525..bc97c10756fd4 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsController.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsController.kt @@ -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 + dataCallback: Consumer, + cancelWrapper: Consumer ) - /** - * Cancels a pending load call - */ - fun cancelLoad() - /** * Request to subscribe for favorited controls per structure * diff --git a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt index 5626a5de2e3cb..8e88756b16fd3 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt @@ -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>() @@ -276,28 +274,29 @@ class ControlsControllerImpl @Inject constructor ( override fun loadForComponent( componentName: ComponentName, - dataCallback: Consumer + dataCallback: Consumer, + cancelWrapper: Consumer ) { 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) { - 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, diff --git a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsEditingActivity.kt b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsEditingActivity.kt index 273e47c1c0944..640c90d2ba597 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsEditingActivity.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsEditingActivity.kt @@ -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(R.id.controls_management_root) saveButton = requireViewById