Merge changes from topic "remove-ethernet-shims"

* changes:
  ethernet: add test for tethered interface callbacks
  ethernet: stop using EthernetManager shims
  ethernet: broadcast state change for server interfaces
  ethernet: increase timeout to deflake tests

service-t/src/com/android/server/ethernet/EthernetTracker.java

@@ -228,7 +228,7 @@ public class EthernetTracker {
      */
     protected void broadcastInterfaceStateChange(@NonNull String iface) {
         ensureRunningOnEthernetServiceThread();
-        final int state = mFactory.getInterfaceState(iface);
+        final int state = getInterfaceState(iface);
         final int role = getInterfaceRole(iface);
         final IpConfiguration config = getIpConfigurationForCallback(iface, state);
         final int n = mListeners.beginBroadcast();
@@ -435,15 +435,34 @@ public class EthernetTracker {
         if (mDefaultInterface != null) {
             removeInterface(mDefaultInterface);
             addInterface(mDefaultInterface);
+            // when this broadcast is sent, any calls to notifyTetheredInterfaceAvailable or
+            // notifyTetheredInterfaceUnavailable have already happened
+            broadcastInterfaceStateChange(mDefaultInterface);
         }
     }
 
+    private int getInterfaceState(final String iface) {
+        if (mFactory.hasInterface(iface)) {
+            return mFactory.getInterfaceState(iface);
+        }
+        if (getInterfaceMode(iface) == INTERFACE_MODE_SERVER) {
+            // server mode interfaces are not tracked by the factory.
+            // TODO(b/234743836): interface state for server mode interfaces is not tracked
+            // properly; just return link up.
+            return EthernetManager.STATE_LINK_UP;
+        }
+        return EthernetManager.STATE_ABSENT;
+    }
+
     private int getInterfaceRole(final String iface) {
-        if (!mFactory.hasInterface(iface)) return EthernetManager.ROLE_NONE;
+        if (mFactory.hasInterface(iface)) {
-        final int mode = getInterfaceMode(iface);
+            // only client mode interfaces are tracked by the factory.
-        return (mode == INTERFACE_MODE_CLIENT)
+            return EthernetManager.ROLE_CLIENT;
-                ? EthernetManager.ROLE_CLIENT
+        }
-                : EthernetManager.ROLE_SERVER;
+        if (getInterfaceMode(iface) == INTERFACE_MODE_SERVER) {
+            return EthernetManager.ROLE_SERVER;
+        }
+        return EthernetManager.ROLE_NONE;
     }
 
     private int getInterfaceMode(final String iface) {

tests/cts/net/src/android/net/cts/EthernetManagerTest.kt

@@ -20,6 +20,16 @@ import android.Manifest.permission.MANAGE_TEST_NETWORKS
 import android.Manifest.permission.NETWORK_SETTINGS
 import android.content.Context
 import android.net.ConnectivityManager
+import android.net.EthernetManager
+import android.net.EthernetManager.InterfaceStateListener
+import android.net.EthernetManager.ROLE_CLIENT
+import android.net.EthernetManager.ROLE_NONE
+import android.net.EthernetManager.ROLE_SERVER
+import android.net.EthernetManager.STATE_ABSENT
+import android.net.EthernetManager.STATE_LINK_DOWN
+import android.net.EthernetManager.STATE_LINK_UP
+import android.net.EthernetManager.TetheredInterfaceCallback
+import android.net.EthernetManager.TetheredInterfaceRequest
 import android.net.EthernetNetworkSpecifier
 import android.net.InetAddresses
 import android.net.IpConfiguration
@@ -32,46 +42,46 @@ import android.net.NetworkRequest
 import android.net.TestNetworkInterface
 import android.net.TestNetworkManager
 import android.net.cts.EthernetManagerTest.EthernetStateListener.CallbackEntry.InterfaceStateChanged
+import android.os.Build
 import android.os.Handler
 import android.os.HandlerExecutor
 import android.os.Looper
+import android.os.SystemProperties
 import android.platform.test.annotations.AppModeFull
 import android.util.ArraySet
 import androidx.test.platform.app.InstrumentationRegistry
-import androidx.test.runner.AndroidJUnit4
 import com.android.net.module.util.ArrayTrackRecord
 import com.android.net.module.util.TrackRecord
-import com.android.networkstack.apishim.EthernetManagerShimImpl
-import com.android.networkstack.apishim.common.EthernetManagerShim.InterfaceStateListener
-import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_CLIENT
-import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_NONE
-import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_ABSENT
-import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_LINK_DOWN
-import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_LINK_UP
 import com.android.testutils.anyNetwork
 import com.android.testutils.DevSdkIgnoreRule
+import com.android.testutils.DevSdkIgnoreRunner
 import com.android.testutils.RecorderCallback.CallbackEntry.Available
 import com.android.testutils.RecorderCallback.CallbackEntry.Lost
 import com.android.testutils.RouterAdvertisementResponder
-import com.android.testutils.SC_V2
 import com.android.testutils.TapPacketReader
 import com.android.testutils.TestableNetworkCallback
 import com.android.testutils.runAsShell
 import com.android.testutils.waitForIdle
 import org.junit.After
+import org.junit.Assume.assumeFalse
 import org.junit.Before
-import org.junit.Rule
 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.TimeUnit
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
 import kotlin.test.fail
 
-private const val TIMEOUT_MS = 1000L
+// TODO: try to lower this timeout in the future. Currently, ethernet tests are still flaky because
+// the interface is not ready fast enough (mostly due to the up / up / down / up issue).
+private const val TIMEOUT_MS = 2000L
 private const val NO_CALLBACK_TIMEOUT_MS = 200L
 private val DEFAULT_IP_CONFIGURATION = IpConfiguration(IpConfiguration.IpAssignment.DHCP,
     IpConfiguration.ProxySettings.NONE, null, null)
@@ -82,14 +92,13 @@ private val ETH_REQUEST: NetworkRequest = NetworkRequest.Builder()
     .build()
 
 @AppModeFull(reason = "Instant apps can't access EthernetManager")
-@RunWith(AndroidJUnit4::class)
+// EthernetManager is not updatable before T, so tests do not need to be backwards compatible.
+@RunWith(DevSdkIgnoreRunner::class)
+@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2)
 class EthernetManagerTest {
-    // EthernetManager is not updatable before T, so tests do not need to be backwards compatible
-    @get:Rule
-    val ignoreRule = DevSdkIgnoreRule(ignoreClassUpTo = SC_V2)
 
     private val context by lazy { InstrumentationRegistry.getInstrumentation().context }
-    private val em by lazy { EthernetManagerShimImpl.newInstance(context) }
+    private val em by lazy { context.getSystemService(EthernetManager::class.java) }
     private val cm by lazy { context.getSystemService(ConnectivityManager::class.java) }
 
     private val ifaceListener = EthernetStateListener()
@@ -97,6 +106,8 @@ class EthernetManagerTest {
     private val addedListeners = ArrayList<EthernetStateListener>()
     private val networkRequests = ArrayList<TestableNetworkCallback>()
 
+    private var tetheredInterfaceRequest: TetheredInterfaceRequest? = null
+
     private class EthernetTestInterface(
         context: Context,
         private val handler: Handler
@@ -161,11 +172,11 @@ class EthernetManagerTest {
         }
 
         fun expectCallback(iface: EthernetTestInterface, state: Int, role: Int) {
-            expectCallback(createChangeEvent(iface, state, role))
+            expectCallback(createChangeEvent(iface.interfaceName, state, role))
         }
 
-        fun createChangeEvent(iface: EthernetTestInterface, state: Int, role: Int) =
+        fun createChangeEvent(iface: String, state: Int, role: Int) =
-                InterfaceStateChanged(iface.interfaceName, state, role,
+                InterfaceStateChanged(iface, state, role,
                         if (state != STATE_ABSENT) DEFAULT_IP_CONFIGURATION else null)
 
         fun pollForNextCallback(): CallbackEntry {
@@ -174,8 +185,12 @@ 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) {
-            assertNotNull(eventuallyExpect(createChangeEvent(iface, state, role)))
+            eventuallyExpect(iface.interfaceName, state, role)
         }
 
         fun assertNoCallback() {
@@ -184,6 +199,34 @@ class EthernetManagerTest {
         }
     }
 
+    private class TetheredInterfaceListener : TetheredInterfaceCallback {
+        private val available = CompletableFuture<String>()
+
+        override fun onAvailable(iface: String) {
+            available.complete(iface)
+        }
+
+        override fun onUnavailable() {
+            available.completeExceptionally(IllegalStateException("onUnavailable was called"))
+        }
+
+        fun expectOnAvailable(): String {
+            return available.get(TIMEOUT_MS, TimeUnit.MILLISECONDS)
+        }
+
+        fun expectOnUnavailable() {
+            // Assert that the future fails with the IllegalStateException from the
+            // completeExceptionally() call inside onUnavailable.
+            assertFailsWith(IllegalStateException::class) {
+                try {
+                    available.get(TIMEOUT_MS, TimeUnit.MILLISECONDS)
+                } catch (e: ExecutionException) {
+                    throw e.cause!!
+                }
+            }
+        }
+    }
+
     @Before
     fun setUp() {
         setIncludeTestInterfaces(true)
@@ -201,6 +244,7 @@ class EthernetManagerTest {
             em.removeInterfaceStateListener(listener)
         }
         networkRequests.forEach { cm.unregisterNetworkCallback(it) }
+        releaseTetheredInterface()
     }
 
     private fun addInterfaceStateListener(listener: EthernetStateListener) {
@@ -247,6 +291,19 @@ class EthernetManagerTest {
         networkRequests.remove(cb)
     }
 
+    private fun requestTetheredInterface() = TetheredInterfaceListener().also {
+        tetheredInterfaceRequest = runAsShell(NETWORK_SETTINGS) {
+            em.requestTetheredInterface(HandlerExecutor(Handler(Looper.getMainLooper())), it)
+        }
+    }
+
+    private fun releaseTetheredInterface() {
+        runAsShell(NETWORK_SETTINGS) {
+            tetheredInterfaceRequest?.release()
+            tetheredInterfaceRequest = null
+        }
+    }
+
     private fun NetworkRequest.createCopyWithEthernetSpecifier(ifaceName: String) =
         NetworkRequest.Builder(NetworkRequest(ETH_REQUEST))
             .setNetworkSpecifier(EthernetNetworkSpecifier(ifaceName)).build()
@@ -299,6 +356,34 @@ 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)
+    }
+
+    @Test
+    fun testCallbacks_forServerModeInterfaces() {
+        // do not run this test when adb might be connected over ethernet.
+        assumeFalse(isAdbOverNetwork())
+
+        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)
+
+        releaseTetheredInterface()
+        listener.eventuallyExpect(ifaceName, STATE_LINK_UP, ROLE_CLIENT)
+    }
+
     /**
      * Validate all interfaces are returned for an EthernetStateListener upon registration.
      */
@@ -314,7 +399,10 @@ class EthernetManagerTest {
             assertTrue(ifaces.contains(iface), "Untracked interface $iface returned")
             // If the event's iface was created in the test, additional criteria can be validated.
             createdIfaces.find { it.interfaceName.equals(iface) }?.let {
-                assertEquals(event, listener.createChangeEvent(it, STATE_LINK_UP, ROLE_CLIENT))
+                assertEquals(event,
+                    listener.createChangeEvent(it.interfaceName,
+                                                        STATE_LINK_UP,
+                                                        ROLE_CLIENT))
             }
         }
         // Assert all callbacks are accounted for.

tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java

@@ -28,6 +28,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.reset;
@@ -69,6 +70,7 @@ import org.mockito.MockitoAnnotations;
 
 import java.net.InetAddress;
 import java.util.ArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 @SmallTest
 @RunWith(DevSdkIgnoreRunner.class)
@@ -435,7 +437,20 @@ public class EthernetTrackerTest {
         when(mNetd.interfaceGetList()).thenReturn(new String[] {testIface});
         when(mNetd.interfaceGetCfg(eq(testIface))).thenReturn(ifaceParcel);
         doReturn(new String[] {testIface}).when(mFactory).getAvailableInterfaces(anyBoolean());
-        doReturn(EthernetManager.STATE_LINK_UP).when(mFactory).getInterfaceState(eq(testIface));
+
+        final AtomicBoolean ifaceUp = new AtomicBoolean(true);
+        doAnswer(inv -> ifaceUp.get()).when(mFactory).hasInterface(testIface);
+        doAnswer(inv ->
+                ifaceUp.get() ? EthernetManager.STATE_LINK_UP : EthernetManager.STATE_ABSENT)
+                .when(mFactory).getInterfaceState(testIface);
+        doAnswer(inv -> {
+            ifaceUp.set(true);
+            return null;
+        }).when(mFactory).addInterface(eq(testIface), eq(testHwAddr), any(), any());
+        doAnswer(inv -> {
+            ifaceUp.set(false);
+            return null;
+        }).when(mFactory).removeInterface(testIface);
 
         final EthernetStateListener listener = spy(new EthernetStateListener());
         tracker.addListener(listener, true /* canUseRestrictedNetworks */);
@@ -446,7 +461,6 @@ public class EthernetTrackerTest {
         verify(listener).onEthernetStateChanged(eq(EthernetManager.ETHERNET_STATE_ENABLED));
         reset(listener);
 
-        doReturn(EthernetManager.STATE_ABSENT).when(mFactory).getInterfaceState(eq(testIface));
         tracker.setEthernetEnabled(false);
         waitForIdle();
         verify(mFactory).removeInterface(eq(testIface));
@@ -455,7 +469,6 @@ public class EthernetTrackerTest {
                 anyInt(), any());
         reset(listener);
 
-        doReturn(EthernetManager.STATE_LINK_UP).when(mFactory).getInterfaceState(eq(testIface));
         tracker.setEthernetEnabled(true);
         waitForIdle();
         verify(mFactory).addInterface(eq(testIface), eq(testHwAddr), any(), any());