Removed unnecessary calls and creations of Binder

The previous behavior registered HotspotController to the WifiManager
every time a callback was added in order to trigger an update in the
number of current connections. This caused two things:
* Every registering with WifiManager creates a new Binder object (see
WifiManager#registerSoftApCallback)
* Every callback added triggered Callback#onHotspotChanged on all the
current callbacks

This removes both behaviors while maintaining the information sent to
the callback on addCallback by posting an update. We assume that the
information currently in HotspotController is up to date, if not, there
would be a callback.

Test: atest
Test: manual (opening and closing QS)
Fixes: 123546508
Change-Id: Ica73082beb752a7c20e11b2b2a441fa84bf948a1

packages/SystemUI/src/com/android/systemui/statusbar/policy/HotspotControllerImpl.java

@@ -93,14 +93,28 @@ public class HotspotControllerImpl implements HotspotController, WifiManager.Sof
         return null;
     }
 
+    /**
+     * Adds {@code callback} to the controller. The controller will update the callback on state
+     * changes. It will immediately trigger the callback added to notify current state.
+     * @param callback
+     */
     @Override
     public void addCallback(Callback callback) {
         synchronized (mCallbacks) {
             if (callback == null || mCallbacks.contains(callback)) return;
             if (DEBUG) Log.d(TAG, "addCallback " + callback);
             mCallbacks.add(callback);
-
-            updateWifiStateListeners(!mCallbacks.isEmpty());
+            if (mWifiManager != null) {
+                if (mCallbacks.size() == 1) {
+                    mWifiManager.registerSoftApCallback(this, mMainHandler);
+                } else {
+                    // mWifiManager#registerSoftApCallback triggers a call to onNumClientsChanged
+                    // on the Main Handler. In order to always update the callback on added, we
+                    // make this call when adding callbacks after the first.
+                    mMainHandler.post(() ->
+                            callback.onHotspotChanged(isHotspotEnabled(), mNumConnectedDevices));
+                }
+            }
         }
     }
 
@@ -110,27 +124,9 @@ public class HotspotControllerImpl implements HotspotController, WifiManager.Sof
         if (DEBUG) Log.d(TAG, "removeCallback " + callback);
         synchronized (mCallbacks) {
             mCallbacks.remove(callback);
-            updateWifiStateListeners(!mCallbacks.isEmpty());
-        }
-    }
-
-    /**
-     * Updates the wifi state receiver to either start or stop listening to get updates to the
-     * hotspot status. Additionally starts listening to wifi manager state to track the number of
-     * connected devices.
-     *
-     * @param shouldListen whether we should start listening to various wifi statuses
-     */
-    private void updateWifiStateListeners(boolean shouldListen) {
-        if (mWifiManager == null) {
-            return;
-        }
-        if (shouldListen) {
-            mWifiManager.registerSoftApCallback(
-                    this,
-                    mMainHandler);
-        } else {
-            mWifiManager.unregisterSoftApCallback(this);
+            if (mCallbacks.isEmpty() && mWifiManager != null) {
+                mWifiManager.unregisterSoftApCallback(this);
+            }
         }
     }
 

packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/HotspotControllerImplTest.java

@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2019 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.statusbar.policy;
+
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import android.net.ConnectivityManager;
+import android.net.wifi.WifiManager;
+import android.os.Handler;
+import android.testing.AndroidTestingRunner;
+import android.testing.TestableLooper;
+
+import androidx.test.filters.SmallTest;
+
+import com.android.systemui.SysuiTestCase;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.invocation.InvocationOnMock;
+
+@SmallTest
+@RunWith(AndroidTestingRunner.class)
+@TestableLooper.RunWithLooper
+public class HotspotControllerImplTest extends SysuiTestCase {
+
+    @Mock
+    private ConnectivityManager mConnectivityManager;
+    @Mock
+    private WifiManager mWifiManager;
+    @Mock
+    private HotspotController.Callback mCallback1;
+    @Mock
+    private HotspotController.Callback mCallback2;
+    private HotspotControllerImpl mController;
+    private TestableLooper mLooper;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+        mLooper = TestableLooper.get(this);
+
+        mContext.addMockSystemService(ConnectivityManager.class, mConnectivityManager);
+        mContext.addMockSystemService(WifiManager.class, mWifiManager);
+
+        doAnswer((InvocationOnMock invocation) -> {
+            ((WifiManager.SoftApCallback) invocation.getArgument(0)).onNumClientsChanged(1);
+            return null;
+        }).when(mWifiManager).registerSoftApCallback(any(WifiManager.SoftApCallback.class),
+                any(Handler.class));
+
+        mController = new HotspotControllerImpl(mContext, new Handler(mLooper.getLooper()));
+    }
+
+    @Test
+    public void testAddingTwoCallbacksRegistersToWifiManagerOnce() {
+        mController.addCallback(mCallback1);
+        mController.addCallback(mCallback2);
+
+        verify(mWifiManager, times(1)).registerSoftApCallback(eq(mController), any());
+    }
+
+    @Test
+    public void testAddingCallbacksDoesntUpdateAll() {
+        mController.addCallback(mCallback1);
+        mController.addCallback(mCallback2);
+
+        mLooper.processAllMessages();
+        // Each callback should be updated only once
+        verify(mCallback1, times(1)).onHotspotChanged(anyBoolean(), anyInt());
+        verify(mCallback2, times(1)).onHotspotChanged(anyBoolean(), anyInt());
+    }
+
+    @Test
+    public void testRemovingTwoCallbacksUnegistersToWifiManagerOnce() {
+        mController.addCallback(mCallback1);
+        mController.addCallback(mCallback2);
+
+        mController.removeCallback(mCallback1);
+        mController.removeCallback(mCallback2);
+
+        verify(mWifiManager, times(1)).unregisterSoftApCallback(mController);
+    }
+
+    @Test
+    public void testDoNotUnregisterIfRemainingCallbacks() {
+        mController.addCallback(mCallback1);
+        mController.addCallback(mCallback2);
+
+        verify(mWifiManager, never()).unregisterSoftApCallback(any());
+    }
+
+}