Commit faf41677 authored by Paul Jensen's avatar Paul Jensen Committed by Commit Bot

Make NetorkChangeNotifierAndroid not use WiFi SSID.

Previously the SSID was used to identify networks, this poses several
problems however:
1. It's not precise enough as different networks can have the same SSID.
2. Accessing the SSID can be hinged on having location permission.
Instead this CL changes network identification to use the Network
objects for Android Marshmallow and later releases.  It might be
possible to use Network objects in Android Lollipop as well, but
Android Marshmallow added an efficient API for getting the default
Network.

Bug: 825939
Change-Id: Iff8c89a84c548693d7a8b0366003676720dfe1d0
Reviewed-on: https://chromium-review.googlesource.com/980967
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548040}
parent 5bcdfd9c
...@@ -57,16 +57,15 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -57,16 +57,15 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
private final boolean mConnected; private final boolean mConnected;
private final int mType; private final int mType;
private final int mSubtype; private final int mSubtype;
// WIFI SSID of the connection. Always non-null (i.e. instead of null it'll be an empty // WIFI SSID of the connection on pre-Marshmallow, NetID starting with Marshmallow. Always
// string) to facilitate .equals(). // non-null (i.e. instead of null it'll be an empty string) to facilitate .equals().
private final String mWifiSsid; private final String mNetworkIdentifier;
public NetworkState(boolean connected, int type, int subtype, String wifiSsid) { public NetworkState(boolean connected, int type, int subtype, String networkIdentifier) {
mConnected = connected; mConnected = connected;
mType = type; mType = type;
mSubtype = subtype; mSubtype = subtype;
assert mType == ConnectivityManager.TYPE_WIFI || wifiSsid == null; mNetworkIdentifier = networkIdentifier == null ? "" : networkIdentifier;
mWifiSsid = wifiSsid == null ? "" : wifiSsid;
} }
public boolean isConnected() { public boolean isConnected() {
...@@ -81,9 +80,9 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -81,9 +80,9 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
return mSubtype; return mSubtype;
} }
// WiFi SSID, always non-null to facilitate .equals() // Always non-null to facilitate .equals().
public String getWifiSsid() { public String getNetworkIdentifier() {
return mWifiSsid; return mNetworkIdentifier;
} }
/** /**
...@@ -169,11 +168,11 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -169,11 +168,11 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
} }
/** /**
* @param networkInfo The NetworkInfo for the active network.
* @return the info of the network that is available to this app. * @return the info of the network that is available to this app.
*/ */
@TargetApi(Build.VERSION_CODES.LOLLIPOP) @TargetApi(Build.VERSION_CODES.LOLLIPOP)
private NetworkInfo getActiveNetworkInfo() { private NetworkInfo processActiveNetworkInfo(NetworkInfo networkInfo) {
final NetworkInfo networkInfo = mConnectivityManager.getActiveNetworkInfo();
if (networkInfo == null) { if (networkInfo == null) {
return null; return null;
} }
...@@ -209,10 +208,23 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -209,10 +208,23 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
* default network. * default network.
*/ */
NetworkState getNetworkState(WifiManagerDelegate wifiManagerDelegate) { NetworkState getNetworkState(WifiManagerDelegate wifiManagerDelegate) {
final NetworkInfo networkInfo = getActiveNetworkInfo(); Network network = null;
NetworkInfo networkInfo;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
network = getDefaultNetwork();
networkInfo = mConnectivityManager.getNetworkInfo(network);
} else {
networkInfo = mConnectivityManager.getActiveNetworkInfo();
}
networkInfo = processActiveNetworkInfo(networkInfo);
if (networkInfo == null) { if (networkInfo == null) {
return new NetworkState(false, -1, -1, null); return new NetworkState(false, -1, -1, null);
} }
if (network != null) {
return new NetworkState(true, networkInfo.getType(), networkInfo.getSubtype(),
String.valueOf(networkToNetId(network)));
}
assert Build.VERSION.SDK_INT < Build.VERSION_CODES.M;
// If Wifi, then fetch SSID also // If Wifi, then fetch SSID also
if (networkInfo.getType() == ConnectivityManager.TYPE_WIFI) { if (networkInfo.getType() == ConnectivityManager.TYPE_WIFI) {
// Since Android 4.2 the SSID can be retrieved from NetworkInfo.getExtraInfo(). // Since Android 4.2 the SSID can be retrieved from NetworkInfo.getExtraInfo().
...@@ -333,22 +345,29 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -333,22 +345,29 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
} }
/** /**
* Returns the NetID of the current default network. Returns * Returns the current default {@link Network}, or {@code null} if disconnected.
* NetId.INVALID if no current default network connected.
* Only callable on Lollipop and newer releases. * Only callable on Lollipop and newer releases.
*/ */
@TargetApi(Build.VERSION_CODES.LOLLIPOP) @TargetApi(Build.VERSION_CODES.LOLLIPOP)
long getDefaultNetId() { Network getDefaultNetwork() {
Network defaultNetwork = null;
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
defaultNetwork = mConnectivityManager.getActiveNetwork();
// getActiveNetwork() returning null cannot be trusted to indicate disconnected
// as it suffers from https://crbug.com/677365.
if (defaultNetwork != null) {
return defaultNetwork;
}
}
// Android Lollipop had no API to get the default network; only an // Android Lollipop had no API to get the default network; only an
// API to return the NetworkInfo for the default network. To // API to return the NetworkInfo for the default network. To
// determine the default network one can find the network with // determine the default network one can find the network with
// type matching that of the default network. // type matching that of the default network.
final NetworkInfo defaultNetworkInfo = mConnectivityManager.getActiveNetworkInfo(); final NetworkInfo defaultNetworkInfo = mConnectivityManager.getActiveNetworkInfo();
if (defaultNetworkInfo == null) { if (defaultNetworkInfo == null) {
return NetId.INVALID; return null;
} }
final Network[] networks = getAllNetworksFiltered(this, null); final Network[] networks = getAllNetworksFiltered(this, null);
long defaultNetId = NetId.INVALID;
for (Network network : networks) { for (Network network : networks) {
final NetworkInfo networkInfo = getNetworkInfo(network); final NetworkInfo networkInfo = getNetworkInfo(network);
if (networkInfo != null if (networkInfo != null
...@@ -363,13 +382,12 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -363,13 +382,12 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
// There should not be multiple connected networks of the // There should not be multiple connected networks of the
// same type. At least as of Android Marshmallow this is // same type. At least as of Android Marshmallow this is
// not supported. If this becomes supported this assertion // not supported. If this becomes supported this assertion
// may trigger. At that point ConnectivityManager.getDefaultNetwork() // may trigger.
// could be used though it's only available with Android Marshmallow. assert defaultNetwork == null;
assert defaultNetId == NetId.INVALID; defaultNetwork = network;
defaultNetId = networkToNetId(network);
} }
} }
return defaultNetId; return defaultNetwork;
} }
} }
...@@ -389,6 +407,8 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -389,6 +407,8 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
private WifiManager mWifiManager; private WifiManager mWifiManager;
WifiManagerDelegate(Context context) { WifiManagerDelegate(Context context) {
// Getting SSID requires more permissions in later Android releases.
assert Build.VERSION.SDK_INT < Build.VERSION_CODES.M;
mContext = context; mContext = context;
} }
...@@ -738,7 +758,9 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -738,7 +758,9 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
mObserver = observer; mObserver = observer;
mConnectivityManagerDelegate = mConnectivityManagerDelegate =
new ConnectivityManagerDelegate(ContextUtils.getApplicationContext()); new ConnectivityManagerDelegate(ContextUtils.getApplicationContext());
mWifiManagerDelegate = new WifiManagerDelegate(ContextUtils.getApplicationContext()); if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
mWifiManagerDelegate = new WifiManagerDelegate(ContextUtils.getApplicationContext());
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
mNetworkCallback = new MyNetworkCallback(); mNetworkCallback = new MyNetworkCallback();
mNetworkRequest = new NetworkRequest.Builder() mNetworkRequest = new NetworkRequest.Builder()
...@@ -946,7 +968,8 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -946,7 +968,8 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) { if (Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) {
return NetId.INVALID; return NetId.INVALID;
} }
return mConnectivityManagerDelegate.getDefaultNetId(); Network network = mConnectivityManagerDelegate.getDefaultNetwork();
return network == null ? NetId.INVALID : networkToNetId(network);
} }
/** /**
...@@ -1023,7 +1046,8 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver { ...@@ -1023,7 +1046,8 @@ public class NetworkChangeNotifierAutoDetect extends BroadcastReceiver {
private void connectionTypeChanged() { private void connectionTypeChanged() {
NetworkState networkState = getCurrentNetworkState(); NetworkState networkState = getCurrentNetworkState();
if (networkState.getConnectionType() != mNetworkState.getConnectionType() if (networkState.getConnectionType() != mNetworkState.getConnectionType()
|| !networkState.getWifiSsid().equals(mNetworkState.getWifiSsid())) { || !networkState.getNetworkIdentifier().equals(
mNetworkState.getNetworkIdentifier())) {
mObserver.onConnectionTypeChanged(networkState.getConnectionType()); mObserver.onConnectionTypeChanged(networkState.getConnectionType());
} }
if (networkState.getConnectionType() != mNetworkState.getConnectionType() if (networkState.getConnectionType() != mNetworkState.getConnectionType()
......
...@@ -231,8 +231,8 @@ public class NetworkChangeNotifierTest { ...@@ -231,8 +231,8 @@ public class NetworkChangeNotifierTest {
// Dummy implementations to avoid NullPointerExceptions in default implementations: // Dummy implementations to avoid NullPointerExceptions in default implementations:
@Override @Override
public long getDefaultNetId() { public Network getDefaultNetwork() {
return NetId.INVALID; return null;
} }
@Override @Override
...@@ -710,8 +710,12 @@ public class NetworkChangeNotifierTest { ...@@ -710,8 +710,12 @@ public class NetworkChangeNotifierTest {
public void testConnectivityManagerDelegateDoesNotCrash() { public void testConnectivityManagerDelegateDoesNotCrash() {
ConnectivityManagerDelegate delegate = ConnectivityManagerDelegate delegate =
new ConnectivityManagerDelegate(InstrumentationRegistry.getTargetContext()); new ConnectivityManagerDelegate(InstrumentationRegistry.getTargetContext());
delegate.getNetworkState( if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
new WifiManagerDelegate(InstrumentationRegistry.getTargetContext())); delegate.getNetworkState(null);
} else {
delegate.getNetworkState(
new WifiManagerDelegate(InstrumentationRegistry.getTargetContext()));
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
// getConnectionType(Network) doesn't crash upon invalid Network argument. // getConnectionType(Network) doesn't crash upon invalid Network argument.
Network invalidNetwork = Helper.netIdToNetwork(NetId.INVALID); Network invalidNetwork = Helper.netIdToNetwork(NetId.INVALID);
...@@ -723,7 +727,7 @@ public class NetworkChangeNotifierTest { ...@@ -723,7 +727,7 @@ public class NetworkChangeNotifierTest {
if (networks.length >= 1) { if (networks.length >= 1) {
delegate.getConnectionType(networks[0]); delegate.getConnectionType(networks[0]);
} }
delegate.getDefaultNetId(); delegate.getDefaultNetwork();
NetworkCallback networkCallback = new NetworkCallback(); NetworkCallback networkCallback = new NetworkCallback();
NetworkRequest networkRequest = new NetworkRequest.Builder().build(); NetworkRequest networkRequest = new NetworkRequest.Builder().build();
delegate.registerNetworkCallback(networkRequest, networkCallback); delegate.registerNetworkCallback(networkRequest, networkCallback);
...@@ -786,8 +790,8 @@ public class NetworkChangeNotifierTest { ...@@ -786,8 +790,8 @@ public class NetworkChangeNotifierTest {
} }
@Override @Override
long getDefaultNetId() { Network getDefaultNetwork() {
return Integer.parseInt(mNetworks[1].toString()); return mNetworks[1];
} }
@Override @Override
...@@ -803,7 +807,7 @@ public class NetworkChangeNotifierTest { ...@@ -803,7 +807,7 @@ public class NetworkChangeNotifierTest {
// Verify that the mock delegate connectivity manager is being used // Verify that the mock delegate connectivity manager is being used
// by the network change notifier auto-detector. // by the network change notifier auto-detector.
Assert.assertEquals(333, ncn.getDefaultNetId()); Assert.assertEquals(333, demungeNetId(ncn.getDefaultNetId()));
// The api {@link NetworkChangeNotifierAutoDetect#getNetworksAndTypes()} // The api {@link NetworkChangeNotifierAutoDetect#getNetworksAndTypes()}
// returns an array of a repeated sequence of: (NetID, ConnectionType). // returns an array of a repeated sequence of: (NetID, ConnectionType).
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment