Merge changes I3250bdb9,I20f6e5e1,I70c96a09,I88f77bd4,If6cefa51, ...
* changes: ethernet: rename methods in EthernetNetworkFactory ethernet: move registerNetworkOffer into its own function ethernet: provider to keep track of requestId ethernet: return result when removing interface ethernet: enable testCallbacks_forServerModeInterfaces on CF ethernet: expand server mode callbacks test ethernet: add test for removing interface while in server mode
This commit is contained in:
@@ -60,7 +60,9 @@ import java.util.Set;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
|
||||
/**
|
||||
* {@link NetworkProvider} that manages NetworkOffers for Ethernet networks.
|
||||
* Class that manages NetworkOffers for Ethernet networks.
|
||||
*
|
||||
* TODO: this class should be merged into EthernetTracker.
|
||||
*/
|
||||
public class EthernetNetworkFactory {
|
||||
private final static String TAG = EthernetNetworkFactory.class.getSimpleName();
|
||||
@@ -221,11 +223,17 @@ public class EthernetNetworkFactory {
|
||||
}
|
||||
|
||||
@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
|
||||
protected void removeInterface(String interfaceName) {
|
||||
protected boolean removeInterface(String interfaceName) {
|
||||
NetworkInterfaceState iface = mTrackingInterfaces.remove(interfaceName);
|
||||
if (iface != null) {
|
||||
iface.destroy();
|
||||
iface.unregisterNetworkOfferAndStop();
|
||||
return true;
|
||||
}
|
||||
// TODO(b/236892130): if an interface is currently in server mode, it may not be properly
|
||||
// removed.
|
||||
// TODO: when false is returned, do not send a STATE_ABSENT callback.
|
||||
Log.w(TAG, interfaceName + " is not tracked and cannot be removed");
|
||||
return false;
|
||||
}
|
||||
|
||||
/** Returns true if state has been modified */
|
||||
@@ -292,7 +300,7 @@ public class EthernetNetworkFactory {
|
||||
private boolean mLinkUp;
|
||||
private int mLegacyType;
|
||||
private LinkProperties mLinkProperties = new LinkProperties();
|
||||
private Set<NetworkRequest> mRequests = new ArraySet<>();
|
||||
private final Set<Integer> mRequestIds = new ArraySet<>();
|
||||
|
||||
private volatile @Nullable IpClientManager mIpClient;
|
||||
private @NonNull NetworkCapabilities mCapabilities;
|
||||
@@ -401,7 +409,7 @@ public class EthernetNetworkFactory {
|
||||
// existing requests.
|
||||
// ConnectivityService filters requests for us based on the NetworkCapabilities
|
||||
// passed in the registerNetworkOffer() call.
|
||||
mRequests.add(request);
|
||||
mRequestIds.add(request.requestId);
|
||||
// if the network is already started, this is a no-op.
|
||||
start();
|
||||
}
|
||||
@@ -412,8 +420,12 @@ public class EthernetNetworkFactory {
|
||||
Log.d(TAG,
|
||||
String.format("%s: onNetworkUnneeded for request: %s", name, request));
|
||||
}
|
||||
mRequests.remove(request);
|
||||
if (mRequests.isEmpty()) {
|
||||
if (!mRequestIds.remove(request.requestId)) {
|
||||
// This can only happen if onNetworkNeeded was not called for a request or if
|
||||
// the requestId changed. Both should *never* happen.
|
||||
Log.wtf(TAG, "onNetworkUnneeded called for unknown request");
|
||||
}
|
||||
if (mRequestIds.isEmpty()) {
|
||||
// not currently serving any requests, stop the network.
|
||||
stop();
|
||||
}
|
||||
@@ -454,7 +466,7 @@ public class EthernetNetworkFactory {
|
||||
+ "transport type.");
|
||||
}
|
||||
|
||||
private static NetworkScore getBestNetworkScore() {
|
||||
private static NetworkScore getNetworkScore() {
|
||||
return new NetworkScore.Builder().build();
|
||||
}
|
||||
|
||||
@@ -465,9 +477,7 @@ public class EthernetNetworkFactory {
|
||||
if (mLinkUp) {
|
||||
// registering a new network offer will update the existing one, not install a
|
||||
// new one.
|
||||
mNetworkProvider.registerNetworkOffer(getBestNetworkScore(),
|
||||
new NetworkCapabilities(capabilities), cmd -> mHandler.post(cmd),
|
||||
mNetworkOfferCallback);
|
||||
registerNetworkOffer();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -629,14 +639,12 @@ public class EthernetNetworkFactory {
|
||||
|
||||
if (!up) { // was up, goes down
|
||||
// retract network offer and stop IpClient.
|
||||
destroy();
|
||||
unregisterNetworkOfferAndStop();
|
||||
// If only setting the interface down, send a callback to signal completion.
|
||||
EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, name, null);
|
||||
} else { // was down, goes up
|
||||
// register network offer
|
||||
mNetworkProvider.registerNetworkOffer(getBestNetworkScore(),
|
||||
new NetworkCapabilities(mCapabilities), (cmd) -> mHandler.post(cmd),
|
||||
mNetworkOfferCallback);
|
||||
registerNetworkOffer();
|
||||
}
|
||||
|
||||
return true;
|
||||
@@ -660,10 +668,16 @@ public class EthernetNetworkFactory {
|
||||
mLinkProperties.clear();
|
||||
}
|
||||
|
||||
public void destroy() {
|
||||
private void registerNetworkOffer() {
|
||||
mNetworkProvider.registerNetworkOffer(getNetworkScore(),
|
||||
new NetworkCapabilities(mCapabilities), cmd -> mHandler.post(cmd),
|
||||
mNetworkOfferCallback);
|
||||
}
|
||||
|
||||
public void unregisterNetworkOfferAndStop() {
|
||||
mNetworkProvider.unregisterNetworkOffer(mNetworkOfferCallback);
|
||||
stop();
|
||||
mRequests.clear();
|
||||
mRequestIds.clear();
|
||||
}
|
||||
|
||||
private static void provisionIpClient(@NonNull final IpClientManager ipClient,
|
||||
|
||||
@@ -49,7 +49,6 @@ import android.os.Build
|
||||
import android.os.Handler
|
||||
import android.os.Looper
|
||||
import android.os.OutcomeReceiver
|
||||
import android.os.SystemProperties
|
||||
import android.platform.test.annotations.AppModeFull
|
||||
import android.util.ArraySet
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
@@ -69,13 +68,13 @@ import com.android.testutils.runAsShell
|
||||
import com.android.testutils.waitForIdle
|
||||
import org.junit.After
|
||||
import org.junit.Assume.assumeTrue
|
||||
import org.junit.Assume.assumeFalse
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import java.net.Inet6Address
|
||||
import java.util.concurrent.CompletableFuture
|
||||
import java.util.concurrent.ExecutionException
|
||||
import java.util.concurrent.TimeoutException
|
||||
import java.util.concurrent.TimeUnit
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFailsWith
|
||||
@@ -206,12 +205,8 @@ class EthernetManagerTest {
|
||||
|
||||
fun eventuallyExpect(expected: CallbackEntry) = events.poll(TIMEOUT_MS) { it == expected }
|
||||
|
||||
fun eventuallyExpect(interfaceName: String, state: Int, role: Int) {
|
||||
assertNotNull(eventuallyExpect(createChangeEvent(interfaceName, state, role)))
|
||||
}
|
||||
|
||||
fun eventuallyExpect(iface: EthernetTestInterface, state: Int, role: Int) {
|
||||
eventuallyExpect(iface.name, state, role)
|
||||
assertNotNull(eventuallyExpect(createChangeEvent(iface.name, state, role)))
|
||||
}
|
||||
|
||||
fun assertNoCallback() {
|
||||
@@ -458,32 +453,45 @@ class EthernetManagerTest {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: this function is now used in two places (EthernetManagerTest and
|
||||
// EthernetTetheringTest), so it should be moved to testutils.
|
||||
private fun isAdbOverNetwork(): Boolean {
|
||||
// If adb TCP port opened, this test may running by adb over network.
|
||||
return (SystemProperties.getInt("persist.adb.tcp.port", -1) > -1 ||
|
||||
SystemProperties.getInt("service.adb.tcp.port", -1) > -1)
|
||||
private fun assumeNoInterfaceForTetheringAvailable() {
|
||||
// Interfaces that have configured NetworkCapabilities will never be used for tethering,
|
||||
// see aosp/2123900.
|
||||
try {
|
||||
// assumeException does not exist.
|
||||
requestTetheredInterface().expectOnAvailable()
|
||||
// interface used for tethering is available, throw an assumption error.
|
||||
assumeTrue(false)
|
||||
} catch (e: TimeoutException) {
|
||||
// do nothing -- the TimeoutException indicates that no interface is available for
|
||||
// tethering.
|
||||
releaseTetheredInterface()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testCallbacks_forServerModeInterfaces() {
|
||||
// do not run this test when adb might be connected over ethernet.
|
||||
assumeFalse(isAdbOverNetwork())
|
||||
// do not run this test if an interface that can be used for tethering already exists.
|
||||
assumeNoInterfaceForTetheringAvailable()
|
||||
|
||||
val iface = createInterface()
|
||||
requestTetheredInterface().expectOnAvailable()
|
||||
|
||||
val listener = EthernetStateListener()
|
||||
addInterfaceStateListener(listener)
|
||||
|
||||
// it is possible that a physical interface is present, so it is not guaranteed that iface
|
||||
// will be put into server mode. This should not matter for the test though. Calling
|
||||
// createInterface() makes sure we have at least one interface available.
|
||||
val iface = createInterface()
|
||||
val cb = requestTetheredInterface()
|
||||
val ifaceName = cb.expectOnAvailable()
|
||||
listener.eventuallyExpect(ifaceName, STATE_LINK_UP, ROLE_SERVER)
|
||||
// TODO(b/236895792): THIS IS A BUG! Existing server mode interfaces are not reported when
|
||||
// an InterfaceStateListener is registered.
|
||||
// Note: using eventuallyExpect as there may be other interfaces present.
|
||||
// listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_SERVER)
|
||||
|
||||
releaseTetheredInterface()
|
||||
listener.eventuallyExpect(ifaceName, STATE_LINK_UP, ROLE_CLIENT)
|
||||
listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
|
||||
|
||||
requestTetheredInterface().expectOnAvailable()
|
||||
// This should be changed to expectCallback, once b/236895792 is fixed.
|
||||
listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_SERVER)
|
||||
|
||||
releaseTetheredInterface()
|
||||
listener.expectCallback(iface, STATE_LINK_UP, ROLE_CLIENT)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -651,4 +659,26 @@ class EthernetManagerTest {
|
||||
iface.setCarrierEnabled(false)
|
||||
cb.eventuallyExpectLost()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testRemoveInterface_whileInServerMode() {
|
||||
assumeNoInterfaceForTetheringAvailable()
|
||||
|
||||
val listener = EthernetStateListener()
|
||||
addInterfaceStateListener(listener)
|
||||
|
||||
val iface = createInterface()
|
||||
val ifaceName = requestTetheredInterface().expectOnAvailable()
|
||||
|
||||
assertEquals(iface.name, ifaceName)
|
||||
listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_SERVER)
|
||||
|
||||
removeInterface(iface)
|
||||
|
||||
// Note: removeInterface already verifies that a STATE_ABSENT, ROLE_NONE callback is
|
||||
// received, but it can't hurt to explicitly check for it.
|
||||
listener.expectCallback(iface, STATE_ABSENT, ROLE_NONE)
|
||||
releaseTetheredInterface()
|
||||
listener.assertNoCallback()
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user