Revert "Change network management listener to outcome receiver" am: 3ea0765ae6 am: a1e987759b am: 6ce84fe2b5

Original change: https://android-review.googlesource.com/c/platform/frameworks/opt/net/ethernet/+/2030085

Change-Id: Ibfaa65b1baf1f3cc49a8b73830edee7f29e5557f

6 files changed, 75 insertions, 71 deletions

diff --git a/java/com/android/server/ethernet/EthernetNetworkFactory.java b/java/com/android/server/ethernet/EthernetNetworkFactory.java
index 4f15355..ef3abba 100644
--- a/java/com/android/server/ethernet/EthernetNetworkFactory.java
+++ b/java/com/android/server/ethernet/EthernetNetworkFactory.java
@@ -24,8 +24,8 @@ import android.net.ConnectivityManager;
 import android.net.ConnectivityResources;
 import android.net.EthernetManager;
 import android.net.EthernetNetworkSpecifier;
+import android.net.IEthernetNetworkManagementListener;
 import android.net.EthernetNetworkManagementException;
-import android.net.INetworkInterfaceOutcomeReceiver;
 import android.net.IpConfiguration;
 import android.net.IpConfiguration.IpAssignment;
 import android.net.IpConfiguration.ProxySettings;
@@ -239,14 +239,14 @@ public class EthernetNetworkFactory extends NetworkFactory {
      *                     {@code null} is passed, then the network's current
      *                     {@link NetworkCapabilities} will be used in support of existing APIs as
      *                     the public API does not allow this.
-     * @param listener an optional {@link INetworkInterfaceOutcomeReceiver} to notify callers of
+     * @param listener an optional {@link IEthernetNetworkManagementListener} to notify callers of
      *                 completion.
      */
     @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
     protected void updateInterface(@NonNull final String ifaceName,
             @Nullable final IpConfiguration ipConfig,
             @Nullable final NetworkCapabilities capabilities,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         if (!hasInterface(ifaceName)) {
             maybeSendNetworkManagementCallbackForUntracked(ifaceName, listener);
             return;
@@ -295,7 +295,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
     /** Returns true if state has been modified */
     @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)
     protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         if (!hasInterface(ifaceName)) {
             maybeSendNetworkManagementCallbackForUntracked(ifaceName, listener);
             return false;
@@ -310,7 +310,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
     }
 
     private void maybeSendNetworkManagementCallbackForUntracked(
-            String ifaceName, INetworkInterfaceOutcomeReceiver listener) {
+            String ifaceName, IEthernetNetworkManagementListener listener) {
         maybeSendNetworkManagementCallback(listener, null,
                 new EthernetNetworkManagementException(
                         ifaceName + " can't be updated as it is not available."));
@@ -353,19 +353,15 @@ public class EthernetNetworkFactory extends NetworkFactory {
     }
 
     private static void maybeSendNetworkManagementCallback(
-            @Nullable final INetworkInterfaceOutcomeReceiver listener,
-            @Nullable final String iface,
+            @Nullable final IEthernetNetworkManagementListener listener,
+            @Nullable final Network network,
             @Nullable final EthernetNetworkManagementException e) {
         if (null == listener) {
             return;
         }
 
         try {
-            if (iface != null) {
-                listener.onResult(iface);
-            } else {
-                listener.onError(e);
-            }
+            listener.onComplete(network, e);
         } catch (RemoteException re) {
             Log.e(TAG, "Can't send onComplete for network management callback", re);
         }
@@ -419,9 +415,9 @@ public class EthernetNetworkFactory extends NetworkFactory {
         private class EthernetIpClientCallback extends IpClientCallbacks {
             private final ConditionVariable mIpClientStartCv = new ConditionVariable(false);
             private final ConditionVariable mIpClientShutdownCv = new ConditionVariable(false);
-            @Nullable INetworkInterfaceOutcomeReceiver mNetworkManagementListener;
+            @Nullable IEthernetNetworkManagementListener mNetworkManagementListener;
 
-            EthernetIpClientCallback(@Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            EthernetIpClientCallback(@Nullable final IEthernetNetworkManagementListener listener) {
                 mNetworkManagementListener = listener;
             }
 
@@ -506,7 +502,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
 
         void updateInterface(@Nullable final IpConfiguration ipConfig,
                 @Nullable final NetworkCapabilities capabilities,
-                @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+                @Nullable final IEthernetNetworkManagementListener listener) {
             if (DBG) {
                 Log.d(TAG, "updateInterface, iface: " + name
                         + ", ipConfig: " + ipConfig + ", old ipConfig: " + mIpConfig
@@ -537,7 +533,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
             start(null);
         }
 
-        private void start(@Nullable final INetworkInterfaceOutcomeReceiver listener) {
+        private void start(@Nullable final IEthernetNetworkManagementListener listener) {
             if (mIpClient != null) {
                 if (DBG) Log.d(TAG, "IpClient already started");
                 return;
@@ -557,7 +553,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
         }
 
         void onIpLayerStarted(@NonNull final LinkProperties linkProperties,
-                @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+                @Nullable final IEthernetNetworkManagementListener listener) {
             if(mIpClient == null) {
                 // This call comes from a message posted on the handler thread, but the IpClient has
                 // since been stopped such as may be the case if updateInterfaceLinkState() is
@@ -597,10 +593,10 @@ public class EthernetNetworkFactory extends NetworkFactory {
                     });
             mNetworkAgent.register();
             mNetworkAgent.markConnected();
-            realizeNetworkManagementCallback(name, null);
+            realizeNetworkManagementCallback(mNetworkAgent.getNetwork(), null);
         }
 
-        void onIpLayerStopped(@Nullable final INetworkInterfaceOutcomeReceiver listener) {
+        void onIpLayerStopped(@Nullable final IEthernetNetworkManagementListener listener) {
             // This cannot happen due to provisioning timeout, because our timeout is 0. It can
             // happen due to errors while provisioning or on provisioning loss.
             if(mIpClient == null) {
@@ -626,7 +622,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
         }
 
         // Must be called on the handler thread
-        private void realizeNetworkManagementCallback(@Nullable final String iface,
+        private void realizeNetworkManagementCallback(@Nullable final Network network,
                 @Nullable final EthernetNetworkManagementException e) {
             ensureRunningOnEthernetHandlerThread();
             if (null == mIpClientCallback) {
@@ -634,7 +630,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
             }
 
             EthernetNetworkFactory.maybeSendNetworkManagementCallback(
-                    mIpClientCallback.mNetworkManagementListener, iface, e);
+                    mIpClientCallback.mNetworkManagementListener, network, e);
             // Only send a single callback per listener.
             mIpClientCallback.mNetworkManagementListener = null;
         }
@@ -675,7 +671,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
 
         /** Returns true if state has been modified */
         boolean updateLinkState(final boolean up,
-                @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+                @Nullable final IEthernetNetworkManagementListener listener) {
             if (mLinkUp == up)  {
                 EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, null,
                         new EthernetNetworkManagementException(
@@ -685,11 +681,13 @@ public class EthernetNetworkFactory extends NetworkFactory {
             mLinkUp = up;
 
             if (!up) { // was up, goes down
+                // Save an instance of the current network to use with the callback before stop().
+                final Network network = mNetworkAgent != null ? mNetworkAgent.getNetwork() : null;
                 // Send an abort on a provisioning request callback if necessary before stopping.
                 maybeSendNetworkManagementCallbackForAbort();
                 stop();
                 // If only setting the interface down, send a callback to signal completion.
-                EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, name, null);
+                EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, network, null);
             } else { // was down, goes up
                 stop();
                 start(listener);
@@ -744,7 +742,7 @@ public class EthernetNetworkFactory extends NetworkFactory {
             restart(null);
         }
 
-        void restart(@Nullable final INetworkInterfaceOutcomeReceiver listener) {
+        void restart(@Nullable final IEthernetNetworkManagementListener listener){
             if (DBG) Log.d(TAG, "reconnecting Ethernet");
             stop();
             start(listener);
diff --git a/java/com/android/server/ethernet/EthernetServiceImpl.java b/java/com/android/server/ethernet/EthernetServiceImpl.java
index edda321..50b4684 100644
--- a/java/com/android/server/ethernet/EthernetServiceImpl.java
+++ b/java/com/android/server/ethernet/EthernetServiceImpl.java
@@ -24,7 +24,7 @@ import android.content.Context;
 import android.content.pm.PackageManager;
 import android.net.IEthernetManager;
 import android.net.IEthernetServiceListener;
-import android.net.INetworkInterfaceOutcomeReceiver;
+import android.net.IEthernetNetworkManagementListener;
 import android.net.ITetheredInterfaceCallback;
 import android.net.EthernetNetworkUpdateRequest;
 import android.net.IpConfiguration;
@@ -243,7 +243,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
     @Override
     public void updateConfiguration(@NonNull final String iface,
             @NonNull final EthernetNetworkUpdateRequest request,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         Objects.requireNonNull(iface);
         Objects.requireNonNull(request);
         throwIfEthernetNotStarted();
@@ -260,7 +260,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
 
     @Override
     public void connectNetwork(@NonNull final String iface,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         Log.i(TAG, "connectNetwork called with: iface=" + iface + ", listener=" + listener);
         Objects.requireNonNull(iface);
         throwIfEthernetNotStarted();
@@ -272,7 +272,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub {
 
     @Override
     public void disconnectNetwork(@NonNull final String iface,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         Log.i(TAG, "disconnectNetwork called with: iface=" + iface + ", listener=" + listener);
         Objects.requireNonNull(iface);
         throwIfEthernetNotStarted();
diff --git a/java/com/android/server/ethernet/EthernetTracker.java b/java/com/android/server/ethernet/EthernetTracker.java
index 0a0e327..074c81b 100644
--- a/java/com/android/server/ethernet/EthernetTracker.java
+++ b/java/com/android/server/ethernet/EthernetTracker.java
@@ -27,7 +27,7 @@ import android.content.res.Resources;
 import android.net.ConnectivityResources;
 import android.net.EthernetManager;
 import android.net.IEthernetServiceListener;
-import android.net.INetworkInterfaceOutcomeReceiver;
+import android.net.IEthernetNetworkManagementListener;
 import android.net.INetd;
 import android.net.ITetheredInterfaceCallback;
 import android.net.InterfaceConfigurationParcel;
@@ -291,7 +291,7 @@ public class EthernetTracker {
     protected void updateConfiguration(@NonNull final String iface,
             @Nullable final IpConfiguration ipConfig,
             @Nullable final NetworkCapabilities capabilities,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         if (DBG) {
             Log.i(TAG, "updateConfiguration, iface: " + iface + ", capabilities: " + capabilities
                     + ", ipConfig: " + ipConfig);
@@ -314,13 +314,13 @@ public class EthernetTracker {
 
     @VisibleForTesting(visibility = PACKAGE)
     protected void connectNetwork(@NonNull final String iface,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         mHandler.post(() -> updateInterfaceState(iface, true, listener));
     }
 
     @VisibleForTesting(visibility = PACKAGE)
     protected void disconnectNetwork(@NonNull final String iface,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         mHandler.post(() -> updateInterfaceState(iface, false, listener));
     }
 
@@ -505,7 +505,7 @@ public class EthernetTracker {
     }
 
     private void updateInterfaceState(@NonNull final String iface, final boolean up,
-            @Nullable final INetworkInterfaceOutcomeReceiver listener) {
+            @Nullable final IEthernetNetworkManagementListener listener) {
         final int mode = getInterfaceMode(iface);
         final boolean factoryLinkStateUpdated = (mode == INTERFACE_MODE_CLIENT)
                 && mFactory.updateInterfaceLinkState(iface, up, listener);
diff --git a/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
index 5d23aaf..726833f 100644
--- a/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
+++ b/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java
@@ -41,8 +41,8 @@ import android.content.Context;
 import android.content.res.Resources;
 import android.net.ConnectivityManager;
 import android.net.EthernetNetworkSpecifier;
+import android.net.IEthernetNetworkManagementListener;
 import android.net.EthernetNetworkManagementException;
-import android.net.INetworkInterfaceOutcomeReceiver;
 import android.net.IpConfiguration;
 import android.net.LinkAddress;
 import android.net.LinkProperties;
@@ -85,7 +85,7 @@ import java.util.concurrent.TimeUnit;
 public class EthernetNetworkFactoryTest {
     private static final int TIMEOUT_MS = 2_000;
     private static final String TEST_IFACE = "test123";
-    private static final INetworkInterfaceOutcomeReceiver NULL_LISTENER = null;
+    private static final IEthernetNetworkManagementListener NULL_LISTENER = null;
     private static final String IP_ADDR = "192.0.2.2/25";
     private static final LinkAddress LINK_ADDR = new LinkAddress(IP_ADDR);
     private static final String HW_ADDR = "01:02:03:04:05:06";
@@ -322,7 +322,7 @@ public class EthernetNetworkFactoryTest {
 
         assertTrue(ret);
         verify(mIpClient).shutdown();
-        assertEquals(listener.expectOnResult(), TEST_IFACE);
+        assertSuccessfulListener(listener, null);
     }
 
     @Test
@@ -336,7 +336,7 @@ public class EthernetNetworkFactoryTest {
 
         assertTrue(ret);
         verifyStop();
-        assertEquals(listener.expectOnResult(), TEST_IFACE);
+        assertSuccessfulListener(listener, mMockNetwork);
     }
 
     @Test
@@ -353,7 +353,7 @@ public class EthernetNetworkFactoryTest {
         verify(mDeps, never()).makeIpClient(any(), any(), any());
         verify(mDeps, never())
                 .makeEthernetNetworkAgent(any(), any(), any(), any(), any(), any(), any());
-        assertEquals(listener.expectOnResult(), TEST_IFACE);
+        assertSuccessfulListener(listener, null);
     }
 
     @Test
@@ -367,7 +367,7 @@ public class EthernetNetworkFactoryTest {
 
         assertFalse(ret);
         verifyNoStopOrStart();
-        listener.expectOnErrorWithMessage("can't be updated as it is not available");
+        assertFailedListener(listener, "can't be updated as it is not available");
     }
 
     @Test
@@ -381,7 +381,7 @@ public class EthernetNetworkFactoryTest {
 
         assertFalse(ret);
         verifyNoStopOrStart();
-        listener.expectOnErrorWithMessage("No changes");
+        assertFailedListener(listener, "No changes");
     }
 
     @Test
@@ -638,31 +638,18 @@ public class EthernetNetworkFactoryTest {
     }
 
     private static final class TestNetworkManagementListener
-            implements INetworkInterfaceOutcomeReceiver {
-        private final CompletableFuture<String> mResult = new CompletableFuture<>();
-        private final CompletableFuture<EthernetNetworkManagementException> mError =
-                new CompletableFuture<>();
+            implements IEthernetNetworkManagementListener {
+        private final CompletableFuture<Pair<Network, EthernetNetworkManagementException>> mDone
+                = new CompletableFuture<>();
 
         @Override
-        public void onResult(@NonNull String iface) {
-            mResult.complete(iface);
+        public void onComplete(final Network network,
+                final EthernetNetworkManagementException exception) {
+            mDone.complete(new Pair<>(network, exception));
         }
 
-        @Override
-        public void onError(@NonNull EthernetNetworkManagementException exception) {
-            mError.complete(exception);
-        }
-
-        String expectOnResult() throws Exception {
-            return mResult.get(TIMEOUT_MS, TimeUnit.MILLISECONDS);
-        }
-
-        EthernetNetworkManagementException expectOnError() throws Exception {
-            return mError.get(TIMEOUT_MS, TimeUnit.MILLISECONDS);
-        }
-
-        void expectOnErrorWithMessage(String msg) throws Exception {
-            assertTrue(expectOnError().getMessage().contains(msg));
+        Pair<Network, EthernetNetworkManagementException> expectOnComplete() throws Exception {
+            return mDone.get(TIMEOUT_MS, TimeUnit.MILLISECONDS);
         }
 
         @Override
@@ -682,7 +669,7 @@ public class EthernetNetworkFactoryTest {
         mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener);
         triggerOnProvisioningSuccess();
 
-        assertEquals(listener.expectOnResult(), TEST_IFACE);
+        assertSuccessfulListener(listener, mMockNetwork);
     }
 
     @DevSdkIgnoreRule.IgnoreUpTo(SC_V2) // TODO: Use to Build.VERSION_CODES.SC_V2 when available
@@ -722,7 +709,26 @@ public class EthernetNetworkFactoryTest {
                     triggerOnProvisioningSuccess();
                 });
 
-        assertEquals(successfulListener.expectOnResult(), TEST_IFACE);
+        assertSuccessfulListener(successfulListener, mMockNetwork);
+    }
+
+    private void assertSuccessfulListener(
+            @NonNull final TestNetworkManagementListener successfulListener,
+            @NonNull final Network expectedNetwork) throws Exception {
+        final Pair<Network, EthernetNetworkManagementException> successfulResult =
+                successfulListener.expectOnComplete();
+        assertEquals(expectedNetwork, successfulResult.first);
+        assertNull(successfulResult.second);
+    }
+
+    private void assertFailedListener(@NonNull final TestNetworkManagementListener failedListener,
+            @NonNull final String errMsg)
+            throws Exception {
+        final Pair<Network, EthernetNetworkManagementException> failedResult =
+                failedListener.expectOnComplete();
+        assertNull(failedResult.first);
+        assertNotNull(failedResult.second);
+        assertTrue(failedResult.second.getMessage().contains(errMsg));
     }
 
     private void verifyNetworkManagementCallIsAbortedWhenInterrupted(
@@ -737,7 +743,7 @@ public class EthernetNetworkFactoryTest {
         mNetFactory.updateInterface(iface, ipConfiguration, capabilities, failedListener);
         interruptingRunnable.run();
 
-        failedListener.expectOnErrorWithMessage("aborted");
+        assertFailedListener(failedListener, "aborted");
     }
 
     @Test
@@ -751,7 +757,7 @@ public class EthernetNetworkFactoryTest {
         mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener);
         triggerOnProvisioningSuccess();
 
-        assertEquals(listener.expectOnResult(), TEST_IFACE);
+        listener.expectOnComplete();
         verify(mDeps).makeEthernetNetworkAgent(any(), any(),
                 eq(capabilities), any(), any(), any(), any());
         verifyRestart(ipConfiguration);
@@ -768,7 +774,7 @@ public class EthernetNetworkFactoryTest {
         mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener);
 
         verifyNoStopOrStart();
-        listener.expectOnErrorWithMessage("can't be updated as it is not available");
+        assertFailedListener(listener, "can't be updated as it is not available");
     }
 
     @Test
diff --git a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
index e67c4c8..2131f7f 100644
--- a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
+++ b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java
@@ -32,7 +32,7 @@ import android.Manifest;
 import android.annotation.NonNull;
 import android.content.Context;
 import android.content.pm.PackageManager;
-import android.net.INetworkInterfaceOutcomeReceiver;
+import android.net.IEthernetNetworkManagementListener;
 import android.net.EthernetNetworkUpdateRequest;
 import android.net.IpConfiguration;
 import android.net.NetworkCapabilities;
@@ -64,7 +64,7 @@ public class EthernetServiceImplTest {
             new EthernetNetworkUpdateRequest.Builder()
                     .setNetworkCapabilities(new NetworkCapabilities.Builder().build())
                     .build();
-    private static final INetworkInterfaceOutcomeReceiver NULL_LISTENER = null;
+    private static final IEthernetNetworkManagementListener NULL_LISTENER = null;
     private EthernetServiceImpl mEthernetServiceImpl;
     @Mock private Context mContext;
     @Mock private Handler mHandler;
diff --git a/tests/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/java/com/android/server/ethernet/EthernetTrackerTest.java
index bab9643..ef70d94 100644
--- a/tests/java/com/android/server/ethernet/EthernetTrackerTest.java
+++ b/tests/java/com/android/server/ethernet/EthernetTrackerTest.java
@@ -33,7 +33,7 @@ import static org.mockito.Mockito.when;
 import android.content.Context;
 import android.content.res.Resources;
 import android.net.InetAddresses;
-import android.net.INetworkInterfaceOutcomeReceiver;
+import android.net.IEthernetNetworkManagementListener;
 import android.net.INetd;
 import android.net.IpConfiguration;
 import android.net.IpConfiguration.IpAssignment;
@@ -66,7 +66,7 @@ public class EthernetTrackerTest {
     private static final String TEST_IFACE = "test123";
     private static final int TIMEOUT_MS = 1_000;
     private static final String THREAD_NAME = "EthernetServiceThread";
-    private static final INetworkInterfaceOutcomeReceiver NULL_LISTENER = null;
+    private static final IEthernetNetworkManagementListener NULL_LISTENER = null;
     private EthernetTracker tracker;
     private HandlerThread mHandlerThread;
     @Mock private Context mContext;
@@ -334,7 +334,7 @@ public class EthernetTrackerTest {
                 new StaticIpConfiguration.Builder().setIpAddress(linkAddr).build();
         final IpConfiguration ipConfig =
                 new IpConfiguration.Builder().setStaticIpConfiguration(staticIpConfig).build();
-        final INetworkInterfaceOutcomeReceiver listener = null;
+        final IEthernetNetworkManagementListener listener = null;
 
         tracker.updateConfiguration(TEST_IFACE, ipConfig, capabilities, listener);
         waitForIdle();