Merge "Add a ClearingIpAddressesState."

am: d82280212c

Change-Id: Ied4dfad68bd4e5680e3e51e9d11dd2f3fd7cabc0

2 files changed, 100 insertions, 32 deletions

diff --git a/src/android/net/ip/IpClient.java b/src/android/net/ip/IpClient.java
index 8808ee2..669dbfd 100644
--- a/src/android/net/ip/IpClient.java
+++ b/src/android/net/ip/IpClient.java
@@ -310,7 +310,7 @@ public class IpClient extends StateMachine {
     // Internal commands to use instead of trying to call transitionTo() inside
     // a given State's enter() method. Calling transitionTo() from enter/exit
     // encounters a Log.wtf() that can cause trouble on eng builds.
-    private static final int CMD_JUMP_STARTED_TO_RUNNING          = 100;
+    private static final int CMD_ADDRESSES_CLEARED                = 100;
     private static final int CMD_JUMP_RUNNING_TO_STOPPING         = 101;
     private static final int CMD_JUMP_STOPPING_TO_STOPPED         = 102;
 
@@ -342,6 +342,7 @@ public class IpClient extends StateMachine {
 
     private final State mStoppedState = new StoppedState();
     private final State mStoppingState = new StoppingState();
+    private final State mClearingIpAddressesState = new ClearingIpAddressesState();
     private final State mStartedState = new StartedState();
     private final State mRunningState = new RunningState();
 
@@ -621,6 +622,7 @@ public class IpClient extends StateMachine {
         // CHECKSTYLE:OFF IndentationCheck
         addState(mStoppedState);
         addState(mStartedState);
+            addState(mClearingIpAddressesState, mStartedState);
             addState(mRunningState, mStartedState);
         addState(mStoppingState);
         // CHECKSTYLE:ON IndentationCheck
@@ -1355,7 +1357,7 @@ public class IpClient extends StateMachine {
 
                 case CMD_START:
                     mConfiguration = (android.net.shared.ProvisioningConfiguration) msg.obj;
-                    transitionTo(mStartedState);
+                    transitionTo(mClearingIpAddressesState);
                     break;
 
                 case EVENT_NETLINK_LINKPROPERTIES_CHANGED:
@@ -1437,18 +1439,11 @@ public class IpClient extends StateMachine {
         }
     }
 
-    class StartedState extends State {
+    class ClearingIpAddressesState extends State {
         @Override
         public void enter() {
-            mStartTimeMillis = SystemClock.elapsedRealtime();
-            if (mConfiguration.mProvisioningTimeoutMs > 0) {
-                final long alarmTime = SystemClock.elapsedRealtime()
-                        + mConfiguration.mProvisioningTimeoutMs;
-                mProvisioningTimeoutAlarm.schedule(alarmTime);
-            }
-
             if (readyToProceed()) {
-                deferMessage(obtainMessage(CMD_JUMP_STARTED_TO_RUNNING));
+                deferMessage(obtainMessage(CMD_ADDRESSES_CLEARED));
             } else {
                 // Clear all IPv4 and IPv6 before proceeding to RunningState.
                 // Clean up any leftover state from an abnormal exit from
@@ -1458,21 +1453,12 @@ public class IpClient extends StateMachine {
         }
 
         @Override
-        public void exit() {
-            mProvisioningTimeoutAlarm.cancel();
-        }
-
-        @Override
         public boolean processMessage(Message msg) {
             switch (msg.what) {
-                case CMD_JUMP_STARTED_TO_RUNNING:
+                case CMD_ADDRESSES_CLEARED:
                     transitionTo(mRunningState);
                     break;
 
-                case CMD_STOP:
-                    transitionTo(mStoppingState);
-                    break;
-
                 case EVENT_NETLINK_LINKPROPERTIES_CHANGED:
                     handleLinkPropertiesUpdate(NO_CALLBACKS);
                     if (readyToProceed()) {
@@ -1480,6 +1466,50 @@ public class IpClient extends StateMachine {
                     }
                     break;
 
+                case CMD_STOP:
+                case EVENT_PROVISIONING_TIMEOUT:
+                    // Fall through to StartedState.
+                    return NOT_HANDLED;
+
+                default:
+                    // It's safe to process messages out of order because the
+                    // only message that can both
+                    //     a) be received at this time and
+                    //     b) affect provisioning state
+                    // is EVENT_NETLINK_LINKPROPERTIES_CHANGED (handled above).
+                    deferMessage(msg);
+            }
+            return HANDLED;
+        }
+
+        private boolean readyToProceed() {
+            return (!mLinkProperties.hasIpv4Address() && !mLinkProperties.hasGlobalIpv6Address());
+        }
+    }
+
+    class StartedState extends State {
+        @Override
+        public void enter() {
+            mStartTimeMillis = SystemClock.elapsedRealtime();
+            if (mConfiguration.mProvisioningTimeoutMs > 0) {
+                final long alarmTime = SystemClock.elapsedRealtime()
+                        + mConfiguration.mProvisioningTimeoutMs;
+                mProvisioningTimeoutAlarm.schedule(alarmTime);
+            }
+        }
+
+        @Override
+        public void exit() {
+            mProvisioningTimeoutAlarm.cancel();
+        }
+
+        @Override
+        public boolean processMessage(Message msg) {
+            switch (msg.what) {
+                case CMD_STOP:
+                    transitionTo(mStoppingState);
+                    break;
+
                 case CMD_UPDATE_L2KEY_GROUPHINT: {
                     final Pair<String, String> args = (Pair<String, String>) msg.obj;
                     mL2Key = args.first;
@@ -1497,22 +1527,14 @@ public class IpClient extends StateMachine {
                 case EVENT_PROVISIONING_TIMEOUT:
                     handleProvisioningFailure();
                     break;
+
                 default:
-                    // It's safe to process messages out of order because the
-                    // only message that can both
-                    //     a) be received at this time and
-                    //     b) affect provisioning state
-                    // is EVENT_NETLINK_LINKPROPERTIES_CHANGED (handled above).
-                    deferMessage(msg);
+                    return NOT_HANDLED;
             }
 
             mMsgStateLogger.handled(this, getCurrentState());
             return HANDLED;
         }
-
-        private boolean readyToProceed() {
-            return (!mLinkProperties.hasIpv4Address() && !mLinkProperties.hasGlobalIpv6Address());
-        }
     }
 
     class RunningState extends State {
diff --git a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
index 2bbaf06..b1efa71 100644
--- a/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
+++ b/tests/integration/src/android/net/ip/IpClientIntegrationTest.java
@@ -51,6 +51,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.argThat;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.eq;
@@ -919,4 +920,49 @@ public class IpClientIntegrationTest {
 
         verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningFailure(captor.capture());
         lp = captor.getValue();
-    }}
+        assertNotNull(lp);
+        assertEquals(0, lp.getDnsServers().size());
+        reset(mCb);
+    }
+
+    @Test
+    public void testIpClientClearingIpAddressState() throws Exception {
+        final long currentTime = System.currentTimeMillis();
+        performDhcpHandshake(true /* isSuccessLease */, TEST_LEASE_DURATION_S,
+                true /* isDhcpLeaseCacheEnabled */, false /* isDhcpRapidCommitEnabled */,
+                TEST_DEFAULT_MTU);
+        assertIpMemoryStoreNetworkAttributes(TEST_LEASE_DURATION_S, currentTime, TEST_DEFAULT_MTU);
+
+        ArgumentCaptor<LinkProperties> captor = ArgumentCaptor.forClass(LinkProperties.class);
+        verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningSuccess(captor.capture());
+        LinkProperties lp = captor.getValue();
+        assertNotNull(lp);
+        assertEquals(1, lp.getAddresses().size());
+        assertTrue(lp.getAddresses().contains(InetAddress.getByName(CLIENT_ADDR.getHostAddress())));
+
+        // Stop IpClient and expect a final LinkProperties callback with an empty LP.
+        mIpc.stop();
+        verify(mCb, timeout(TEST_TIMEOUT_MS)).onLinkPropertiesChange(argThat(
+                x -> x.getAddresses().size() == 0
+                        && x.getRoutes().size() == 0
+                        && x.getDnsServers().size() == 0));
+        reset(mCb);
+
+        // Pretend that something else (e.g., Tethering) used the interface and left an IP address
+        // configured on it. When IpClient starts, it must clear this address before proceeding.
+        // TODO: test IPv6 instead, since the DHCP client will remove this address by replacing it
+        // with the new address.
+        mNetd.interfaceAddAddress(mIfaceName, "192.0.2.99", 26);
+
+        // start IpClient again and should enter Clearing State and wait for the message from kernel
+        performDhcpHandshake(true /* isSuccessLease */, TEST_LEASE_DURATION_S,
+                true /* isDhcpLeaseCacheEnabled */, false /* isDhcpRapidCommitEnabled */,
+                TEST_DEFAULT_MTU);
+
+        verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningSuccess(captor.capture());
+        lp = captor.getValue();
+        assertNotNull(lp);
+        assertEquals(1, lp.getAddresses().size());
+        assertTrue(lp.getAddresses().contains(InetAddress.getByName(CLIENT_ADDR.getHostAddress())));
+    }
+}