Send empty LinkProperties when entering StoppedState.

Additionally, no longer try to transition from within a State's
enter() method (this can encounter Log.wtf()s). Introduce some
CMD_JUMP_* commands and use deferMessage().

Test: as follows
    - built
    - flashed
    - booted
    - runtest frameworks-net passes (except for IpConnectivityMetricsTest failures)
    - manual changing from DHCP to static configurations works:
  2017-12-11T19:06:19.082 - INVOKE onLinkPropertiesChange({{InterfaceName: wlan0 LinkAddresses: []  Routes: [] DnsAddresses: [] Domains: null MTU: 0}})

Bug: 69800563
Bug: 70394432
Change-Id: Ice249a48b66806c0270ec3f11dd2e8e387d4e29b

diff --git a/services/net/java/android/net/ip/IpClient.java b/services/net/java/android/net/ip/IpClient.java
index fdb366c..d1d02a1 100644 (file)
--- a/services/net/java/android/net/ip/IpClient.java
+++ b/services/net/java/android/net/ip/IpClient.java
@@ -163,10 +163,10 @@ public class IpClient extends StateMachine {
     // TODO: Find an lighter weight approach.
     private class LoggingCallbackWrapper extends Callback {
         private static final String PREFIX = "INVOKE ";
-        private Callback mCallback;
+        private final Callback mCallback;
 
         public LoggingCallbackWrapper(Callback callback) {
-            mCallback = callback;
+            mCallback = (callback != null) ? callback : new Callback();
         }
 
         private void log(String msg) {
@@ -524,6 +524,13 @@ public class IpClient extends StateMachine {
     private static final int EVENT_PROVISIONING_TIMEOUT           = 10;
     private static final int EVENT_DHCPACTION_TIMEOUT             = 11;
 
+    // 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_STARTING_TO_RUNNING         = 100;
+    private static final int CMD_JUMP_RUNNING_TO_STOPPING         = 101;
+    private static final int CMD_JUMP_STOPPING_TO_STOPPED         = 102;
+
     private static final int MAX_LOG_RECORDS = 500;
     private static final int MAX_PACKET_RECORDS = 100;
 
@@ -1284,6 +1291,9 @@ public class IpClient extends StateMachine {
 
             resetLinkProperties();
             if (mStartTimeMillis > 0) {
+                // Completed a life-cycle; send a final empty LinkProperties
+                // (cleared in resetLinkProperties() above) and record an event.
+                mCallback.onLinkPropertiesChange(new LinkProperties(mLinkProperties));
                 recordMetric(IpManagerEvent.COMPLETE_LIFECYCLE);
                 mStartTimeMillis = 0;
             }
@@ -1342,13 +1352,17 @@ public class IpClient extends StateMachine {
         public void enter() {
             if (mDhcpClient == null) {
                 // There's no DHCPv4 for which to wait; proceed to stopped.
-                transitionTo(mStoppedState);
+                deferMessage(obtainMessage(CMD_JUMP_STOPPING_TO_STOPPED));
             }
         }
 
         @Override
         public boolean processMessage(Message msg) {
             switch (msg.what) {
+                case CMD_JUMP_STOPPING_TO_STOPPED:
+                    transitionTo(mStoppedState);
+                    break;
+
                 case CMD_STOP:
                     break;
 
@@ -1382,7 +1396,7 @@ public class IpClient extends StateMachine {
             }
 
             if (readyToProceed()) {
-                transitionTo(mRunningState);
+                deferMessage(obtainMessage(CMD_JUMP_STARTING_TO_RUNNING));
             } else {
                 // Clear all IPv4 and IPv6 before proceeding to RunningState.
                 // Clean up any leftover state from an abnormal exit from
@@ -1399,6 +1413,10 @@ public class IpClient extends StateMachine {
         @Override
         public boolean processMessage(Message msg) {
             switch (msg.what) {
+                case CMD_JUMP_STARTING_TO_RUNNING:
+                    transitionTo(mRunningState);
+                    break;
+
                 case CMD_STOP:
                     transitionTo(mStoppingState);
                     break;
@@ -1427,7 +1445,7 @@ public class IpClient extends StateMachine {
             return HANDLED;
         }
 
-        boolean readyToProceed() {
+        private boolean readyToProceed() {
             return (!mLinkProperties.hasIPv4Address() &&
                     !mLinkProperties.hasGlobalIPv6Address());
         }
@@ -1459,13 +1477,13 @@ public class IpClient extends StateMachine {
 
             if (mConfiguration.mEnableIPv6 && !startIPv6()) {
                 doImmediateProvisioningFailure(IpManagerEvent.ERROR_STARTING_IPV6);
-                transitionTo(mStoppingState);
+                enqueueJumpToStoppingState();
                 return;
             }
 
             if (mConfiguration.mEnableIPv4 && !startIPv4()) {
                 doImmediateProvisioningFailure(IpManagerEvent.ERROR_STARTING_IPV4);
-                transitionTo(mStoppingState);
+                enqueueJumpToStoppingState();
                 return;
             }
 
@@ -1473,14 +1491,14 @@ public class IpClient extends StateMachine {
             if ((config != null) && !applyInitialConfig(config)) {
                 // TODO introduce a new IpManagerEvent constant to distinguish this error case.
                 doImmediateProvisioningFailure(IpManagerEvent.ERROR_INVALID_PROVISIONING);
-                transitionTo(mStoppingState);
+                enqueueJumpToStoppingState();
                 return;
             }
 
             if (mConfiguration.mUsingIpReachabilityMonitor && !startIpReachabilityMonitor()) {
                 doImmediateProvisioningFailure(
                         IpManagerEvent.ERROR_STARTING_IPREACHABILITYMONITOR);
-                transitionTo(mStoppingState);
+                enqueueJumpToStoppingState();
                 return;
             }
         }
@@ -1512,6 +1530,10 @@ public class IpClient extends StateMachine {
             resetLinkProperties();
         }
 
+        private void enqueueJumpToStoppingState() {
+            deferMessage(obtainMessage(CMD_JUMP_RUNNING_TO_STOPPING));
+        }
+
         private ConnectivityPacketTracker createPacketTracker() {
             try {
                 return new ConnectivityPacketTracker(
@@ -1542,6 +1564,7 @@ public class IpClient extends StateMachine {
         @Override
         public boolean processMessage(Message msg) {
             switch (msg.what) {
+                case CMD_JUMP_RUNNING_TO_STOPPING:
                 case CMD_STOP:
                     transitionTo(mStoppingState);
                     break;

diff --git a/tests/net/java/android/net/ip/IpManagerTest.java b/tests/net/java/android/net/ip/IpManagerTest.java
index 22d88fb..7c3c427 100644 (file)
--- a/tests/net/java/android/net/ip/IpManagerTest.java
+++ b/tests/net/java/android/net/ip/IpManagerTest.java
@@ -69,6 +69,8 @@ import java.util.Set;
 
 /**
  * Tests for IpManager.
+ *
+ * TODO: Rename to IpClientTest.
  */
 @RunWith(AndroidJUnit4.class)
 @SmallTest
@@ -111,9 +113,18 @@ public class IpManagerTest {
         verify(mNMService, times(1)).registerObserver(arg.capture());
         mObserver = arg.getValue();
         reset(mNMService);
+        // Verify IpClient doesn't call onLinkPropertiesChange() when it starts.
+        verify(mCb, never()).onLinkPropertiesChange(any());
+        reset(mCb);
         return ipm;
     }
 
+    private static LinkProperties makeEmptyLinkProperties(String iface) {
+        final LinkProperties empty = new LinkProperties();
+        empty.setInterfaceName(iface);
+        return empty;
+    }
+
     @Test
     public void testNullCallbackDoesNotThrow() throws Exception {
         final IpManager ipm = new IpManager(mContext, "lo", null, mNMService);
@@ -144,6 +155,8 @@ public class IpManagerTest {
         ipm.stop();
         verify(mNMService, timeout(100).times(1)).disableIpv6(iface);
         verify(mNMService, timeout(100).times(1)).clearInterfaceAddresses(iface);
+        verify(mCb, timeout(100).times(1))
+                .onLinkPropertiesChange(eq(makeEmptyLinkProperties(iface)));
     }
 
     @Test
@@ -193,6 +206,8 @@ public class IpManagerTest {
         ipm.stop();
         verify(mNMService, timeout(100).times(1)).disableIpv6(iface);
         verify(mNMService, timeout(100).times(1)).clearInterfaceAddresses(iface);
+        verify(mCb, timeout(100).times(1))
+                .onLinkPropertiesChange(eq(makeEmptyLinkProperties(iface)));
     }
 
     @Test