am fd183706: am 22262f31: am 0bc35668: am 017223ac: Merge changes from topic \'packet-keepalive-fixes\' into mnc-dr-dev

* commit 'fd18370675f8794807747a18276dd7385e25f06e':
  Require the new PACKET_KEEPALIVE_OFFLOAD permission.
  Add an error code for generic hardware error.
  Fix bugs and crashes in PacketKeepalive API.
  Add tests for the PacketKeepalive API.
  Add a PACKET_KEEPALIVE_OFFLOAD permission.
  Use a CountDownLatch instead of sleep() in NetworkFactory tests.
  Get rid of shortSleep() in ConnectivityServiceTest.
  Make ConnectivityServiceTest a bit more readable.

core/java/android/net/ConnectivityManager.java

@@ -1243,6 +1243,8 @@ public class ConnectivityManager {
 
         /** The hardware does not support this request. */
         public static final int ERROR_HARDWARE_UNSUPPORTED = -30;
+        /** The hardware returned an error. */
+        public static final int ERROR_HARDWARE_ERROR = -31;
 
         public static final int NATT_PORT = 4500;
 

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

@@ -359,6 +359,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
      */
     private static final int EVENT_REGISTER_NETWORK_LISTENER_WITH_INTENT = 31;
 
+    /** Handler thread used for both of the handlers below. */
+    @VisibleForTesting
+    protected final HandlerThread mHandlerThread;
     /** Handler used for internal events. */
     final private InternalHandler mHandler;
     /** Handler used for incoming {@link NetworkStateTracker} events. */
@@ -614,6 +617,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
     private LegacyTypeTracker mLegacyTypeTracker = new LegacyTypeTracker();
 
+    @VisibleForTesting
+    protected HandlerThread createHandlerThread() {
+        return new HandlerThread("ConnectivityServiceThread");
+    }
+
     public ConnectivityService(Context context, INetworkManagementService netManager,
             INetworkStatsService statsService, INetworkPolicyManager policyManager) {
         if (DBG) log("ConnectivityService starting up");
@@ -627,10 +635,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
         mDefaultMobileDataRequest = createInternetRequestForTransport(
                 NetworkCapabilities.TRANSPORT_CELLULAR);
 
-        HandlerThread handlerThread = new HandlerThread("ConnectivityServiceThread");
-        handlerThread.start();
-        mHandler = new InternalHandler(handlerThread.getLooper());
-        mTrackerHandler = new NetworkStateTrackerHandler(handlerThread.getLooper());
+        mHandlerThread = createHandlerThread();
+        mHandlerThread.start();
+        mHandler = new InternalHandler(mHandlerThread.getLooper());
+        mTrackerHandler = new NetworkStateTrackerHandler(mHandlerThread.getLooper());
 
         // setup our unique device name
         if (TextUtils.isEmpty(SystemProperties.get("net.hostname"))) {
@@ -1458,7 +1466,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
 
     private void enforceKeepalivePermission() {
-        mContext.enforceCallingPermission(KeepaliveTracker.PERMISSION, "ConnectivityService");
+        mContext.enforceCallingOrSelfPermission(KeepaliveTracker.PERMISSION, "ConnectivityService");
     }
 
     public void sendConnectedBroadcast(NetworkInfo info) {

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

@@ -71,6 +71,7 @@ public class KeepalivePacketData {
         // Check we have two IP addresses of the same family.
         if (srcAddress == null || dstAddress == null ||
                 !srcAddress.getClass().getName().equals(dstAddress.getClass().getName())) {
+            throw new InvalidPacketException(ERROR_INVALID_IP_ADDRESS);
         }
 
         // Set the protocol.
@@ -102,7 +103,7 @@ public class KeepalivePacketData {
             InetAddress srcAddress, int srcPort,
             InetAddress dstAddress, int dstPort) throws InvalidPacketException {
 
-        if (!(srcAddress instanceof Inet4Address)) {
+        if (!(srcAddress instanceof Inet4Address) || !(dstAddress instanceof Inet4Address)) {
             throw new InvalidPacketException(ERROR_INVALID_IP_ADDRESS);
         }
 

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

@@ -62,8 +62,7 @@ public class KeepaliveTracker {
     private static final String TAG = "KeepaliveTracker";
     private static final boolean DBG = true;
 
-    // TODO: Change this to a system-only permission.
-    public static final String PERMISSION = android.Manifest.permission.CHANGE_NETWORK_STATE;
+    public static final String PERMISSION = android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD;
 
     /** Keeps track of keepalive requests. */
     private final HashMap <NetworkAgentInfo, HashMap<Integer, KeepaliveInfo>> mKeepalives =
@@ -208,6 +207,8 @@ public class KeepaliveTracker {
                 Log.d(TAG, "Stopping keepalive " + mSlot + " on " + mNai.name());
                 mNai.asyncChannel.sendMessage(CMD_STOP_PACKET_KEEPALIVE, mSlot);
             }
+            // TODO: at the moment we unconditionally return failure here. In cases where the
+            // NetworkAgent is alive, should we ask it to reply, so it can return failure?
             notifyMessenger(mSlot, reason);
             unlinkDeathRecipient();
         }
@@ -233,17 +234,14 @@ public class KeepaliveTracker {
             mKeepalives.put(nai, networkKeepalives);
         }
 
-        // Find the lowest-numbered free slot.
+        // Find the lowest-numbered free slot. Slot numbers start from 1, because that's what two
+        // separate chipset implementations independently came up with.
         int slot;
-        for (slot = 0; slot < networkKeepalives.size(); slot++) {
+        for (slot = 1; slot <= networkKeepalives.size(); slot++) {
             if (networkKeepalives.get(slot) == null) {
                 return slot;
             }
         }
-        // No free slot, pick one at the end.
-
-        // HACK for broadcom hardware that does not support slot 0!
-        if (slot == 0) slot = 1;
         return slot;
     }
 
@@ -267,14 +265,15 @@ public class KeepaliveTracker {
     }
 
     public void handleStopKeepalive(NetworkAgentInfo nai, int slot, int reason) {
+        String networkName = (nai == null) ? "(null)" : nai.name();
         HashMap <Integer, KeepaliveInfo> networkKeepalives = mKeepalives.get(nai);
         if (networkKeepalives == null) {
-            Log.e(TAG, "Attempt to stop keepalive on nonexistent network " + nai.name());
+            Log.e(TAG, "Attempt to stop keepalive on nonexistent network " + networkName);
             return;
         }
         KeepaliveInfo ki = networkKeepalives.get(slot);
         if (ki == null) {
-            Log.e(TAG, "Attempt to stop nonexistent keepalive " + slot + " on " + nai.name());
+            Log.e(TAG, "Attempt to stop nonexistent keepalive " + slot + " on " + networkName);
             return;
         }
         ki.stop(reason);
@@ -332,6 +331,11 @@ public class KeepaliveTracker {
 
     public void startNattKeepalive(NetworkAgentInfo nai, int intervalSeconds, Messenger messenger,
             IBinder binder, String srcAddrString, int srcPort, String dstAddrString, int dstPort) {
+        if (nai == null) {
+            notifyMessenger(messenger, NO_KEEPALIVE, ERROR_INVALID_NETWORK);
+            return;
+        }
+
         InetAddress srcAddress, dstAddress;
         try {
             srcAddress = NetworkUtils.numericToInetAddress(srcAddrString);