Merge "Fix race when starting NetworkMonitor"

am: ef8485cc65

Change-Id: I3fe792704b9dfaafe09c83da57081d6668216a97

services/core/java/com/android/server/ConnectivityService.java

@@ -5392,7 +5392,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 mContext, mTrackerHandler, new NetworkMisc(networkMisc), this, mNetd, mDnsResolver,
                 mNMS, factorySerialNumber);
         // Make sure the network capabilities reflect what the agent info says.
-        nai.networkCapabilities = mixInCapabilities(nai, nc);
+        nai.setNetworkCapabilities(mixInCapabilities(nai, nc));
         final String extraInfo = networkInfo.getExtraInfo();
         final String name = TextUtils.isEmpty(extraInfo)
                 ? nai.networkCapabilities.getSSID() : extraInfo;
@@ -5485,12 +5485,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
             // Start or stop DNS64 detection and 464xlat according to network state.
             networkAgent.clatd.update();
             notifyIfacesChangedForNetworkStats();
-            if (networkAgent.everConnected) {
             try {
-                    networkAgent.networkMonitor().notifyLinkPropertiesChanged();
+                networkAgent.networkMonitor().notifyLinkPropertiesChanged(newLp);
             } catch (RemoteException e) {
                 e.rethrowFromSystemServer();
             }
+            if (networkAgent.everConnected) {
                 notifyNetworkCallbacks(networkAgent, ConnectivityManager.CALLBACK_IP_CHANGED);
             }
         }
@@ -5718,7 +5718,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         final NetworkCapabilities prevNc;
         synchronized (nai) {
             prevNc = nai.networkCapabilities;
-            nai.networkCapabilities = newNc;
+            nai.setNetworkCapabilities(newNc);
         }
 
         updateUids(nai, prevNc, newNc);
@@ -5733,11 +5733,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
             // If the requestable capabilities have changed or the score changed, we can't have been
             // called by rematchNetworkAndRequests, so it's safe to start a rematch.
             rematchAllNetworksAndRequests(nai, oldScore);
-            try {
-                nai.networkMonitor().notifyNetworkCapabilitiesChanged();
-            } catch (RemoteException e) {
-                e.rethrowFromSystemServer();
-            }
             notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_CAP_CHANGED);
         }
 
@@ -5996,11 +5991,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
 
         if (capabilitiesChanged) {
-            try {
-                nai.networkMonitor().notifyNetworkCapabilitiesChanged();
-            } catch (RemoteException e) {
-                e.rethrowFromSystemServer();
-            }
             notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_CAP_CHANGED);
         }
 
@@ -6409,7 +6399,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 if (networkAgent.networkMisc.acceptPartialConnectivity) {
                     networkAgent.networkMonitor().setAcceptPartialConnectivity();
                 }
-                networkAgent.networkMonitor().notifyNetworkConnected();
+                networkAgent.networkMonitor().notifyNetworkConnected(
+                        networkAgent.linkProperties, networkAgent.networkCapabilities);
             } catch (RemoteException e) {
                 e.rethrowFromSystemServer();
             }

services/core/java/com/android/server/connectivity/NetworkAgentInfo.java

@@ -30,6 +30,7 @@ import android.net.NetworkState;
 import android.os.Handler;
 import android.os.INetworkManagementService;
 import android.os.Messenger;
+import android.os.RemoteException;
 import android.os.SystemClock;
 import android.util.Log;
 import android.util.SparseArray;
@@ -121,7 +122,8 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
     // This Network object is always valid.
     public final Network network;
     public LinkProperties linkProperties;
-    // This should only be modified via ConnectivityService.updateCapabilities().
+    // This should only be modified by ConnectivityService, via setNetworkCapabilities().
+    // TODO: make this private with a getter.
     public NetworkCapabilities networkCapabilities;
     public final NetworkMisc networkMisc;
     // Indicates if netd has been told to create this Network. From this point on the appropriate
@@ -279,6 +281,25 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
         mNetworkMonitor = networkMonitor;
     }
 
+    /**
+     * Set the NetworkCapabilities on this NetworkAgentInfo. Also attempts to notify NetworkMonitor
+     * of the new capabilities, if NetworkMonitor has been created.
+     *
+     * <p>If {@link NetworkMonitor#notifyNetworkCapabilitiesChanged(NetworkCapabilities)} fails,
+     * the exception is logged but not reported to callers.
+     */
+    public void setNetworkCapabilities(NetworkCapabilities nc) {
+        networkCapabilities = nc;
+        final INetworkMonitor nm = mNetworkMonitor;
+        if (nm != null) {
+            try {
+                nm.notifyNetworkCapabilitiesChanged(nc);
+            } catch (RemoteException e) {
+                Log.e(TAG, "Error notifying NetworkMonitor of updated NetworkCapabilities", e);
+            }
+        }
+    }
+
     public ConnectivityService connService() {
         return mConnService;
     }

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -499,7 +499,7 @@ public class ConnectivityServiceTest {
             };
 
             try {
-                doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected();
+                doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected(any(), any());
                 doAnswer(validateAnswer).when(mNetworkMonitor).forceReevaluation(anyInt());
             } catch (RemoteException e) {
                 fail(e.getMessage());