User binder as identifier for scanResultcallback

Bug: 143138047 Test: atest android.net.wifi Test: atest com.android.server.wifi Change-Id: I91d68b247490ac33656decc519c39c77bfe786a4
2019-11-20 10:23:23 -08:00
parent 40ff6d65cc
commit 10bc5db45a
7 changed files with 183 additions and 83 deletions
--- 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 {
--- a/wifi/java/android/net/wifi/IScanResultsCallback.aidl
+++ 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();
+}
--- 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();
--- 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);

--- 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();
-    }
+        public abstract void onScanResultsAvailable();

-    private class ScanResultsListenerProxy extends IScanResultsListener.Stub {
-        private final Executor mExecutor;
-        private final ScanResultsListener mListener;
-
-        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();
        }
    }

--- 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,17 +513,29 @@ 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,
--- 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 removeScanResultsListener.
+     * Verify client register unregister then register again, to ensure callback still works.
     */
    @Test
-    public void testRemoveScanResultsListener() throws Exception {
-        mWifiManager.removeScanResultsListener(mScanResultListener);
-        verify(mWifiService).unregisterScanResultsListener(anyInt());
+    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 with null listener will cause an exception.
+     * Verify client unregisterScanResultsCallback.
+     */
+    @Test
+    public void testUnregisterScanResultsCallback() throws Exception {
+        mWifiManager.unregisterScanResultsCallback(mScanResultsCallback);
+        verify(mWifiService).unregisterScanResultsCallback(any());
+    }
+
+    /**
+     * 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);
    }

    /**