diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java index 64cb7353ed..51c750c1c4 100644 --- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java +++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java @@ -187,22 +187,19 @@ public class EthernetNetworkFactory { * {@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 cb a {@link EthernetCallback} 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, - final EthernetCallback cb) { + @Nullable final NetworkCapabilities capabilities) { if (!hasInterface(ifaceName)) { - cb.onError(ifaceName + " is not tracked by the ethernet service"); return; } final NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); - iface.updateInterface(ipConfig, capabilities, cb); + iface.updateInterface(ipConfig, capabilities); mTrackingInterfaces.put(ifaceName, iface); + return; } private static NetworkCapabilities mixInCapabilities(NetworkCapabilities nc, @@ -303,11 +300,6 @@ public class EthernetNetworkFactory { private class EthernetIpClientCallback extends IpClientCallbacks { private final ConditionVariable mIpClientStartCv = new ConditionVariable(false); private final ConditionVariable mIpClientShutdownCv = new ConditionVariable(false); - EthernetCallback mCb; - - EthernetIpClientCallback(EthernetCallback cb) { - mCb = cb; - } @Override public void onIpClientCreated(IIpClient ipClient) { @@ -350,7 +342,7 @@ public class EthernetNetworkFactory { public void onProvisioningFailure(LinkProperties newLp) { // This cannot happen due to provisioning timeout, because our timeout is 0. It can // happen due to errors while provisioning or on provisioning loss. - handleIpEvent(() -> onIpLayerStopped(mCb)); + handleIpEvent(() -> onIpLayerStopped()); } @Override @@ -462,13 +454,11 @@ public class EthernetNetworkFactory { } void updateInterface(@Nullable final IpConfiguration ipConfig, - @Nullable final NetworkCapabilities capabilities, - final EthernetCallback cb) { + @Nullable final NetworkCapabilities capabilities) { if (DBG) { Log.d(TAG, "updateInterface, iface: " + name + ", ipConfig: " + ipConfig + ", old ipConfig: " + mIpConfig + ", capabilities: " + capabilities + ", old capabilities: " + mCapabilities - + ", cb: " + cb ); } @@ -481,7 +471,7 @@ public class EthernetNetworkFactory { // TODO: Update this logic to only do a restart if required. Although a restart may // be required due to the capabilities or ipConfiguration values, not all // capabilities changes require a restart. - restart(cb); + restart(); } boolean isRestricted() { @@ -489,11 +479,6 @@ public class EthernetNetworkFactory { } private void start() { - // TODO: remove EthernetCallback from this class entirely. - start(new EthernetCallback(null)); - } - - private void start(EthernetCallback cb) { if (mIpClient != null) { if (DBG) Log.d(TAG, "IpClient already started"); return; @@ -502,7 +487,7 @@ public class EthernetNetworkFactory { Log.d(TAG, String.format("Starting Ethernet IpClient(%s)", name)); } - mIpClientCallback = new EthernetIpClientCallback(cb); + mIpClientCallback = new EthernetIpClientCallback(); mDeps.makeIpClient(mContext, name, mIpClientCallback); mIpClientCallback.awaitIpClientStart(); @@ -544,20 +529,18 @@ public class EthernetNetworkFactory { }); mNetworkAgent.register(); mNetworkAgent.markConnected(); - mIpClientCallback.mCb.onResult(name); } - void onIpLayerStopped(EthernetCallback cb) { + void onIpLayerStopped() { // There is no point in continuing if the interface is gone as stop() will be triggered // by removeInterface() when processed on the handler thread and start() won't // work for a non-existent interface. if (null == mDeps.getNetworkInterfaceByName(name)) { if (DBG) Log.d(TAG, name + " is no longer available."); // Send a callback in case a provisioning request was in progress. - mIpClientCallback.mCb.onError("IP provisioning has been aborted."); return; } - restart(cb); + restart(); } private void ensureRunningOnEthernetHandlerThread() { @@ -613,11 +596,8 @@ public class EthernetNetworkFactory { mIpClientCallback.awaitIpClientShutdown(); mIpClient = null; } - // Send an abort callback if an updateInterface request was in progress. - if (mIpClientCallback != null) { - mIpClientCallback.mCb.onError("IP provisioning has been aborted."); - mIpClientCallback = null; - } + + mIpClientCallback = null; if (mNetworkAgent != null) { mNetworkAgent.unregister(); @@ -673,14 +653,9 @@ public class EthernetNetworkFactory { } void restart() { - // TODO: remove EthernetCallback from this class entirely - restart(new EthernetCallback(null)); - } - - void restart(EthernetCallback cb) { if (DBG) Log.d(TAG, "reconnecting Ethernet"); stop(); - start(cb); + start(); } @Override diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index a24a11945f..8b956fa53f 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -270,7 +270,7 @@ public class EthernetTracker { } writeIpConfiguration(iface, ipConfiguration); mHandler.post(() -> { - mFactory.updateInterface(iface, ipConfiguration, null, null); + mFactory.updateInterface(iface, ipConfiguration, null); broadcastInterfaceStateChange(iface); }); } @@ -352,8 +352,16 @@ public class EthernetTracker { mNetworkCapabilities.put(iface, capabilities); } mHandler.post(() -> { - mFactory.updateInterface(iface, localIpConfig, capabilities, cb); - broadcastInterfaceStateChange(iface); + mFactory.updateInterface(iface, localIpConfig, capabilities); + + // only broadcast state change when the ip configuration is updated. + if (ipConfig != null) { + broadcastInterfaceStateChange(iface); + } + // Always return success. Even if the interface does not currently exist, the + // IpConfiguration and NetworkCapabilities were saved and will be applied if an + // interface with the given name is ever added. + cb.onResult(iface); }); } diff --git a/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java index 3b5a9b28b0..dcacf234e8 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java @@ -602,93 +602,16 @@ public class EthernetNetworkFactoryTest { } } - @Test - public void testUpdateInterfaceCallsListenerCorrectlyOnSuccess() throws Exception { - initEthernetNetworkFactory(); - createAndVerifyProvisionedInterface(TEST_IFACE); - final NetworkCapabilities capabilities = createDefaultFilterCaps(); - final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); - - mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); - triggerOnProvisioningSuccess(); - - assertEquals(TEST_IFACE, listener.expectOnResult()); - } - - @Test - public void testUpdateInterfaceAbortsOnConcurrentRemoveInterface() throws Exception { - initEthernetNetworkFactory(); - verifyNetworkManagementCallIsAbortedWhenInterrupted( - TEST_IFACE, - () -> mNetFactory.removeInterface(TEST_IFACE)); - } - - @Test - public void testUpdateInterfaceAbortsOnConcurrentUpdateInterfaceLinkState() throws Exception { - initEthernetNetworkFactory(); - verifyNetworkManagementCallIsAbortedWhenInterrupted( - TEST_IFACE, - () -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_CB)); - } - - @Test - public void testUpdateInterfaceAbortsOnNetworkUneededRemovesAllRequests() throws Exception { - initEthernetNetworkFactory(); - verifyNetworkManagementCallIsAbortedWhenInterrupted( - TEST_IFACE, - () -> mNetworkOfferCallback.onNetworkUnneeded(mRequestToKeepNetworkUp)); - } - - @Test - public void testUpdateInterfaceCallsListenerCorrectlyOnConcurrentRequests() throws Exception { - initEthernetNetworkFactory(); - final NetworkCapabilities capabilities = createDefaultFilterCaps(); - final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener successfulListener = - new TestNetworkManagementListener(); - - // If two calls come in before the first one completes, the first listener will be aborted - // and the second one will be successful. - verifyNetworkManagementCallIsAbortedWhenInterrupted( - TEST_IFACE, - () -> { - mNetFactory.updateInterface( - TEST_IFACE, ipConfiguration, capabilities, successfulListener); - triggerOnProvisioningSuccess(); - }); - - assertEquals(successfulListener.expectOnResult(), TEST_IFACE); - assertEquals(TEST_IFACE, successfulListener.expectOnResult()); - } - - private void verifyNetworkManagementCallIsAbortedWhenInterrupted( - @NonNull final String iface, - @NonNull final Runnable interruptingRunnable) throws Exception { - createAndVerifyProvisionedInterface(iface); - final NetworkCapabilities capabilities = createDefaultFilterCaps(); - final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener failedListener = new TestNetworkManagementListener(); - - // An active update request will be aborted on interrupt prior to provisioning completion. - mNetFactory.updateInterface(iface, ipConfiguration, capabilities, failedListener); - interruptingRunnable.run(); - - failedListener.expectOnError(); - } - @Test public void testUpdateInterfaceRestartsAgentCorrectly() throws Exception { initEthernetNetworkFactory(); createAndVerifyProvisionedInterface(TEST_IFACE); final NetworkCapabilities capabilities = createDefaultFilterCaps(); final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); - mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); + mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities); triggerOnProvisioningSuccess(); - assertEquals(TEST_IFACE, listener.expectOnResult()); verify(mDeps).makeEthernetNetworkAgent(any(), any(), eq(capabilities), any(), any(), any(), any()); verifyRestart(ipConfiguration); @@ -700,12 +623,10 @@ public class EthernetNetworkFactoryTest { // No interface exists due to not calling createAndVerifyProvisionedInterface(...). final NetworkCapabilities capabilities = createDefaultFilterCaps(); final IpConfiguration ipConfiguration = createStaticIpConfig(); - final TestNetworkManagementListener listener = new TestNetworkManagementListener(); - mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); + mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities); verifyNoStopOrStart(); - listener.expectOnError(); } @Test @@ -714,8 +635,8 @@ public class EthernetNetworkFactoryTest { createAndVerifyProvisionedInterface(TEST_IFACE); final IpConfiguration initialIpConfig = createStaticIpConfig(); - mNetFactory.updateInterface(TEST_IFACE, initialIpConfig, null /*capabilities*/, - new EthernetCallback(null /* cb */)); + mNetFactory.updateInterface(TEST_IFACE, initialIpConfig, null /*capabilities*/); + triggerOnProvisioningSuccess(); verifyRestart(initialIpConfig); @@ -726,8 +647,7 @@ public class EthernetNetworkFactoryTest { // verify that sending a null ipConfig does not update the current ipConfig. - mNetFactory.updateInterface(TEST_IFACE, null /*ipConfig*/, null /*capabilities*/, - new EthernetCallback(null /* cb */)); + mNetFactory.updateInterface(TEST_IFACE, null /*ipConfig*/, null /*capabilities*/); triggerOnProvisioningSuccess(); verifyRestart(initialIpConfig); } diff --git a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java index 3e1e4a5510..1df56c73dd 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java @@ -349,7 +349,7 @@ public class EthernetTrackerTest { waitForIdle(); verify(mFactory).updateInterface( - eq(TEST_IFACE), eq(ipConfig), eq(capabilities), eq(listener)); + eq(TEST_IFACE), eq(ipConfig), eq(capabilities)); } @Test