Always send all available state when calling onAvailable().

The state that needs to be transferred includes:
    - NetworkCapabilities
    - LinkProperties
    - whether the network is currently suspended

Additionally:
    - Rename notifyNetworkCallback() to notifyNetworkAvailable()
      in order to clarify its real function.
    - fix previous copy/paste error in unittest

Test: as follows
    - built (bullhead)
    - flashed
    - booted
    - runtest frameworks-net passes
    - USB tethering with mobile and Wi-Fi upstream toggling
Bug: 32163131
Change-Id: Ib4460bcd5d08863a9feac9e8ab41a238897bb3ea

diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index 30afdc2..81dbc7e 100644 (file)
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -1061,6 +1061,8 @@ public class ConnectivityManager {
      * registerNetworkCallback() are not specific to any particular Network so
      * do not cause any updates.
      *
+     * TODO: Delete once callers are updated.
+     *
      * @hide
      */
     public void requestLinkProperties(NetworkCallback networkCallback) {
@@ -1098,6 +1100,8 @@ public class ConnectivityManager {
      * registerNetworkCallback() are not specific to any particular Network so
      * do not cause any updates.
      *
+     * TODO: Delete once callers are updated.
+     *
      * @hide
      */
     public void requestNetworkCapabilities(NetworkCallback networkCallback) {

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 954a94e..d907035 100644 (file)
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -4348,6 +4348,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_LISTENER, nri));
     }
 
+    // TODO: Delete once callers are updated.
     @Override
     public void requestLinkProperties(NetworkRequest networkRequest) {
         ensureNetworkRequestHasType(networkRequest);
@@ -4356,6 +4357,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 EVENT_REQUEST_LINKPROPERTIES, getCallingUid(), 0, networkRequest));
     }
 
+    // TODO: Delete once callers are updated.
     @Override
     public void requestNetworkCapabilities(NetworkRequest networkRequest) {
         ensureNetworkRequestHasType(networkRequest);
@@ -4856,7 +4858,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             if (!nr.isListen()) continue;
             if (nai.satisfies(nr) && !nai.isSatisfyingRequest(nr.requestId)) {
                 nai.addRequest(nr);
-                notifyNetworkCallback(nai, nri);
+                notifyNetworkAvailable(nai, nri);
             }
         }
     }
@@ -5038,7 +5040,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
         // do this after the default net is switched, but
         // before LegacyTypeTracker sends legacy broadcasts
-        for (NetworkRequestInfo nri : addedRequests) notifyNetworkCallback(newNetwork, nri);
+        for (NetworkRequestInfo nri : addedRequests) notifyNetworkAvailable(newNetwork, nri);
 
         // Linger any networks that are no longer needed. This should be done after sending the
         // available callback for newNetwork.
@@ -5201,7 +5203,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
 
     private void updateNetworkInfo(NetworkAgentInfo networkAgent, NetworkInfo newInfo) {
-        NetworkInfo.State state = newInfo.getState();
+        final NetworkInfo.State state = newInfo.getState();
         NetworkInfo oldInfo = null;
         final int oldScore = networkAgent.getCurrentScore();
         synchronized (networkAgent) {
@@ -5328,15 +5330,27 @@ public class ConnectivityService extends IConnectivityManager.Stub
         sendUpdatedScoreToFactories(nai);
     }
 
-    // notify only this one new request of the current state
-    protected void notifyNetworkCallback(NetworkAgentInfo nai, NetworkRequestInfo nri) {
-        int notifyType = ConnectivityManager.CALLBACK_AVAILABLE;
+    // Notify only this one new request of the current state. Transfer all the
+    // current state by calling NetworkCapabilities and LinkProperties callbacks
+    // so that callers can be guaranteed to have as close to atomicity in state
+    // transfer as can be supported by this current API.
+    protected void notifyNetworkAvailable(NetworkAgentInfo nai, NetworkRequestInfo nri) {
         mHandler.removeMessages(EVENT_TIMEOUT_NETWORK_REQUEST, nri);
-        if (nri.mPendingIntent == null) {
-            callCallbackForRequest(nri, nai, notifyType, 0);
-        } else {
-            sendPendingIntentForRequest(nri, nai, notifyType);
+        if (nri.mPendingIntent != null) {
+            sendPendingIntentForRequest(nri, nai, ConnectivityManager.CALLBACK_AVAILABLE);
+            // Attempt no subsequent state pushes where intents are involved.
+            return;
+        }
+
+        callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_AVAILABLE, 0);
+        // Whether a network is currently suspended is also an important
+        // element of state to be transferred (it would not otherwise be
+        // delivered by any currently available mechanism).
+        if (nai.networkInfo.getState() == NetworkInfo.State.SUSPENDED) {
+            callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_SUSPENDED, 0);
         }
+        callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_CAP_CHANGED, 0);
+        callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_IP_CHANGED, 0);
     }
 
     private void sendLegacyNetworkBroadcast(NetworkAgentInfo nai, DetailedState state, int type) {

diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 52d2b63..52bde7c 100644 (file)
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -1170,15 +1170,11 @@ public class ConnectivityServiceTest extends AndroidTestCase {
 
         void expectAvailableCallbacks(MockNetworkAgent agent, boolean expectSuspended, int timeoutMs) {
             expectCallback(CallbackState.AVAILABLE, agent, timeoutMs);
-
-            final boolean HAS_DATASYNC_ON_AVAILABLE = false;
-            if (HAS_DATASYNC_ON_AVAILABLE) {
-                if (expectSuspended) {
-                    expectCallback(CallbackState.SUSPENDED, agent, timeoutMs);
-                }
-                expectCallback(CallbackState.NETWORK_CAPABILITIES, agent, timeoutMs);
-                expectCallback(CallbackState.LINK_PROPERTIES, agent, timeoutMs);
+            if (expectSuspended) {
+                expectCallback(CallbackState.SUSPENDED, agent, timeoutMs);
             }
+            expectCallback(CallbackState.NETWORK_CAPABILITIES, agent, timeoutMs);
+            expectCallback(CallbackState.LINK_PROPERTIES, agent, timeoutMs);
         }
 
         void expectAvailableCallbacks(MockNetworkAgent agent) {
@@ -1190,7 +1186,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         }
 
         void expectAvailableAndValidatedCallbacks(MockNetworkAgent agent) {
-            expectAvailableCallbacks(agent, true, TIMEOUT_MS);
+            expectAvailableCallbacks(agent, false, TIMEOUT_MS);
             expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, agent);
         }