From 9e225540687d5d4eb17bdfbe4f062c22ee22b560 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Thu, 8 Jun 2017 17:48:48 +0900 Subject: [PATCH] Delete legacy "wifi interface name guessing" behaviour Now that Wi-Fi always passes us the AP interface name (and mode) we no longer need to guess which interface on which we're supposed to be starting IP serving (either tethering or local-only hotspot). Test: as follows - built - flashed - booted - TetheringTest passes Bug: 32163131 Bug: 62343300 Change-Id: I6019410ee5adff4929690d35ba09294765fcd6a4 --- .../server/connectivity/Tethering.java | 17 +--- .../tethering/TetheringConfiguration.java | 3 + .../server/connectivity/TetheringTest.java | 94 +++++++++++-------- 3 files changed, 62 insertions(+), 52 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java index 4b71cc15abd6d..3f825c58e6ea9 100644 --- a/services/core/java/com/android/server/connectivity/Tethering.java +++ b/services/core/java/com/android/server/connectivity/Tethering.java @@ -859,23 +859,16 @@ public class Tethering extends BaseNetworkObserver implements IControlsTethering ipServingMode = IControlsTethering.STATE_LOCAL_ONLY; break; default: - // Resort to legacy "guessing" behaviour. - // - // When the AP comes up and we've been requested to tether it, - // do so. Otherwise, assume it's a local-only hotspot request. - // - // TODO: Once all AP broadcasts are known to include ifname and - // mode information delete this code path and log an error. - ipServingMode = mWifiTetherRequested - ? IControlsTethering.STATE_TETHERED - : IControlsTethering.STATE_LOCAL_ONLY; - break; + mLog.e("Cannot enable IP serving in unknown WiFi mode: " + wifiIpMode); + return; } if (!TextUtils.isEmpty(ifname)) { changeInterfaceState(ifname, ipServingMode); } else { - tetherMatchingInterfaces(ipServingMode, ConnectivityManager.TETHERING_WIFI); + mLog.e(String.format( + "Cannot enable IP serving in mode %s on missing interface name", + ipServingMode)); } } diff --git a/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java b/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java index 69411936eb1a0..e87d860b501ab 100644 --- a/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java +++ b/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java @@ -77,6 +77,9 @@ public class TetheringConfiguration { public TetheringConfiguration(Context ctx) { tetherableUsbRegexs = ctx.getResources().getStringArray( com.android.internal.R.array.config_tether_usb_regexs); + // TODO: Evaluate deleting this altogether now that Wi-Fi always passes + // us an interface name. Careful consideration needs to be given to + // implications for Settings and for provisioning checks. tetherableWifiRegexs = ctx.getResources().getStringArray( com.android.internal.R.array.config_tether_wifi_regexs); tetherableBluetoothRegexs = ctx.getResources().getStringArray( diff --git a/tests/net/java/com/android/server/connectivity/TetheringTest.java b/tests/net/java/com/android/server/connectivity/TetheringTest.java index 50d7f043dfdac..281223e1fe587 100644 --- a/tests/net/java/com/android/server/connectivity/TetheringTest.java +++ b/tests/net/java/com/android/server/connectivity/TetheringTest.java @@ -219,10 +219,7 @@ public class TetheringTest { mServiceContext.sendStickyBroadcastAsUser(intent, UserHandle.ALL); } - private void verifyInterfaceServingModeStarted(boolean ifnameKnown) throws Exception { - if (!ifnameKnown) { - verify(mNMService, times(1)).listInterfaces(); - } + private void verifyInterfaceServingModeStarted() throws Exception { verify(mNMService, times(1)).getInterfaceConfig(mTestIfname); verify(mNMService, times(1)) .setInterfaceConfig(eq(mTestIfname), any(InterfaceConfiguration.class)); @@ -238,21 +235,36 @@ public class TetheringTest { mIntents.remove(bcast); } - public void workingLocalOnlyHotspot(boolean enrichedApBroadcast) throws Exception { + @Test + public void failingLocalOnlyHotspotLegacyApBroadcast() throws Exception { when(mConnectivityManager.isTetheringSupported()).thenReturn(true); // Emulate externally-visible WifiManager effects, causing the // per-interface state machine to start up, and telling us that // hotspot mode is to be started. mTethering.interfaceStatusChanged(mTestIfname, true); - if (enrichedApBroadcast) { - sendWifiApStateChanged(WIFI_AP_STATE_ENABLED, mTestIfname, IFACE_IP_MODE_LOCAL_ONLY); - } else { - sendWifiApStateChanged(WIFI_AP_STATE_ENABLED); - } + sendWifiApStateChanged(WIFI_AP_STATE_ENABLED); mLooper.dispatchAll(); - verifyInterfaceServingModeStarted(enrichedApBroadcast); + verify(mConnectivityManager, atLeastOnce()).isTetheringSupported(); + verifyTetheringBroadcast(mTestIfname, ConnectivityManager.EXTRA_AVAILABLE_TETHER); + verifyNoMoreInteractions(mConnectivityManager); + verifyNoMoreInteractions(mNMService); + verifyNoMoreInteractions(mWifiManager); + } + + @Test + public void workingLocalOnlyHotspotEnrichedApBroadcast() throws Exception { + when(mConnectivityManager.isTetheringSupported()).thenReturn(true); + + // Emulate externally-visible WifiManager effects, causing the + // per-interface state machine to start up, and telling us that + // hotspot mode is to be started. + mTethering.interfaceStatusChanged(mTestIfname, true); + sendWifiApStateChanged(WIFI_AP_STATE_ENABLED, mTestIfname, IFACE_IP_MODE_LOCAL_ONLY); + mLooper.dispatchAll(); + + verifyInterfaceServingModeStarted(); verifyTetheringBroadcast(mTestIfname, ConnectivityManager.EXTRA_AVAILABLE_TETHER); verify(mNMService, times(1)).setIpForwardingEnabled(true); verify(mNMService, times(1)).startTethering(any(String[].class)); @@ -293,16 +305,7 @@ public class TetheringTest { } @Test - public void workingLocalOnlyHotspotLegacyApBroadcast() throws Exception { - workingLocalOnlyHotspot(false); - } - - @Test - public void workingLocalOnlyHotspotEnrichedApBroadcast() throws Exception { - workingLocalOnlyHotspot(true); - } - - public void workingWifiTethering(boolean enrichedApBroadcast) throws Exception { + public void failingWifiTetheringLegacyApBroadcast() throws Exception { when(mConnectivityManager.isTetheringSupported()).thenReturn(true); when(mWifiManager.startSoftAp(any(WifiConfiguration.class))).thenReturn(true); @@ -318,14 +321,37 @@ public class TetheringTest { // per-interface state machine to start up, and telling us that // tethering mode is to be started. mTethering.interfaceStatusChanged(mTestIfname, true); - if (enrichedApBroadcast) { - sendWifiApStateChanged(WIFI_AP_STATE_ENABLED, mTestIfname, IFACE_IP_MODE_TETHERED); - } else { - sendWifiApStateChanged(WIFI_AP_STATE_ENABLED); - } + sendWifiApStateChanged(WIFI_AP_STATE_ENABLED); mLooper.dispatchAll(); - verifyInterfaceServingModeStarted(enrichedApBroadcast); + verify(mConnectivityManager, atLeastOnce()).isTetheringSupported(); + verifyTetheringBroadcast(mTestIfname, ConnectivityManager.EXTRA_AVAILABLE_TETHER); + verifyNoMoreInteractions(mConnectivityManager); + verifyNoMoreInteractions(mNMService); + verifyNoMoreInteractions(mWifiManager); + } + + @Test + public void workingWifiTetheringEnrichedApBroadcast() throws Exception { + when(mConnectivityManager.isTetheringSupported()).thenReturn(true); + when(mWifiManager.startSoftAp(any(WifiConfiguration.class))).thenReturn(true); + + // Emulate pressing the WiFi tethering button. + mTethering.startTethering(ConnectivityManager.TETHERING_WIFI, null, false); + mLooper.dispatchAll(); + verify(mWifiManager, times(1)).startSoftAp(null); + verifyNoMoreInteractions(mWifiManager); + verifyNoMoreInteractions(mConnectivityManager); + verifyNoMoreInteractions(mNMService); + + // Emulate externally-visible WifiManager effects, causing the + // per-interface state machine to start up, and telling us that + // tethering mode is to be started. + mTethering.interfaceStatusChanged(mTestIfname, true); + sendWifiApStateChanged(WIFI_AP_STATE_ENABLED, mTestIfname, IFACE_IP_MODE_TETHERED); + mLooper.dispatchAll(); + + verifyInterfaceServingModeStarted(); verifyTetheringBroadcast(mTestIfname, ConnectivityManager.EXTRA_AVAILABLE_TETHER); verify(mNMService, times(1)).setIpForwardingEnabled(true); verify(mNMService, times(1)).startTethering(any(String[].class)); @@ -386,16 +412,6 @@ public class TetheringTest { mTethering.getLastTetherError(mTestIfname)); } - @Test - public void workingWifiTetheringLegacyApBroadcast() throws Exception { - workingWifiTethering(false); - } - - @Test - public void workingWifiTetheringEnrichedApBroadcast() throws Exception { - workingWifiTethering(true); - } - @Test public void failureEnablingIpForwarding() throws Exception { when(mConnectivityManager.isTetheringSupported()).thenReturn(true); @@ -414,11 +430,9 @@ public class TetheringTest { // per-interface state machine to start up, and telling us that // tethering mode is to be started. mTethering.interfaceStatusChanged(mTestIfname, true); - sendWifiApStateChanged(WifiManager.WIFI_AP_STATE_ENABLED); + sendWifiApStateChanged(WIFI_AP_STATE_ENABLED, mTestIfname, IFACE_IP_MODE_TETHERED); mLooper.dispatchAll(); - // Activity caused by test_wlan0 becoming available. - verify(mNMService, times(1)).listInterfaces(); // We verify get/set called twice here: once for setup and once during // teardown because all events happen over the course of the single // dispatchAll() above.