Fix propagating underlying caps when a network disconnects.

aosp/1513052, which generalized support for underlying networks,
broke default network switching when the network underlying a VPN
disconnects.

This is because it calls propagateUnderlyingNetworkCapabilities
in the middle of the bookkeeping operations needed when a
network is disconnected (specifically, after all satisified
requests are removed from the disconnecting network, but before
mDefaultNetworkNai is updated). This is completely incorrect
because propagateUnderlyingNetworkCapabilities can trigger a
network rematch, and running a rematch when the request data
structures are inconsistent is obviously wrong. See the test
changes in this CL for an example of the damage.

Fix this by moving propagateUnderlyingNetworkCapabilities to
before the bookeeping operations begin. It must be before
mDefaultNetworkNai is updated, because otherwise it will not know
that the default network is disconnecting, and it will not be
able to propagate capabilities to VPNs that set underlying
networks to null (i.e., to the default network). It must be
after the nai is removed from mNetworkForNetId because
otherwise it will think that the underlying network is still
connected.

Bug: 173331190
Test: accompanying unit test shows lots of bugs removed
Change-Id: Ibf376a6fa4b34d1c96f8506fa8abbb7595a8c272
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 90a17e7..9012fbc 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3466,6 +3466,7 @@
             // available until we've told netd to delete it below.
             mNetworkForNetId.remove(nai.network.getNetId());
         }
+        propagateUnderlyingNetworkCapabilities(nai.network);
         // Remove all previously satisfied requests.
         for (int i = 0; i < nai.numNetworkRequests(); i++) {
             NetworkRequest request = nai.requestAt(i);
@@ -3478,7 +3479,6 @@
             }
         }
         nai.clearLingerState();
-        propagateUnderlyingNetworkCapabilities(nai.network);
         if (nai.isSatisfyingRequest(mDefaultRequest.requestId)) {
             mDefaultNetworkNai = null;
             updateDataActivityTracking(null /* newNetwork */, nai);
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index c652261..bca4986 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -5981,32 +5981,40 @@
         assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
         assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
         assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED);  // BUG: VPN caps have NOT_SUSPENDED.
-        assertNull(mCm.getActiveNetworkInfo());  // ???
+        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
         // BUG: the device has connectivity, so this should return true.
         assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
 
-        // Suspending and resuming reveals other bugs.
+        // Re-suspending the current network fixes the problem.
         mCellNetworkAgent.suspend();
-        callback.assertNoCallback();  // BUG: should get callback that VPN is suspended.
+        callback.expectCapabilitiesThat(mMockVpn,
+                nc -> !nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
+                        && nc.hasTransport(TRANSPORT_CELLULAR));
+        callback.expectCallback(CallbackEntry.SUSPENDED, mMockVpn);
+        callback.assertNoCallback();
 
-        assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
-                .hasCapability(NET_CAPABILITY_NOT_SUSPENDED));  // BUG: VPN should be SUSPENDED.
+        assertFalse(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
+                .hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
         assertNetworkInfo(TYPE_MOBILE, DetailedState.SUSPENDED);
         assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
         assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED);
-        assertNull(mCm.getActiveNetworkInfo());  // ???
+        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.SUSPENDED);
         assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
 
         mCellNetworkAgent.resume();
-        callback.assertNoCallback();  // BUG: should get callback that VPN is no longer suspended.
+        callback.expectCapabilitiesThat(mMockVpn,
+                nc -> nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)
+                        && nc.hasTransport(TRANSPORT_CELLULAR));
+        callback.expectCallback(CallbackEntry.RESUMED, mMockVpn);
+        callback.assertNoCallback();
 
         assertTrue(mCm.getNetworkCapabilities(mMockVpn.getNetwork())
                 .hasCapability(NET_CAPABILITY_NOT_SUSPENDED));
         assertNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
         assertNetworkInfo(TYPE_WIFI, DetailedState.DISCONNECTED);
-        assertNetworkInfo(TYPE_VPN, DetailedState.SUSPENDED);
-        assertNull(mCm.getActiveNetworkInfo());  // ???
-        assertGetNetworkInfoOfGetActiveNetworkIsConnected(false);
+        assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED);
+        assertActiveNetworkInfo(TYPE_MOBILE, DetailedState.CONNECTED);
+        assertGetNetworkInfoOfGetActiveNetworkIsConnected(true);
     }
 
     @Test