Merge "Revert "Send empty LinkProperties when entering StoppedState.""

This commit is contained in:
Erik Kline
2017-12-15 07:09:24 +00:00
committed by Gerrit Code Review
2 changed files with 9 additions and 47 deletions

View File

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

View File

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