Merge "Nat464Xlat: rely on netd events being called on handler thread." into main
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index f888da5..3ae3e2d 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -11418,7 +11418,7 @@
         public void onInterfaceLinkStateChanged(@NonNull String iface, boolean up) {
             mHandler.post(() -> {
                 for (NetworkAgentInfo nai : mNetworkAgentInfos) {
-                    nai.clatd.interfaceLinkStateChanged(iface, up);
+                    nai.clatd.handleInterfaceLinkStateChanged(iface, up);
                 }
             });
         }
@@ -11427,7 +11427,7 @@
         public void onInterfaceRemoved(@NonNull String iface) {
             mHandler.post(() -> {
                 for (NetworkAgentInfo nai : mNetworkAgentInfos) {
-                    nai.clatd.interfaceRemoved(iface);
+                    nai.clatd.handleInterfaceRemoved(iface);
                 }
             });
         }
diff --git a/service/src/com/android/server/connectivity/Nat464Xlat.java b/service/src/com/android/server/connectivity/Nat464Xlat.java
index f9e07fd..065922d 100644
--- a/service/src/com/android/server/connectivity/Nat464Xlat.java
+++ b/service/src/com/android/server/connectivity/Nat464Xlat.java
@@ -483,8 +483,9 @@
 
     /**
      * Adds stacked link on base link and transitions to RUNNING state.
+     * Must be called on the handler thread.
      */
-    private void handleInterfaceLinkStateChanged(String iface, boolean up) {
+    public void handleInterfaceLinkStateChanged(String iface, boolean up) {
         // TODO: if we call start(), then stop(), then start() again, and the
         // interfaceLinkStateChanged notification for the first start is delayed past the first
         // stop, then the code becomes out of sync with system state and will behave incorrectly.
@@ -499,6 +500,7 @@
         // Once this code is converted to StateMachine, it will be possible to use deferMessage to
         // ensure it stays in STARTING state until the interfaceLinkStateChanged notification fires,
         // and possibly use a timeout (or provide some guarantees at the lower layer) to address #1.
+        ensureRunningOnHandlerThread();
         if (!isStarting() || !up || !Objects.equals(mIface, iface)) {
             return;
         }
@@ -519,8 +521,10 @@
 
     /**
      * Removes stacked link on base link and transitions to IDLE state.
+     * Must be called on the handler thread.
      */
-    private void handleInterfaceRemoved(String iface) {
+    public void handleInterfaceRemoved(String iface) {
+        ensureRunningOnHandlerThread();
         if (!Objects.equals(mIface, iface)) {
             return;
         }
@@ -536,14 +540,6 @@
         stop();
     }
 
-    public void interfaceLinkStateChanged(String iface, boolean up) {
-        mNetwork.handler().post(() -> { handleInterfaceLinkStateChanged(iface, up); });
-    }
-
-    public void interfaceRemoved(String iface) {
-        mNetwork.handler().post(() -> handleInterfaceRemoved(iface));
-    }
-
     /**
      * Translate the input v4 address to v6 clat address.
      */
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 2fccdcb..c8cbce1 100755
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -10772,6 +10772,8 @@
         final RouteInfo ipv4Subnet = new RouteInfo(myIpv4, null, MOBILE_IFNAME);
         final RouteInfo stackedDefault =
                 new RouteInfo((IpPrefix) null, myIpv4.getAddress(), CLAT_MOBILE_IFNAME);
+        final BaseNetdUnsolicitedEventListener netdUnsolicitedListener =
+                getRegisteredNetdUnsolicitedEventListener();
 
         final NetworkRequest networkRequest = new NetworkRequest.Builder()
                 .addTransportType(TRANSPORT_CELLULAR)
@@ -10839,7 +10841,6 @@
         assertRoutesRemoved(cellNetId, ipv4Subnet);
 
         // When NAT64 prefix discovery succeeds, LinkProperties are updated and clatd is started.
-        Nat464Xlat clat = getNat464Xlat(mCellAgent);
         assertNull(mCm.getLinkProperties(mCellAgent.getNetwork()).getNat64Prefix());
         mService.mResolverUnsolEventCallback.onNat64PrefixEvent(
                 makeNat64PrefixEvent(cellNetId, PREFIX_OPERATION_ADDED, kNat64PrefixString, 96));
@@ -10850,7 +10851,8 @@
         verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString());
 
         // Clat iface comes up. Expect stacked link to be added.
-        clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
+        netdUnsolicitedListener.onInterfaceLinkStateChanged(
+                CLAT_MOBILE_IFNAME, true);
         networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent);
         List<LinkProperties> stackedLps = mCm.getLinkProperties(mCellAgent.getNetwork())
                 .getStackedLinks();
@@ -10896,7 +10898,7 @@
                 kOtherNat64Prefix.toString());
         networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent,
                 cb -> cb.getLp().getNat64Prefix().equals(kOtherNat64Prefix));
-        clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
+        netdUnsolicitedListener.onInterfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
         networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent,
                 cb -> cb.getLp().getStackedLinks().size() == 1);
         assertRoutesAdded(cellNetId, stackedDefault);
@@ -10924,7 +10926,7 @@
         assertRoutesRemoved(cellNetId, stackedDefault);
 
         // The interface removed callback happens but has no effect after stop is called.
-        clat.interfaceRemoved(CLAT_MOBILE_IFNAME);
+        netdUnsolicitedListener.onInterfaceRemoved(CLAT_MOBILE_IFNAME);
         networkCallback.assertNoCallback();
         verify(mMockNetd, times(1)).networkRemoveInterface(cellNetId, CLAT_MOBILE_IFNAME);
 
@@ -10961,7 +10963,7 @@
         verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString());
 
         // Clat iface comes up. Expect stacked link to be added.
-        clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
+        netdUnsolicitedListener.onInterfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true);
         networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent,
                 cb -> cb.getLp().getStackedLinks().size() == 1
                         && cb.getLp().getNat64Prefix() != null);
@@ -11029,8 +11031,7 @@
 
         // Clatd is started and clat iface comes up. Expect stacked link to be added.
         verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString());
-        clat = getNat464Xlat(mCellAgent);
-        clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true /* up */);
+        netdUnsolicitedListener.onInterfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true /* up */);
         networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent,
                 cb -> cb.getLp().getStackedLinks().size() == 1
                         && cb.getLp().getNat64Prefix().equals(kNat64Prefix));
diff --git a/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java b/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java
index 58c0114..2fe8713 100644
--- a/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java
+++ b/tests/unit/java/com/android/server/connectivity/Nat464XlatTest.java
@@ -86,7 +86,6 @@
     @Mock ClatCoordinator mClatCoordinator;
 
     TestLooper mLooper;
-    Handler mHandler;
     NetworkAgentConfig mAgentConfig = new NetworkAgentConfig();
 
     Nat464Xlat makeNat464Xlat(boolean isCellular464XlatEnabled) {
@@ -96,6 +95,14 @@
             }
         };
 
+        // The test looper needs to be created here on the test case thread and not in setUp,
+        // because setUp and test cases are run in different threads. Creating the test looper in
+        // setUp would make Looper.getThread() return the setUp thread, which does not match the
+        // test case thread that is actually used to process the messages.
+        mLooper = new TestLooper();
+        final Handler handler = new Handler(mLooper.getLooper());
+        doReturn(handler).when(mNai).handler();
+
         return new Nat464Xlat(mNai, mNetd, mDnsResolver, deps) {
             @Override protected int getNetId() {
                 return NETID;
@@ -117,9 +124,6 @@
 
     @Before
     public void setUp() throws Exception {
-        mLooper = new TestLooper();
-        mHandler = new Handler(mLooper.getLooper());
-
         MockitoAnnotations.initMocks(this);
 
         mNai.linkProperties = new LinkProperties();
@@ -130,7 +134,6 @@
         markNetworkConnected();
         when(mNai.connService()).thenReturn(mConnectivity);
         when(mNai.netAgentConfig()).thenReturn(mAgentConfig);
-        when(mNai.handler()).thenReturn(mHandler);
         final InterfaceConfigurationParcel mConfig = new InterfaceConfigurationParcel();
         when(mNetd.interfaceGetCfg(eq(STACKED_IFACE))).thenReturn(mConfig);
         mConfig.ipv4Addr = ADDR.getAddress().getHostAddress();
@@ -272,8 +275,7 @@
         verifyClatdStart(null /* inOrder */);
 
         // Stacked interface up notification arrives.
-        nat.interfaceLinkStateChanged(STACKED_IFACE, true);
-        mLooper.dispatchNext();
+        nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
 
         verify(mNetd).interfaceGetCfg(eq(STACKED_IFACE));
         verify(mConnectivity).handleUpdateLinkProperties(eq(mNai), c.capture());
@@ -294,8 +296,7 @@
         // Verify the generated v6 is reset when clat is stopped.
         assertNull(nat.mIPv6Address);
         // Stacked interface removed notification arrives and is ignored.
-        nat.interfaceRemoved(STACKED_IFACE);
-        mLooper.dispatchNext();
+        nat.handleInterfaceRemoved(STACKED_IFACE);
 
         verifyNoMoreInteractions(mNetd, mConnectivity);
     }
@@ -324,8 +325,7 @@
         verifyClatdStart(inOrder);
 
         // Stacked interface up notification arrives.
-        nat.interfaceLinkStateChanged(STACKED_IFACE, true);
-        mLooper.dispatchNext();
+        nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
 
         inOrder.verify(mConnectivity).handleUpdateLinkProperties(eq(mNai), c.capture());
         assertFalse(c.getValue().getStackedLinks().isEmpty());
@@ -344,10 +344,8 @@
 
         if (interfaceRemovedFirst) {
             // Stacked interface removed notification arrives and is ignored.
-            nat.interfaceRemoved(STACKED_IFACE);
-            mLooper.dispatchNext();
-            nat.interfaceLinkStateChanged(STACKED_IFACE, false);
-            mLooper.dispatchNext();
+            nat.handleInterfaceRemoved(STACKED_IFACE);
+            nat.handleInterfaceLinkStateChanged(STACKED_IFACE, false);
         }
 
         assertTrue(c.getValue().getStackedLinks().isEmpty());
@@ -361,15 +359,12 @@
 
         if (!interfaceRemovedFirst) {
             // Stacked interface removed notification arrives and is ignored.
-            nat.interfaceRemoved(STACKED_IFACE);
-            mLooper.dispatchNext();
-            nat.interfaceLinkStateChanged(STACKED_IFACE, false);
-            mLooper.dispatchNext();
+            nat.handleInterfaceRemoved(STACKED_IFACE);
+            nat.handleInterfaceLinkStateChanged(STACKED_IFACE, false);
         }
 
         // Stacked interface up notification arrives.
-        nat.interfaceLinkStateChanged(STACKED_IFACE, true);
-        mLooper.dispatchNext();
+        nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
 
         inOrder.verify(mConnectivity).handleUpdateLinkProperties(eq(mNai), c.capture());
         assertFalse(c.getValue().getStackedLinks().isEmpty());
@@ -411,8 +406,7 @@
         verifyClatdStart(null /* inOrder */);
 
         // Stacked interface up notification arrives.
-        nat.interfaceLinkStateChanged(STACKED_IFACE, true);
-        mLooper.dispatchNext();
+        nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
 
         verify(mNetd).interfaceGetCfg(eq(STACKED_IFACE));
         verify(mConnectivity, times(1)).handleUpdateLinkProperties(eq(mNai), c.capture());
@@ -421,8 +415,7 @@
         assertRunning(nat);
 
         // Stacked interface removed notification arrives (clatd crashed, ...).
-        nat.interfaceRemoved(STACKED_IFACE);
-        mLooper.dispatchNext();
+        nat.handleInterfaceRemoved(STACKED_IFACE);
 
         verifyClatdStop(null /* inOrder */);
         verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture());
@@ -457,12 +450,10 @@
         assertIdle(nat);
 
         // In-flight interface up notification arrives: no-op
-        nat.interfaceLinkStateChanged(STACKED_IFACE, true);
-        mLooper.dispatchNext();
+        nat.handleInterfaceLinkStateChanged(STACKED_IFACE, true);
 
         // Interface removed notification arrives after stopClatd() takes effect: no-op.
-        nat.interfaceRemoved(STACKED_IFACE);
-        mLooper.dispatchNext();
+        nat.handleInterfaceRemoved(STACKED_IFACE);
 
         assertIdle(nat);