User binder as identifier for scanResultcallback

Bug: 143138047
Test: atest android.net.wifi
Test: atest com.android.server.wifi
Change-Id: I91d68b247490ac33656decc519c39c77bfe786a4

7 files changed, 183 insertions, 83 deletions

diff --git a/api/current.txt b/api/current.txt
index c2939fd38e4b..9b5077dd81db 100644
--- a/api/current.txt
+++ b/api/current.txt
@@ -30062,7 +30062,6 @@ package android.net.wifi {
     method @Deprecated public int addNetwork(android.net.wifi.WifiConfiguration);
     method @RequiresPermission(android.Manifest.permission.CHANGE_WIFI_STATE) public int addNetworkSuggestions(@NonNull java.util.List<android.net.wifi.WifiNetworkSuggestion>);
     method public void addOrUpdatePasspointConfiguration(android.net.wifi.hotspot2.PasspointConfiguration);
-    method @RequiresPermission(android.Manifest.permission.ACCESS_WIFI_STATE) public void addScanResultsListener(@NonNull java.util.concurrent.Executor, @NonNull android.net.wifi.WifiManager.ScanResultsListener);
     method @RequiresPermission(allOf={android.Manifest.permission.ACCESS_FINE_LOCATION, android.Manifest.permission.ACCESS_WIFI_STATE}) public void addSuggestionConnectionStatusListener(@NonNull java.util.concurrent.Executor, @NonNull android.net.wifi.WifiManager.SuggestionConnectionStatusListener);
     method @Deprecated public static int calculateSignalLevel(int, int);
     method public int calculateSignalLevel(int);
@@ -30099,10 +30098,10 @@ package android.net.wifi {
     method @Deprecated public boolean pingSupplicant();
     method @Deprecated public boolean reassociate();
     method @Deprecated public boolean reconnect();
+    method @RequiresPermission(android.Manifest.permission.ACCESS_WIFI_STATE) public void registerScanResultsCallback(@NonNull java.util.concurrent.Executor, @NonNull android.net.wifi.WifiManager.ScanResultsCallback);
     method @Deprecated public boolean removeNetwork(int);
     method @RequiresPermission(android.Manifest.permission.CHANGE_WIFI_STATE) public int removeNetworkSuggestions(@NonNull java.util.List<android.net.wifi.WifiNetworkSuggestion>);
     method @Deprecated @RequiresPermission(anyOf={"android.permission.NETWORK_SETTINGS", "android.permission.NETWORK_CARRIER_PROVISIONING"}) public void removePasspointConfiguration(String);
-    method @RequiresPermission(android.Manifest.permission.ACCESS_WIFI_STATE) public void removeScanResultsListener(@NonNull android.net.wifi.WifiManager.ScanResultsListener);
     method @RequiresPermission(android.Manifest.permission.ACCESS_WIFI_STATE) public void removeSuggestionConnectionStatusListener(@NonNull android.net.wifi.WifiManager.SuggestionConnectionStatusListener);
     method @Deprecated public boolean saveConfiguration();
     method public void setTdlsEnabled(java.net.InetAddress, boolean);
@@ -30111,6 +30110,7 @@ package android.net.wifi {
     method @RequiresPermission(allOf={android.Manifest.permission.CHANGE_WIFI_STATE, android.Manifest.permission.ACCESS_FINE_LOCATION}) public void startLocalOnlyHotspot(android.net.wifi.WifiManager.LocalOnlyHotspotCallback, @Nullable android.os.Handler);
     method @Deprecated public boolean startScan();
     method @Deprecated public void startWps(android.net.wifi.WpsInfo, android.net.wifi.WifiManager.WpsCallback);
+    method @RequiresPermission(android.Manifest.permission.ACCESS_WIFI_STATE) public void unregisterScanResultsCallback(@NonNull android.net.wifi.WifiManager.ScanResultsCallback);
     method @Deprecated public int updateNetwork(android.net.wifi.WifiConfiguration);
     field public static final String ACTION_PICK_WIFI_NETWORK = "android.net.wifi.PICK_WIFI_NETWORK";
     field public static final String ACTION_REQUEST_SCAN_ALWAYS_AVAILABLE = "android.net.wifi.action.REQUEST_SCAN_ALWAYS_AVAILABLE";
@@ -30184,8 +30184,9 @@ package android.net.wifi {
     method public void setReferenceCounted(boolean);
   }
 
-  public static interface WifiManager.ScanResultsListener {
-    method public void onScanResultsAvailable();
+  public abstract static class WifiManager.ScanResultsCallback {
+    ctor public WifiManager.ScanResultsCallback();
+    method public abstract void onScanResultsAvailable();
   }
 
   public static interface WifiManager.SuggestionConnectionStatusListener {
diff --git a/wifi/java/android/net/wifi/IScanResultsCallback.aidl b/wifi/java/android/net/wifi/IScanResultsCallback.aidl
new file mode 100644
index 000000000000..56f602510fd9
--- /dev/null
+++ b/wifi/java/android/net/wifi/IScanResultsCallback.aidl
@@ -0,0 +1,27 @@
+/*
+ * 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 android.net.wifi;
+
+/**
+ * Interface for Wi-Fi scan result available callback.
+ *
+ * @hide
+ */
+oneway interface IScanResultsCallback
+{
+    void onScanResultsAvailable();
+}
diff --git a/wifi/java/android/net/wifi/IScanResultsListener.aidl b/wifi/java/android/net/wifi/IScanResultsListener.aidl
index bec74a620380..e7eaddd712c9 100644
--- a/wifi/java/android/net/wifi/IScanResultsListener.aidl
+++ b/wifi/java/android/net/wifi/IScanResultsListener.aidl
@@ -16,11 +16,8 @@
 
 package android.net.wifi;
 
-/**
- * Interface for Wi-Fi scan result available callback.
- *
- * @hide
- */
+/** @hide */
+
 oneway interface IScanResultsListener
 {
     void onScanResultsAvailable();
diff --git a/wifi/java/android/net/wifi/IWifiManager.aidl b/wifi/java/android/net/wifi/IWifiManager.aidl
index 4619372e2258..4a89c66dd0a0 100644
--- a/wifi/java/android/net/wifi/IWifiManager.aidl
+++ b/wifi/java/android/net/wifi/IWifiManager.aidl
@@ -28,7 +28,7 @@ import android.net.wifi.IActionListener;
 import android.net.wifi.IDppCallback;
 import android.net.wifi.ILocalOnlyHotspotCallback;
 import android.net.wifi.INetworkRequestMatchCallback;
-import android.net.wifi.IScanResultsListener;
+import android.net.wifi.IScanResultsCallback;
 import android.net.wifi.ISoftApCallback;
 import android.net.wifi.ISuggestionConnectionStatusListener;
 import android.net.wifi.ITrafficStateCallback;
@@ -233,9 +233,9 @@ interface IWifiManager
 
     oneway void getTxPacketCount(String packageName, in IBinder binder, in ITxPacketCountListener listener, int callbackIdentifier);
 
-    void registerScanResultsListener(in IBinder binder, in IScanResultsListener Listener, int listenerIdentifier);
+    void registerScanResultsCallback(in IScanResultsCallback callback);
 
-    void unregisterScanResultsListener(int listenerIdentifier);
+    void unregisterScanResultsCallback(in IScanResultsCallback callback);
 
     void registerSuggestionConnectionStatusListener(in IBinder binder, in ISuggestionConnectionStatusListener listener, int listenerIdentifier, String packageName, String featureId);
 
diff --git a/wifi/java/android/net/wifi/WifiManager.java b/wifi/java/android/net/wifi/WifiManager.java
index 942d795a45ca..2a0211b77450 100644
--- a/wifi/java/android/net/wifi/WifiManager.java
+++ b/wifi/java/android/net/wifi/WifiManager.java
@@ -5453,40 +5453,76 @@ public class WifiManager {
     }
 
     /**
-     * Interface for scan results listener. Should be implemented by applications and set when
-     * calling {@link WifiManager#addScanResultsListener(Executor, ScanResultsListener)}.
+     * Abstract class for scan results callback. Should be extended by applications and set when
+     * calling {@link WifiManager#registerScanResultsCallback(Executor, ScanResultsCallback)}.
      */
-    public interface ScanResultsListener {
+    public abstract static class ScanResultsCallback {
+        private final ScanResultsCallbackProxy mScanResultsCallbackProxy;
+
+        public ScanResultsCallback() {
+            mScanResultsCallbackProxy = new ScanResultsCallbackProxy();
+        }
 
         /**
-         * Called when new scan results available.
-         * Caller should use {@link WifiManager#getScanResults()} to get the scan results.
+         * Called when new scan results are available.
+         * Clients should use {@link WifiManager#getScanResults()} to get the scan results.
          */
-        void onScanResultsAvailable();
-    }
-
-    private class ScanResultsListenerProxy extends IScanResultsListener.Stub {
-        private final Executor mExecutor;
-        private final ScanResultsListener mListener;
+        public abstract void onScanResultsAvailable();
 
-        ScanResultsListenerProxy(Executor executor, ScanResultsListener listener) {
-            mExecutor = executor;
-            mListener = listener;
+        /*package*/ @NonNull ScanResultsCallbackProxy getProxy() {
+            return mScanResultsCallbackProxy;
         }
 
-        @Override
-        public void onScanResultsAvailable() {
-            mExecutor.execute(mListener::onScanResultsAvailable);
+        private static class ScanResultsCallbackProxy extends IScanResultsCallback.Stub {
+            private final Object mLock = new Object();
+            @Nullable @GuardedBy("mLock") private Executor mExecutor;
+            @Nullable @GuardedBy("mLock") private ScanResultsCallback mCallback;
+
+            ScanResultsCallbackProxy() {
+                mCallback = null;
+                mExecutor = null;
+            }
+
+            /*package*/ void initProxy(@NonNull Executor executor,
+                    @NonNull ScanResultsCallback callback) {
+                synchronized (mLock) {
+                    mExecutor = executor;
+                    mCallback = callback;
+                }
+            }
+
+            /*package*/ void cleanUpProxy() {
+                synchronized (mLock) {
+                    mExecutor = null;
+                    mCallback = null;
+                }
+            }
+
+            @Override
+            public void onScanResultsAvailable() {
+                ScanResultsCallback callback;
+                Executor executor;
+                synchronized (mLock) {
+                    executor = mExecutor;
+                    callback = mCallback;
+                }
+                if (callback == null || executor == null) {
+                    return;
+                }
+                Binder.clearCallingIdentity();
+                executor.execute(callback::onScanResultsAvailable);
+            }
         }
+
     }
 
     /**
-     * Add a listener for Scan Results. See {@link ScanResultsListener}.
+     * Register a callback for Scan Results. See {@link ScanResultsCallback}.
      * Caller will receive the event when scan results are available.
      * Caller should use {@link WifiManager#getScanResults()} requires
      * {@link android.Manifest.permission#ACCESS_FINE_LOCATION} to get the scan results.
-     * Caller can remove a previously registered listener using
-     * {@link WifiManager#removeScanResultsListener(ScanResultsListener)}
+     * Caller can remove a previously registered callback using
+     * {@link WifiManager#unregisterScanResultsCallback(ScanResultsCallback)}
      * Same caller can add multiple listeners.
      * <p>
      * Applications should have the
@@ -5494,49 +5530,52 @@ public class WifiManager {
      * without the permission will trigger a {@link java.lang.SecurityException}.
      * <p>
      *
-     * @param executor The executor to execute the listener of the {@code listener} object.
-     * @param listener listener for Scan Results events
+     * @param executor The executor to execute the callback of the {@code callback} object.
+     * @param callback callback for Scan Results events
      */
 
     @RequiresPermission(ACCESS_WIFI_STATE)
-    public void addScanResultsListener(@NonNull @CallbackExecutor Executor executor,
-            @NonNull ScanResultsListener listener) {
-        if (listener == null) throw new IllegalArgumentException("listener cannot be null");
+    public void registerScanResultsCallback(@NonNull @CallbackExecutor Executor executor,
+            @NonNull ScanResultsCallback callback) {
         if (executor == null) throw new IllegalArgumentException("executor cannot be null");
-        Log.v(TAG, "addScanResultsListener: listener=" + listener + ", executor=" + executor);
+        if (callback == null) throw new IllegalArgumentException("callback cannot be null");
+
+        Log.v(TAG, "registerScanResultsCallback: callback=" + callback
+                + ", executor=" + executor);
+        ScanResultsCallback.ScanResultsCallbackProxy proxy = callback.getProxy();
+        proxy.initProxy(executor, callback);
         try {
             IWifiManager iWifiManager = getIWifiManager();
             if (iWifiManager == null) {
                 throw new RemoteException("Wifi service is not running");
             }
-            iWifiManager.registerScanResultsListener(
-                    new Binder(),
-                    new ScanResultsListenerProxy(executor, listener),
-                    listener.hashCode());
+            iWifiManager.registerScanResultsCallback(proxy);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         }
     }
 
     /**
-     * Allow callers to remove a previously added listener. After calling this method,
+     * Allow callers to unregister a previously registered callback. After calling this method,
      * applications will no longer receive Scan Results events.
      *
-     * @param listener listener to remove for Scan Results events
+     * @param callback callback to unregister for Scan Results events
      */
     @RequiresPermission(ACCESS_WIFI_STATE)
-    public void removeScanResultsListener(@NonNull ScanResultsListener listener) {
-        if (listener == null) throw new IllegalArgumentException("listener cannot be null");
-        Log.v(TAG, "removeScanResultsListener: listener=" + listener);
-
+    public void unregisterScanResultsCallback(@NonNull ScanResultsCallback callback) {
+        if (callback == null) throw new IllegalArgumentException("callback cannot be null");
+        Log.v(TAG, "unregisterScanResultsCallback: Callback=" + callback);
+        ScanResultsCallback.ScanResultsCallbackProxy proxy = callback.getProxy();
         try {
             IWifiManager iWifiManager = getIWifiManager();
             if (iWifiManager == null) {
                 throw new RemoteException("Wifi service is not running");
             }
-            iWifiManager.unregisterScanResultsListener(listener.hashCode());
+            iWifiManager.unregisterScanResultsCallback(proxy);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
+        } finally {
+            proxy.cleanUpProxy();
         }
     }
 
diff --git a/wifi/java/com/android/server/wifi/BaseWifiService.java b/wifi/java/com/android/server/wifi/BaseWifiService.java
index ee03c11c826a..534e609f9af4 100644
--- a/wifi/java/com/android/server/wifi/BaseWifiService.java
+++ b/wifi/java/com/android/server/wifi/BaseWifiService.java
@@ -26,6 +26,7 @@ import android.net.wifi.IDppCallback;
 import android.net.wifi.ILocalOnlyHotspotCallback;
 import android.net.wifi.INetworkRequestMatchCallback;
 import android.net.wifi.IOnWifiUsabilityStatsListener;
+import android.net.wifi.IScanResultsCallback;
 import android.net.wifi.IScanResultsListener;
 import android.net.wifi.ISoftApCallback;
 import android.net.wifi.ISuggestionConnectionStatusListener;
@@ -512,18 +513,30 @@ public class BaseWifiService extends IWifiManager.Stub {
         throw new UnsupportedOperationException();
     }
 
-    @Override
+    /** @deprecated replaced by {@link #registerScanResultsCallback(IScanResultsCallback)} */
+    @Deprecated
     public void registerScanResultsListener(
             IBinder binder, IScanResultsListener listener, int listenerIdentifier) {
         throw new UnsupportedOperationException();
     }
 
-    @Override
+    /** @deprecated replaced by {@link #unregisterScanResultsCallback(IScanResultsCallback)} */
+    @Deprecated
     public void unregisterScanResultsListener(int listenerIdentifier) {
         throw new UnsupportedOperationException();
     }
 
     @Override
+    public void registerScanResultsCallback(IScanResultsCallback callback) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public void unregisterScanResultsCallback(IScanResultsCallback callback) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
     public void registerSuggestionConnectionStatusListener(IBinder binder,
             ISuggestionConnectionStatusListener listener,
             int listenerIdentifier, String packageName, String featureId) {
diff --git a/wifi/tests/src/android/net/wifi/WifiManagerTest.java b/wifi/tests/src/android/net/wifi/WifiManagerTest.java
index 507d50295d86..8cdcba67386b 100644
--- a/wifi/tests/src/android/net/wifi/WifiManagerTest.java
+++ b/wifi/tests/src/android/net/wifi/WifiManagerTest.java
@@ -60,7 +60,7 @@ import android.net.wifi.WifiManager.LocalOnlyHotspotSubscription;
 import android.net.wifi.WifiManager.NetworkRequestMatchCallback;
 import android.net.wifi.WifiManager.NetworkRequestUserSelectionCallback;
 import android.net.wifi.WifiManager.OnWifiUsabilityStatsListener;
-import android.net.wifi.WifiManager.ScanResultsListener;
+import android.net.wifi.WifiManager.ScanResultsCallback;
 import android.net.wifi.WifiManager.SoftApCallback;
 import android.net.wifi.WifiManager.SuggestionConnectionStatusListener;
 import android.net.wifi.WifiManager.TrafficStateCallback;
@@ -112,15 +112,16 @@ public class WifiManagerTest {
     @Mock TrafficStateCallback mTrafficStateCallback;
     @Mock NetworkRequestMatchCallback mNetworkRequestMatchCallback;
     @Mock OnWifiUsabilityStatsListener mOnWifiUsabilityStatsListener;
-    @Mock ScanResultsListener mScanResultListener;
     @Mock SuggestionConnectionStatusListener mListener;
-    @Mock Executor mCallbackExecutor;
+    @Mock Runnable mRunnable;
     @Mock Executor mExecutor;
+    @Mock Executor mAnotherExecutor;
 
     private Handler mHandler;
     private TestLooper mLooper;
     private WifiManager mWifiManager;
     private WifiNetworkSuggestion mWifiNetworkSuggestion;
+    private ScanResultsCallback mScanResultsCallback;
 
     @Before
     public void setUp() throws Exception {
@@ -133,6 +134,12 @@ public class WifiManagerTest {
         mWifiManager = new WifiManager(mContext, mWifiService, mLooper.getLooper());
         verify(mWifiService).getVerboseLoggingLevel();
         mWifiNetworkSuggestion = new WifiNetworkSuggestion();
+        mScanResultsCallback = new ScanResultsCallback() {
+            @Override
+            public void onScanResultsAvailable() {
+                mRunnable.run();
+            }
+        };
     }
 
     /**
@@ -1778,65 +1785,81 @@ public class WifiManagerTest {
     }
 
     /**
-     * Verify an IllegalArgumentException is thrown if listener is not provided.
+     * Verify an IllegalArgumentException is thrown if callback is not provided.
      */
     @Test(expected = IllegalArgumentException.class)
-    public void testAddScanResultsListenerWithNullListener() throws Exception {
-        mWifiManager.addScanResultsListener(mCallbackExecutor, null);
+    public void testRegisterScanResultsCallbackWithNullCallback() throws Exception {
+        mWifiManager.registerScanResultsCallback(mExecutor, null);
     }
 
     /**
      * Verify an IllegalArgumentException is thrown if executor is not provided.
      */
     @Test(expected = IllegalArgumentException.class)
-    public void testAddScanResultsListenerWithNullExecutor() throws Exception {
-        mWifiManager.addScanResultsListener(null, mScanResultListener);
+    public void testRegisterCallbackWithNullExecutor() throws Exception {
+        mWifiManager.registerScanResultsCallback(null, mScanResultsCallback);
     }
 
     /**
-     * Verify client provided listener is being called to the right listener.
+     * Verify client provided callback is being called to the right callback.
      */
     @Test
-    public void testAddScanResultsListenerAndReceiveEvent() throws Exception {
-        ArgumentCaptor<IScanResultsListener.Stub> callbackCaptor =
-                ArgumentCaptor.forClass(IScanResultsListener.Stub.class);
-        Executor executor = new SynchronousExecutor();
-        mWifiManager.addScanResultsListener(executor, mScanResultListener);
-        verify(mWifiService).registerScanResultsListener(any(IBinder.class),
-                callbackCaptor.capture(), anyInt());
+    public void testAddScanResultsCallbackAndReceiveEvent() throws Exception {
+        ArgumentCaptor<IScanResultsCallback.Stub> callbackCaptor =
+                ArgumentCaptor.forClass(IScanResultsCallback.Stub.class);
+        mWifiManager.registerScanResultsCallback(new SynchronousExecutor(), mScanResultsCallback);
+        verify(mWifiService).registerScanResultsCallback(callbackCaptor.capture());
         callbackCaptor.getValue().onScanResultsAvailable();
-        verify(mScanResultListener).onScanResultsAvailable();
+        verify(mRunnable).run();
     }
 
     /**
-     * Verify client provided listener is being called on the right executor.
+     * Verify client provided callback is being called to the right executor.
      */
     @Test
-    public void testAddScanResultsListenerWithTheTargetExecutor() throws Exception {
-        ArgumentCaptor<IScanResultsListener.Stub> callbackCaptor =
-                ArgumentCaptor.forClass(IScanResultsListener.Stub.class);
-        mWifiManager.addScanResultsListener(mExecutor, mScanResultListener);
-        verify(mWifiService).registerScanResultsListener(any(IBinder.class),
-                callbackCaptor.capture(), anyInt());
+    public void testRegisterScanResultsCallbackWithTheTargetExecutor() throws Exception {
+        ArgumentCaptor<IScanResultsCallback.Stub> callbackCaptor =
+                ArgumentCaptor.forClass(IScanResultsCallback.Stub.class);
+        mWifiManager.registerScanResultsCallback(mExecutor, mScanResultsCallback);
+        verify(mWifiService).registerScanResultsCallback(callbackCaptor.capture());
+        mWifiManager.registerScanResultsCallback(mAnotherExecutor, mScanResultsCallback);
         callbackCaptor.getValue().onScanResultsAvailable();
-        verify(mExecutor).execute(any(Runnable.class));
+        verify(mExecutor, never()).execute(any(Runnable.class));
+        verify(mAnotherExecutor).execute(any(Runnable.class));
+    }
+
+    /**
+     * Verify client register unregister then register again, to ensure callback still works.
+     */
+    @Test
+    public void testRegisterUnregisterThenRegisterAgainWithScanResultCallback() throws Exception {
+        ArgumentCaptor<IScanResultsCallback.Stub> callbackCaptor =
+                ArgumentCaptor.forClass(IScanResultsCallback.Stub.class);
+        mWifiManager.registerScanResultsCallback(new SynchronousExecutor(), mScanResultsCallback);
+        verify(mWifiService).registerScanResultsCallback(callbackCaptor.capture());
+        mWifiManager.unregisterScanResultsCallback(mScanResultsCallback);
+        callbackCaptor.getValue().onScanResultsAvailable();
+        verify(mRunnable, never()).run();
+        mWifiManager.registerScanResultsCallback(new SynchronousExecutor(), mScanResultsCallback);
+        callbackCaptor.getValue().onScanResultsAvailable();
+        verify(mRunnable).run();
     }
 
     /**
-     * Verify client removeScanResultsListener.
+     * Verify client unregisterScanResultsCallback.
      */
     @Test
-    public void testRemoveScanResultsListener() throws Exception {
-        mWifiManager.removeScanResultsListener(mScanResultListener);
-        verify(mWifiService).unregisterScanResultsListener(anyInt());
+    public void testUnregisterScanResultsCallback() throws Exception {
+        mWifiManager.unregisterScanResultsCallback(mScanResultsCallback);
+        verify(mWifiService).unregisterScanResultsCallback(any());
     }
 
     /**
-     * Verify client removeScanResultsListener with null listener will cause an exception.
+     * Verify client unregisterScanResultsCallback with null callback will cause an exception.
      */
     @Test(expected = IllegalArgumentException.class)
-    public void testRemoveScanResultsListenerWithNullListener() throws Exception {
-        mWifiManager.removeScanResultsListener(null);
+    public void testUnregisterScanResultsCallbackWithNullCallback() throws Exception {
+        mWifiManager.unregisterScanResultsCallback(null);
     }
 
     /**