From 4465e4e2bd3b413066f16411b9e51ead773e3ca7 Mon Sep 17 00:00:00 2001
From: Mark <markchien@google.com>
Date: Tue, 29 Nov 2022 04:37:13 +0000
Subject: [PATCH] SyncSM09: Add Callback to UpstreamNetworkMonitor

Instead of passing state machine to UpstreamNetworkMonitor, wrapped it
by an interface would have two benefits:
1. Easier for testing.
2. Isolate state machine operation in callback.

Test: atest TetheringTests
Change-Id: I2a97fd5cbc8c49df8c3c98428c6570916ab31ea5
---
 .../networkstack/tethering/Tethering.java     | 11 ++-
 .../tethering/TetheringDependencies.java      |  7 +-
 .../tethering/UpstreamNetworkMonitor.java     | 55 +++++------
 .../networkstack/tethering/TetheringTest.java | 46 ++++-----
 .../tethering/UpstreamNetworkMonitorTest.java | 97 ++++++++-----------
 5 files changed, 99 insertions(+), 117 deletions(-)

diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java
index 996ee116eb..32773635a4 100644
--- a/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -327,8 +327,10 @@ public class Tethering {
                         return mConfig;
                     }
                 });
-        mUpstreamNetworkMonitor = mDeps.getUpstreamNetworkMonitor(mContext, mTetherMainSM, mLog,
-                TetherMainSM.EVENT_UPSTREAM_CALLBACK);
+        mUpstreamNetworkMonitor = mDeps.getUpstreamNetworkMonitor(mContext, mHandler, mLog,
+                (what, obj) -> {
+                    mTetherMainSM.sendMessage(TetherMainSM.EVENT_UPSTREAM_CALLBACK, what, 0, obj);
+                });
         mForwardedDownstreams = new HashSet<>();
 
         IntentFilter filter = new IntentFilter();
@@ -2879,4 +2881,9 @@ public class Tethering {
             } catch (RemoteException e) { }
         });
     }
+
+    @VisibleForTesting
+    public TetherMainSM getTetherMainSMForTesting() {
+        return mTetherMainSM;
+    }
 }
diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
index c274165680..d02e8e82fc 100644
--- a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
+++ b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
@@ -35,7 +35,6 @@ import android.text.TextUtils;
 import androidx.annotation.NonNull;
 import androidx.annotation.RequiresApi;
 
-import com.android.internal.util.StateMachine;
 import com.android.modules.utils.build.SdkLevel;
 import com.android.net.module.util.SdkUtil.LateSdk;
 import com.android.net.module.util.SharedLog;
