From 1a6a79445f2e34398a464aa501a9e19a3b3c7209 Mon Sep 17 00:00:00 2001
From: Fabian Kozynski <kozynski@google.com>
Date: Tue, 10 Mar 2020 11:19:04 -0400
Subject: [PATCH] Cancels loading when moving out of Favorites screen

This CL adds a way to call cancel the load request. The request cannot
be cancelled once onComplete or onError have been received.

Test: atest
Fixes: 150850363
Change-Id: I1d4034bb024f829edbb7c3b98430df75af7eb581
---
 .../controller/ControlsBindingController.kt   |  3 +-
 .../ControlsBindingControllerImpl.kt          | 15 +++-
 .../controls/controller/ControlsController.kt |  5 ++
 .../controller/ControlsControllerImpl.kt      | 19 +++--
 .../management/ControlsFavoritingActivity.kt  | 23 ++++--
 .../ControlsBindingControllerImplTest.kt      | 65 +++++++++++++++
 .../controller/ControlsControllerImplTest.kt  | 82 +++++++++++++++++++
 7 files changed, 195 insertions(+), 17 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingController.kt b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingController.kt
index fd6e2566b1b6c..c5af436e7e925 100644
--- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingController.kt
+++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingController.kt
@@ -38,8 +38,9 @@ interface ControlsBindingController : UserAwareController {
      *
      * @param component The [ComponentName] of the service to bind
      * @param callback a callback to return the loaded controls to (or an error).
+     * @return a runnable to cancel the load
      */
-    fun bindAndLoad(component: ComponentName, callback: LoadCallback)
+    fun bindAndLoad(component: ComponentName, callback: LoadCallback): Runnable
 
     /**
      * Request to bind to the given service.
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 8f02c252beb1b..f3bd0b57a0cb2 100644
--- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsBindingControllerImpl.kt
@@ -116,8 +116,10 @@ open class ControlsBindingControllerImpl @Inject constructor(
     override fun bindAndLoad(
         component: ComponentName,
         callback: ControlsBindingController.LoadCallback
-    ) {
-        retrieveLifecycleManager(component)?.maybeBindAndLoad(LoadSubscriber(callback))
+    ): Runnable {
+        val subscriber = LoadSubscriber(callback)
+        retrieveLifecycleManager(component)?.maybeBindAndLoad(subscriber)
+        return subscriber.loadCancel()
     }
 
     override fun subscribe(structureInfo: StructureInfo) {
@@ -208,7 +210,6 @@ open class ControlsBindingControllerImpl @Inject constructor(
     ) : CallbackRunnable(token) {
         override fun doRun() {
             callback.accept(list)
-            provider?.unbindService()
         }
     }
 
@@ -292,8 +293,14 @@ open class ControlsBindingControllerImpl @Inject constructor(
     ) : IControlsSubscriber.Stub() {
         val loadedControls = ArrayList<Control>()
         var hasError = false
+        private var _loadCancelInternal: (() -> Unit)? = null
+        fun loadCancel() = Runnable {
+                Log.d(TAG, "Cancel load requested")
+                _loadCancelInternal?.invoke()
+            }
 
         override fun onSubscribe(token: IBinder, subs: IControlsSubscription) {
+            _loadCancelInternal = subs::cancel
             backgroundExecutor.execute(OnSubscribeRunnable(token, subs))
         }
 
@@ -302,10 +309,12 @@ open class ControlsBindingControllerImpl @Inject constructor(
         }
         override fun onError(token: IBinder, s: String) {
             hasError = true
+            _loadCancelInternal = {}
             backgroundExecutor.execute(OnLoadErrorRunnable(token, s, callback))
         }
 
         override fun onComplete(token: IBinder) {
+            _loadCancelInternal = {}
             if (!hasError) {
                 backgroundExecutor.execute(OnLoadRunnable(token, loadedControls, callback))
             }
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 7eafe2e65ccaf..9e0d26c3935f3 100644
--- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsController.kt
+++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsController.kt
@@ -58,6 +58,11 @@ interface ControlsController : UserAwareController {
         dataCallback: Consumer<LoadData>
     )
 
+    /**
+     * 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 e5d36f942ac85..9cb902f51f22a 100644
--- a/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/controls/controller/ControlsControllerImpl.kt
@@ -72,6 +72,8 @@ class ControlsControllerImpl @Inject constructor (
 
     private var userChanging: Boolean = true
 
+    private var loadCanceller: Runnable? = null
+
     private var currentUser = UserHandle.of(ActivityManager.getCurrentUser())
     override val currentUserId
         get() = currentUser.identifier
@@ -213,8 +215,9 @@ class ControlsControllerImpl @Inject constructor (
         if (!confirmAvailability()) {
             if (userChanging) {
                 // Try again later, userChanging should not last forever. If so, we have bigger
-                // problems
-                executor.executeDelayed(
+                // 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
@@ -224,10 +227,11 @@ class ControlsControllerImpl @Inject constructor (
             }
             return
         }
-        bindingController.bindAndLoad(
+        loadCanceller = bindingController.bindAndLoad(
                 componentName,
                 object : ControlsBindingController.LoadCallback {
                     override fun accept(controls: List<Control>) {
+                        loadCanceller = null
                         executor.execute {
                             val favoritesForComponentKeys = Favorites
                                 .getControlsForComponent(componentName).map { it.controlId }
@@ -251,12 +255,12 @@ class ControlsControllerImpl @Inject constructor (
                                 controlsWithFavorite,
                                 favoritesForComponentKeys
                             )
-
                             dataCallback.accept(loadData)
                         }
                     }
 
                     override fun error(message: String) {
+                        loadCanceller = null
                         executor.execute {
                             val loadData = Favorites.getControlsForComponent(componentName)
                                 .let { controls ->
@@ -269,7 +273,6 @@ class ControlsControllerImpl @Inject constructor (
                                         true
                                 )
                             }
-
                             dataCallback.accept(loadData)
                         }
                     }
@@ -277,6 +280,12 @@ class ControlsControllerImpl @Inject constructor (
         )
     }
 
+    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/ControlsFavoritingActivity.kt b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsFavoritingActivity.kt
index 502354a8703af..f2303e622f8de 100644
--- a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsFavoritingActivity.kt
+++ b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsFavoritingActivity.kt
@@ -70,6 +70,7 @@ class ControlsFavoritingActivity @Inject constructor(
     private lateinit var iconFrame: View
     private lateinit var pageIndicator: ManagementPageIndicator
     private var mTooltipManager: TooltipManager? = null
+    private lateinit var doneButton: View
     private var listOfStructures = emptyList<StructureContainer>()
 
     private lateinit var comparator: Comparator<StructureContainer>
@@ -137,6 +138,7 @@ class ControlsFavoritingActivity @Inject constructor(
                     StructureContainer(it.key, AllModel(it.value, favoriteKeys, emptyZoneString))
                 }.sortedWith(comparator)
                 executor.execute {
+                    doneButton.isEnabled = true
                     structurePager.adapter = StructureAdapter(listOfStructures)
                     if (error) {
                         statusText.text = resources.getText(R.string.controls_favorite_load_error)
@@ -180,6 +182,7 @@ class ControlsFavoritingActivity @Inject constructor(
             layoutResource = R.layout.controls_management_favorites
             inflate()
         }
+
         statusText = requireViewById(R.id.status_message)
         if (shouldShowTooltip()) {
             mTooltipManager = TooltipManager(statusText.context,
@@ -248,15 +251,18 @@ class ControlsFavoritingActivity @Inject constructor(
             }
         }
 
-        requireViewById<Button>(R.id.done).setOnClickListener {
-            if (component == null) return@setOnClickListener
-            listOfStructures.forEach {
-                val favoritesForStorage = it.model.favorites.map { it.build() }
-                controller.replaceFavoritesForStructure(StructureInfo(component!!, it.structureName,
-                        favoritesForStorage))
+        doneButton = requireViewById<Button>(R.id.done).apply {
+            isEnabled = false
+            setOnClickListener {
+                if (component == null) return@setOnClickListener
+                listOfStructures.forEach {
+                    val favoritesForStorage = it.model.favorites.map { it.build() }
+                    controller.replaceFavoritesForStructure(
+                        StructureInfo(component!!, it.structureName, favoritesForStorage)
+                    )
+                }
+                finishAffinity()
             }
-
-            finishAffinity()
         }
     }
 
@@ -273,6 +279,7 @@ class ControlsFavoritingActivity @Inject constructor(
     override fun onDestroy() {
         currentUserTracker.stopTracking()
         listingController.removeCallback(listingCallback)
+        controller.cancelLoad()
         super.onDestroy()
     }
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsBindingControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsBindingControllerImplTest.kt
index eceb1ddaaf062..8cb1af46d3073 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsBindingControllerImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsBindingControllerImplTest.kt
@@ -22,6 +22,8 @@ import android.os.Binder
 import android.os.UserHandle
 import android.service.controls.Control
 import android.service.controls.DeviceTypes
+import android.service.controls.IControlsSubscriber
+import android.service.controls.IControlsSubscription
 import android.testing.AndroidTestingRunner
 import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
@@ -34,6 +36,8 @@ import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.ArgumentCaptor
+import org.mockito.Captor
 import org.mockito.Mock
 import org.mockito.Mockito
 import org.mockito.Mockito.`when`
@@ -49,6 +53,7 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {
 
     companion object {
         fun <T> any(): T = Mockito.any<T>()
+        fun <T> capture(argumentCaptor: ArgumentCaptor<T>): T = argumentCaptor.capture()
         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")
@@ -56,6 +61,8 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {
 
     @Mock
     private lateinit var mockControlsController: ControlsController
+    @Captor
+    private lateinit var subscriberCaptor: ArgumentCaptor<IControlsSubscriber.Stub>
 
     private val user = UserHandle.of(mContext.userId)
     private val otherUser = UserHandle.of(user.identifier + 1)
@@ -96,6 +103,64 @@ class ControlsBindingControllerImplTest : SysuiTestCase() {
         verify(providers[0]).maybeBindAndLoad(any())
     }
 
+    @Test
+    fun testBindAndLoad_cancel() {
+        val callback = object : ControlsBindingController.LoadCallback {
+            override fun error(message: String) {}
+
+            override fun accept(t: List<Control>) {}
+        }
+        val subscription = mock(IControlsSubscription::class.java)
+
+        val canceller = controller.bindAndLoad(TEST_COMPONENT_NAME_1, callback)
+
+        verify(providers[0]).maybeBindAndLoad(capture(subscriberCaptor))
+        subscriberCaptor.value.onSubscribe(Binder(), subscription)
+
+        canceller.run()
+        verify(subscription).cancel()
+    }
+
+    @Test
+    fun testBindAndLoad_noCancelAfterOnComplete() {
+        val callback = object : ControlsBindingController.LoadCallback {
+            override fun error(message: String) {}
+
+            override fun accept(t: List<Control>) {}
+        }
+        val subscription = mock(IControlsSubscription::class.java)
+
+        val canceller = controller.bindAndLoad(TEST_COMPONENT_NAME_1, callback)
+
+        verify(providers[0]).maybeBindAndLoad(capture(subscriberCaptor))
+        val b = Binder()
+        subscriberCaptor.value.onSubscribe(b, subscription)
+
+        subscriberCaptor.value.onComplete(b)
+        canceller.run()
+        verify(subscription, never()).cancel()
+    }
+
+    @Test
+    fun testBindAndLoad_noCancelAfterOnError() {
+        val callback = object : ControlsBindingController.LoadCallback {
+            override fun error(message: String) {}
+
+            override fun accept(t: List<Control>) {}
+        }
+        val subscription = mock(IControlsSubscription::class.java)
+
+        val canceller = controller.bindAndLoad(TEST_COMPONENT_NAME_1, callback)
+
+        verify(providers[0]).maybeBindAndLoad(capture(subscriberCaptor))
+        val b = Binder()
+        subscriberCaptor.value.onSubscribe(b, subscription)
+
+        subscriberCaptor.value.onError(b, "")
+        canceller.run()
+        verify(subscription, never()).cancel()
+    }
+
     @Test
     fun testBindService() {
         controller.bindService(TEST_COMPONENT_NAME_1)
diff --git a/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsControllerImplTest.kt
index c70c56a8660b9..f9c98157edb82 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsControllerImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/controls/controller/ControlsControllerImplTest.kt
@@ -345,6 +345,88 @@ class ControlsControllerImplTest : SysuiTestCase() {
         assertTrue(loaded)
     }
 
+    @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
+        controller.replaceFavoritesForStructure(TEST_STRUCTURE_INFO)
+        delayableExecutor.runAllReady()
+        controller.loadForComponent(TEST_COMPONENT, Consumer {
+            loaded = true
+        })
+
+        controller.cancelLoad()
+        delayableExecutor.runAllReady()
+
+        assertFalse(loaded)
+        assertTrue(canceller.ran)
+    }
+
+    @Test
+    fun testCancelLoad_noCancelAfterSuccessfulLoad() {
+        val canceller = object : Runnable {
+            var ran = false
+            override fun run() {
+                ran = true
+            }
+        }
+        `when`(bindingController.bindAndLoad(any(), any())).thenReturn(canceller)
+
+        var loaded = false
+        controller.replaceFavoritesForStructure(TEST_STRUCTURE_INFO)
+        delayableExecutor.runAllReady()
+        controller.loadForComponent(TEST_COMPONENT, Consumer {
+            loaded = true
+        })
+
+        verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
+            capture(controlLoadCallbackCaptor))
+
+        controlLoadCallbackCaptor.value.accept(emptyList())
+
+        controller.cancelLoad()
+        delayableExecutor.runAllReady()
+
+        assertTrue(loaded)
+        assertFalse(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)
+
+        var loaded = false
+        controller.replaceFavoritesForStructure(TEST_STRUCTURE_INFO)
+        delayableExecutor.runAllReady()
+        controller.loadForComponent(TEST_COMPONENT, Consumer {
+            loaded = true
+        })
+
+        verify(bindingController).bindAndLoad(eq(TEST_COMPONENT),
+            capture(controlLoadCallbackCaptor))
+
+        controlLoadCallbackCaptor.value.error("")
+
+        controller.cancelLoad()
+        delayableExecutor.runAllReady()
+
+        assertTrue(loaded)
+        assertFalse(canceller.ran)
+    }
+
     @Test
     fun testFavoriteInformationModifiedOnLoad() {
         controller.replaceFavoritesForStructure(TEST_STRUCTURE_INFO)