@@ -84,9 +83,9 @@ public abstract class TetheringDependencies {
     /**
      * Get a reference to the UpstreamNetworkMonitor to be used by tethering.
      */
-    public UpstreamNetworkMonitor getUpstreamNetworkMonitor(Context ctx, StateMachine target,
-            SharedLog log, int what) {
-        return new UpstreamNetworkMonitor(ctx, target, log, what);
+    public UpstreamNetworkMonitor getUpstreamNetworkMonitor(Context ctx, Handler h,
+            SharedLog log, UpstreamNetworkMonitor.EventListener listener) {
+        return new UpstreamNetworkMonitor(ctx, h, log, listener);
     }
 
     /**
diff --git a/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java b/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
index ac2aa7bfea..7a05d749e7 100644
--- a/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
+++ b/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
@@ -44,7 +44,6 @@ import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
 import com.android.internal.annotations.VisibleForTesting;
-import com.android.internal.util.StateMachine;
 import com.android.net.module.util.SharedLog;
 import com.android.networkstack.apishim.ConnectivityManagerShimImpl;
 import com.android.networkstack.apishim.common.ConnectivityManagerShim;
@@ -111,9 +110,8 @@ public class UpstreamNetworkMonitor {
 
     private final Context mContext;
     private final SharedLog mLog;
-    private final StateMachine mTarget;
     private final Handler mHandler;
-    private final int mWhat;
+    private final EventListener mEventListener;
     private final HashMap<Network, UpstreamNetworkState> mNetworkMap = new HashMap<>();
     private HashSet<IpPrefix> mLocalPrefixes;
     private ConnectivityManager mCM;
@@ -135,12 +133,11 @@ public class UpstreamNetworkMonitor {
     private Network mDefaultInternetNetwork;
     private boolean mPreferTestNetworks;
 
-    public UpstreamNetworkMonitor(Context ctx, StateMachine tgt, SharedLog log, int what) {
+    public UpstreamNetworkMonitor(Context ctx, Handler h, SharedLog log, EventListener listener) {
         mContext = ctx;
-        mTarget = tgt;
-        mHandler = mTarget.getHandler();
+        mHandler = h;
         mLog = log.forSubComponent(TAG);
-        mWhat = what;
+        mEventListener = listener;
         mLocalPrefixes = new HashSet<>();
         mIsDefaultCellularUpstream = false;
         mCM = (ConnectivityManager) ctx.getSystemService(Context.CONNECTIVITY_SERVICE);
@@ -374,11 +371,12 @@ public class UpstreamNetworkMonitor {
                     network, newNc));
         }
 
-        mNetworkMap.put(network, new UpstreamNetworkState(
-                prev.linkProperties, newNc, network));
+        final UpstreamNetworkState uns =
+                new UpstreamNetworkState(prev.linkProperties, newNc, network);
+        mNetworkMap.put(network, uns);
         // TODO: If sufficient information is available to select a more
         // preferable upstream, do so now and notify the target.
-        notifyTarget(EVENT_ON_CAPABILITIES, network);
+        mEventListener.onUpstreamEvent(EVENT_ON_CAPABILITIES, uns);
     }
 
     private @Nullable UpstreamNetworkState updateLinkProperties(@NonNull Network network,
@@ -411,7 +409,7 @@ public class UpstreamNetworkMonitor {
     private void handleLinkProp(Network network, LinkProperties newLp) {
         final UpstreamNetworkState ns = updateLinkProperties(network, newLp);
         if (ns != null) {
-            notifyTarget(EVENT_ON_LINKPROPERTIES, ns);
+            mEventListener.onUpstreamEvent(EVENT_ON_LINKPROPERTIES, ns);
         }
     }
 
@@ -438,7 +436,7 @@ public class UpstreamNetworkMonitor {
         // preferable upstream, do so now and notify the target.  Likewise,
         // if the current upstream network is gone, notify the target of the
         // fact that we now have no upstream at all.
-        notifyTarget(EVENT_ON_LOST, mNetworkMap.remove(network));
+        mEventListener.onUpstreamEvent(EVENT_ON_LOST, mNetworkMap.remove(network));
     }
 
     private void maybeHandleNetworkSwitch(@NonNull Network network) {
@@ -456,14 +454,14 @@ public class UpstreamNetworkMonitor {
         // Default network changed. Update local data and notify tethering.
         Log.d(TAG, "New default Internet network: " + network);
         mDefaultInternetNetwork = network;
-        notifyTarget(EVENT_DEFAULT_SWITCHED, ns);
+        mEventListener.onUpstreamEvent(EVENT_DEFAULT_SWITCHED, ns);
     }
 
     private void recomputeLocalPrefixes() {
         final HashSet<IpPrefix> localPrefixes = allLocalPrefixes(mNetworkMap.values());
         if (!mLocalPrefixes.equals(localPrefixes)) {
             mLocalPrefixes = localPrefixes;
-            notifyTarget(NOTIFY_LOCAL_PREFIXES, localPrefixes.clone());
+            mEventListener.onUpstreamEvent(NOTIFY_LOCAL_PREFIXES, localPrefixes.clone());
         }
     }
 
@@ -502,12 +500,13 @@ public class UpstreamNetworkMonitor {
                 // onLinkPropertiesChanged right after this method and mDefaultInternetNetwork will
                 // be updated then.
                 //
-                // Technically, not updating here isn't necessary, because the notifications to
-                // Tethering sent by notifyTarget are messages sent to a state machine running on
-                // the same thread as this method, and so cannot arrive until after this method has
-                // returned. However, it is not a good idea to rely on that because fact that
-                // Tethering uses multiple state machines running on the same thread is a major
-                // source of race conditions and something that should be fixed.
+                // Technically, mDefaultInternetNetwork could be updated here, because the
+                // Callback#onChange implementation sends messages to the state machine running
+                // on the same thread as this method. If there is new default network change,
+                // the message cannot arrive until onLinkPropertiesChanged returns.
+                // However, it is not a good idea to rely on that because fact that Tethering uses
+                // multiple state machines running on the same thread is a major source of race
+                // conditions and something that should be fixed.
                 //
                 // TODO: is it correct that this code always updates EntitlementManager?
                 // This code runs when the default network connects or changes capabilities, but the
@@ -551,7 +550,7 @@ public class UpstreamNetworkMonitor {
                 mIsDefaultCellularUpstream = false;
                 mEntitlementMgr.notifyUpstream(false);
                 Log.d(TAG, "Lost default Internet network: " + network);
-                notifyTarget(EVENT_DEFAULT_SWITCHED, null);
+                mEventListener.onUpstreamEvent(EVENT_DEFAULT_SWITCHED, null);
                 return;
             }
 
@@ -569,14 +568,6 @@ public class UpstreamNetworkMonitor {
         if (cb != null) cm().unregisterNetworkCallback(cb);
     }
 
-    private void notifyTarget(int which, Network network) {
-        notifyTarget(which, mNetworkMap.get(network));
-    }
-
-    private void notifyTarget(int which, Object obj) {
-        mTarget.sendMessage(mWhat, which, 0, obj);
-    }
-
     private static class TypeStatePair {
         public int type = TYPE_NONE;
         public UpstreamNetworkState ns = null;
@@ -698,4 +689,10 @@ public class UpstreamNetworkMonitor {
     public void setPreferTestNetworks(boolean prefer) {
         mPreferTestNetworks = prefer;
     }
+
+    /** An interface to notify upstream network changes. */
+    public interface EventListener {
+        /** Notify the client of some event */
+        void onUpstreamEvent(int what, Object obj);
+    }
 }
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
index 6eba590a13..5877fc5393 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -187,7 +187,6 @@ import androidx.annotation.Nullable;
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
-import com.android.internal.util.StateMachine;
 import com.android.internal.util.test.BroadcastInterceptingContext;
 import com.android.internal.util.test.FakeSettingsProvider;
 import com.android.net.module.util.CollectionUtils;
@@ -310,6 +309,7 @@ public class TetheringTest {
     private BroadcastReceiver mBroadcastReceiver;
     private Tethering mTethering;
     private TestTetheringEventCallback mTetheringEventCallback;
+    private Tethering.TetherMainSM mTetherMainSM;
     private PhoneStateListener mPhoneStateListener;
     private InterfaceConfigurationParcel mInterfaceConfiguration;
     private TetheringConfiguration mConfig;
@@ -319,6 +319,7 @@ public class TetheringTest {
     private SoftApCallback mSoftApCallback;
     private SoftApCallback mLocalOnlyHotspotCallback;
     private UpstreamNetworkMonitor mUpstreamNetworkMonitor;
+    private UpstreamNetworkMonitor.EventListener mEventListener;
     private TetheredInterfaceCallbackShim mTetheredInterfaceCallbackShim;
 
     private TestConnectivityManager mCm;
@@ -432,7 +433,6 @@ public class TetheringTest {
     }
 
     public class MockTetheringDependencies extends TetheringDependencies {
-        StateMachine mUpstreamNetworkMonitorSM;
         ArrayList<IpServer> mAllDownstreams;
 
         @Override
@@ -456,12 +456,12 @@ public class TetheringTest {
         }
 
         @Override
-        public UpstreamNetworkMonitor getUpstreamNetworkMonitor(Context ctx,
-                StateMachine target, SharedLog log, int what) {
+        public UpstreamNetworkMonitor getUpstreamNetworkMonitor(Context ctx, Handler h,
+                SharedLog log, UpstreamNetworkMonitor.EventListener listener) {
             // Use a real object instead of a mock so that some tests can use a real UNM and some
             // can use a mock.
-            mUpstreamNetworkMonitorSM = target;
-            mUpstreamNetworkMonitor = spy(super.getUpstreamNetworkMonitor(ctx, target, log, what));
+            mEventListener = listener;
+            mUpstreamNetworkMonitor = spy(super.getUpstreamNetworkMonitor(ctx, h, log, listener));
             return mUpstreamNetworkMonitor;
         }
 
@@ -688,6 +688,7 @@ public class TetheringTest {
     private void initTetheringOnTestThread() throws Exception {
         mLooper = new TestLooper();
         mTethering = new Tethering(mTetheringDependencies);
+        mTetherMainSM = mTethering.getTetherMainSMForTesting();
         verify(mStatsManager, times(1)).registerNetworkStatsProvider(anyString(), any());
         verify(mNetd).registerUnsolicitedEventListener(any());
         verifyDefaultNetworkRequestFiled();
@@ -1182,10 +1183,7 @@ public class TetheringTest {
         initTetheringUpstream(upstreamState);
 
         // Upstream LinkProperties changed: UpstreamNetworkMonitor sends EVENT_ON_LINKPROPERTIES.
-        mTetheringDependencies.mUpstreamNetworkMonitorSM.sendMessage(
-                Tethering.TetherMainSM.EVENT_UPSTREAM_CALLBACK,
-                UpstreamNetworkMonitor.EVENT_ON_LINKPROPERTIES,
-                0,
+        mEventListener.onUpstreamEvent(UpstreamNetworkMonitor.EVENT_ON_LINKPROPERTIES,
                 upstreamState);
         mLooper.dispatchAll();
 
@@ -2713,14 +2711,12 @@ public class TetheringTest {
     @Test
     public void testUpstreamNetworkChanged() throws Exception {
         initTetheringOnTestThread();
-        final Tethering.TetherMainSM stateMachine = (Tethering.TetherMainSM)
-                mTetheringDependencies.mUpstreamNetworkMonitorSM;
         final InOrder inOrder = inOrder(mNotificationUpdater);
 
         // Gain upstream.
         final UpstreamNetworkState upstreamState = buildMobileIPv4UpstreamState();
         initTetheringUpstream(upstreamState);
-        stateMachine.chooseUpstreamType(true);
+        mTetherMainSM.chooseUpstreamType(true);
         mTetheringEventCallback.expectUpstreamChanged(upstreamState.network);
         inOrder.verify(mNotificationUpdater)
                 .onUpstreamCapabilitiesChanged(upstreamState.networkCapabilities);
@@ -2728,7 +2724,7 @@ public class TetheringTest {
         // Set the upstream with the same network ID but different object and the same capability.
         final UpstreamNetworkState upstreamState2 = buildMobileIPv4UpstreamState();
         initTetheringUpstream(upstreamState2);
-        stateMachine.chooseUpstreamType(true);
+        mTetherMainSM.chooseUpstreamType(true);
         // Expect that no upstream change event and capabilities changed event.
         mTetheringEventCallback.assertNoUpstreamChangeCallback();
         inOrder.verify(mNotificationUpdater, never()).onUpstreamCapabilitiesChanged(any());
@@ -2738,17 +2734,17 @@ public class TetheringTest {
         assertFalse(upstreamState3.networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED));
         upstreamState3.networkCapabilities.addCapability(NET_CAPABILITY_VALIDATED);
         initTetheringUpstream(upstreamState3);
-        stateMachine.chooseUpstreamType(true);
+        mTetherMainSM.chooseUpstreamType(true);
         // Expect that no upstream change event and capabilities changed event.
         mTetheringEventCallback.assertNoUpstreamChangeCallback();
-        stateMachine.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState3);
+        mTetherMainSM.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState3);
         inOrder.verify(mNotificationUpdater)
                 .onUpstreamCapabilitiesChanged(upstreamState3.networkCapabilities);
 
 
         // Lose upstream.
         initTetheringUpstream(null);
-        stateMachine.chooseUpstreamType(true);
+        mTetherMainSM.chooseUpstreamType(true);
         mTetheringEventCallback.expectUpstreamChanged(NULL_NETWORK);
         inOrder.verify(mNotificationUpdater).onUpstreamCapabilitiesChanged(null);
     }
@@ -2756,17 +2752,15 @@ public class TetheringTest {
     @Test
     public void testUpstreamCapabilitiesChanged() throws Exception {
         initTetheringOnTestThread();
-        final Tethering.TetherMainSM stateMachine = (Tethering.TetherMainSM)
-                mTetheringDependencies.mUpstreamNetworkMonitorSM;
         final InOrder inOrder = inOrder(mNotificationUpdater);
         final UpstreamNetworkState upstreamState = buildMobileIPv4UpstreamState();
         initTetheringUpstream(upstreamState);
 
-        stateMachine.chooseUpstreamType(true);
+        mTetherMainSM.chooseUpstreamType(true);
         inOrder.verify(mNotificationUpdater)
                 .onUpstreamCapabilitiesChanged(upstreamState.networkCapabilities);
 
-        stateMachine.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState);
+        mTetherMainSM.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState);
         inOrder.verify(mNotificationUpdater)
                 .onUpstreamCapabilitiesChanged(upstreamState.networkCapabilities);
 
@@ -2775,7 +2769,7 @@ public class TetheringTest {
         // Expect that capability is changed with new capability VALIDATED.
         assertFalse(upstreamState.networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED));
         upstreamState.networkCapabilities.addCapability(NET_CAPABILITY_VALIDATED);
-        stateMachine.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState);
+        mTetherMainSM.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState);
         inOrder.verify(mNotificationUpdater)
                 .onUpstreamCapabilitiesChanged(upstreamState.networkCapabilities);
 
@@ -2784,7 +2778,7 @@ public class TetheringTest {
         final UpstreamNetworkState upstreamState2 = new UpstreamNetworkState(
                 upstreamState.linkProperties, upstreamState.networkCapabilities,
                 new Network(WIFI_NETID));
-        stateMachine.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState2);
+        mTetherMainSM.handleUpstreamNetworkMonitorCallback(EVENT_ON_CAPABILITIES, upstreamState2);
         inOrder.verify(mNotificationUpdater, never()).onUpstreamCapabilitiesChanged(any());
     }
 
@@ -2907,11 +2901,7 @@ public class TetheringTest {
             final String iface, final int transportType) {
         final UpstreamNetworkState upstream = buildV4UpstreamState(ipv4Address, network, iface,
                 transportType);
-        mTetheringDependencies.mUpstreamNetworkMonitorSM.sendMessage(
-                Tethering.TetherMainSM.EVENT_UPSTREAM_CALLBACK,
-                UpstreamNetworkMonitor.EVENT_ON_LINKPROPERTIES,
-                0,
-                upstream);
+        mEventListener.onUpstreamEvent(UpstreamNetworkMonitor.EVENT_ON_LINKPROPERTIES, upstream);
         mLooper.dispatchAll();
     }
 
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java
index e756bd3d6c..045c0cb08a 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java
@@ -30,10 +30,12 @@ import static com.android.networkstack.tethering.UpstreamNetworkMonitor.TYPE_NON
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyInt;
 import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
@@ -51,27 +53,23 @@ import android.net.NetworkCapabilities;
 import android.net.NetworkRequest;
 import android.os.Build;
 import android.os.Handler;
-import android.os.Looper;
-import android.os.Message;
 import android.os.test.TestLooper;
 
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
-import com.android.internal.util.State;
-import com.android.internal.util.StateMachine;
 import com.android.net.module.util.SharedLog;
 import com.android.networkstack.tethering.TestConnectivityManager.NetworkRequestInfo;
 import com.android.networkstack.tethering.TestConnectivityManager.TestNetworkAgent;
 import com.android.testutils.DevSdkIgnoreRule;
 import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter;
 
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
+import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
@@ -84,8 +82,6 @@ import java.util.Set;
 @RunWith(AndroidJUnit4.class)
 @SmallTest
 public class UpstreamNetworkMonitorTest {
-    private static final int EVENT_UNM_UPDATE = 1;
-
     private static final boolean INCLUDES = true;
     private static final boolean EXCLUDES = false;
 
@@ -102,12 +98,13 @@ public class UpstreamNetworkMonitorTest {
     @Mock private EntitlementManager mEntitleMgr;
     @Mock private IConnectivityManager mCS;
     @Mock private SharedLog mLog;
+    @Mock private UpstreamNetworkMonitor.EventListener mListener;
 
-    private TestStateMachine mSM;
     private TestConnectivityManager mCM;
     private UpstreamNetworkMonitor mUNM;
 
     private final TestLooper mLooper = new TestLooper();
+    private InOrder mCallbackOrder;
 
     @Before public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
@@ -117,17 +114,11 @@ public class UpstreamNetworkMonitorTest {
         when(mLog.forSubComponent(anyString())).thenReturn(mLog);
         when(mEntitleMgr.isCellularUpstreamPermitted()).thenReturn(true);
 
+        mCallbackOrder = inOrder(mListener);
         mCM = spy(new TestConnectivityManager(mContext, mCS));
         when(mContext.getSystemService(eq(Context.CONNECTIVITY_SERVICE))).thenReturn(mCM);
-        mSM = new TestStateMachine(mLooper.getLooper());
-        mUNM = new UpstreamNetworkMonitor(mContext, mSM, mLog, EVENT_UNM_UPDATE);
-    }
-
-    @After public void tearDown() throws Exception {
-        if (mSM != null) {
-            mSM.quit();
-            mSM = null;
-        }
+        mUNM = new UpstreamNetworkMonitor(mContext, new Handler(mLooper.getLooper()), mLog,
+                mListener);
     }
 
     @Test
@@ -603,14 +594,17 @@ public class UpstreamNetworkMonitorTest {
         mCM.makeDefaultNetwork(cellAgent);
         mLooper.dispatchAll();
         verifyCurrentLinkProperties(cellAgent);
-        int messageIndex = mSM.messages.size() - 1;
+        verifyNotifyNetworkCapabilitiesChange(cellAgent.networkCapabilities);
+        verifyNotifyLinkPropertiesChange(cellLp);
+        verifyNotifyDefaultSwitch(cellAgent);
+        verifyNoMoreInteractions(mListener);
 
         addLinkAddresses(cellLp, ipv6Addr1);
         mCM.sendLinkProperties(cellAgent, false /* updateDefaultFirst */);
         mLooper.dispatchAll();
         verifyCurrentLinkProperties(cellAgent);
-        verifyNotifyLinkPropertiesChange(messageIndex);
-        messageIndex = mSM.messages.size() - 1;
+        verifyNotifyLinkPropertiesChange(cellLp);
+        verifyNoMoreInteractions(mListener);
 
         removeLinkAddresses(cellLp, ipv6Addr1);
         addLinkAddresses(cellLp, ipv6Addr2);
@@ -618,7 +612,8 @@ public class UpstreamNetworkMonitorTest {
         mLooper.dispatchAll();
         assertEquals(cellAgent.linkProperties, mUNM.getCurrentPreferredUpstream().linkProperties);
         verifyCurrentLinkProperties(cellAgent);
-        verifyNotifyLinkPropertiesChange(messageIndex);
+        verifyNotifyLinkPropertiesChange(cellLp);
+        verifyNoMoreInteractions(mListener);
     }
 
     private void verifyCurrentLinkProperties(TestNetworkAgent agent) {
@@ -626,12 +621,33 @@ public class UpstreamNetworkMonitorTest {
         assertEquals(agent.linkProperties, mUNM.getCurrentPreferredUpstream().linkProperties);
     }
 
-    private void verifyNotifyLinkPropertiesChange(int lastMessageIndex) {
-        assertEquals(UpstreamNetworkMonitor.EVENT_ON_LINKPROPERTIES,
-                mSM.messages.get(++lastMessageIndex).arg1);
-        assertEquals(UpstreamNetworkMonitor.NOTIFY_LOCAL_PREFIXES,
-                mSM.messages.get(++lastMessageIndex).arg1);
-        assertEquals(lastMessageIndex + 1, mSM.messages.size());
+    private void verifyNotifyNetworkCapabilitiesChange(final NetworkCapabilities cap) {
+        mCallbackOrder.verify(mListener).onUpstreamEvent(
+                eq(UpstreamNetworkMonitor.EVENT_ON_CAPABILITIES),
+                argThat(uns -> uns instanceof UpstreamNetworkState
+                    && cap.equals(((UpstreamNetworkState) uns).networkCapabilities)));
+
+    }
+
+    private void verifyNotifyLinkPropertiesChange(final LinkProperties lp) {
+        mCallbackOrder.verify(mListener).onUpstreamEvent(
+                eq(UpstreamNetworkMonitor.EVENT_ON_LINKPROPERTIES),
+                argThat(uns -> uns instanceof UpstreamNetworkState
+                    && lp.equals(((UpstreamNetworkState) uns).linkProperties)));
+
+        mCallbackOrder.verify(mListener).onUpstreamEvent(
+                eq(UpstreamNetworkMonitor.NOTIFY_LOCAL_PREFIXES), any());
+    }
+
+    private void verifyNotifyDefaultSwitch(TestNetworkAgent agent) {
+        mCallbackOrder.verify(mListener).onUpstreamEvent(
+                eq(UpstreamNetworkMonitor.EVENT_DEFAULT_SWITCHED),
+                argThat(uns ->
+                    uns instanceof UpstreamNetworkState
+                    && agent.networkId.equals(((UpstreamNetworkState) uns).network)
+                    && agent.linkProperties.equals(((UpstreamNetworkState) uns).linkProperties)
+                    && agent.networkCapabilities.equals(
+                        ((UpstreamNetworkState) uns).networkCapabilities)));
     }
 
     private void addLinkAddresses(LinkProperties lp, String... addrs) {
@@ -673,33 +689,6 @@ public class UpstreamNetworkMonitorTest {
         return false;
     }
 
-    public static class TestStateMachine extends StateMachine {
-        public final ArrayList<Message> messages = new ArrayList<>();
-        private final State mLoggingState = new LoggingState();
-
-        class LoggingState extends State {
-            @Override public void enter() {
-                messages.clear();
-            }
-
-            @Override public void exit() {
-                messages.clear();
-            }
-
-            @Override public boolean processMessage(Message msg) {
-                messages.add(msg);
-                return true;
-            }
-        }
-
-        public TestStateMachine(Looper looper) {
-            super("UpstreamNetworkMonitor.TestStateMachine", looper);
-            addState(mLoggingState);
-            setInitialState(mLoggingState);
-            super.start();
-        }
-    }
-
     static void assertPrefixSet(Set<IpPrefix> prefixes, boolean expectation, String... expected) {
         final Set<String> expectedSet = new HashSet<>();
         Collections.addAll(expectedSet, expected);
-- 
GitLab