DO NOT MERGE: ConnectivityManager: a simpler CallbackHandler

This patch simplifies CallbackHandler in the following way:
  - CallbackHandler directly uses the static references to
    sNetworkCallback and sCallbackRefCount. This allows to remove
    instance fields in CallbackHandler.
  - CallbackHandler does not have a reference to ConnectivityManager
    anymore
  - CallbackHandler.getObject() is now generic in a type-safe way.

Test: ConnectivityServiceTest passes
Bug: 28537383
Bug: 32130437

(cherry picked from commit 0b42baf68a)

Change-Id: I1b5fe2a361b5f623a8310ae698497c83d72f3034

core/java/android/net/ConnectivityManager.java

@@ -2690,24 +2690,17 @@ public class ConnectivityManager {
     }
 
     private class CallbackHandler extends Handler {
-        private final HashMap<NetworkRequest, NetworkCallback>mCallbackMap;
-        private final AtomicInteger mRefCount;
         private static final String TAG = "ConnectivityManager.CallbackHandler";
-        private final ConnectivityManager mCm;
         private static final boolean DBG = false;
 
-        CallbackHandler(Looper looper, HashMap<NetworkRequest, NetworkCallback>callbackMap,
+        CallbackHandler(Looper looper) {
-                AtomicInteger refCount, ConnectivityManager cm) {
             super(looper);
-            mCallbackMap = callbackMap;
-            mRefCount = refCount;
-            mCm = cm;
         }
 
         @Override
         public void handleMessage(Message message) {
-            NetworkRequest request = (NetworkRequest) getObject(message, NetworkRequest.class);
+            NetworkRequest request = getObject(message, NetworkRequest.class);
-            Network network = (Network) getObject(message, Network.class);
+            Network network = getObject(message, Network.class);
             if (DBG) {
                 Log.d(TAG, whatToString(message.what) + " for network " + network);
             }
@@ -2750,9 +2743,7 @@ public class ConnectivityManager {
                 case CALLBACK_CAP_CHANGED: {
                     NetworkCallback callback = getCallback(request, "CAP_CHANGED");
                     if (callback != null) {
-                        NetworkCapabilities cap = (NetworkCapabilities)getObject(message,
+                        NetworkCapabilities cap = getObject(message, NetworkCapabilities.class);
-                                NetworkCapabilities.class);
-
                         callback.onCapabilitiesChanged(network, cap);
                     }
                     break;
@@ -2760,9 +2751,7 @@ public class ConnectivityManager {
                 case CALLBACK_IP_CHANGED: {
                     NetworkCallback callback = getCallback(request, "IP_CHANGED");
                     if (callback != null) {
-                        LinkProperties lp = (LinkProperties)getObject(message,
+                        LinkProperties lp = getObject(message, LinkProperties.class);
-                                LinkProperties.class);
-
                         callback.onLinkPropertiesChanged(network, lp);
                     }
                     break;
@@ -2783,12 +2772,12 @@ public class ConnectivityManager {
                 }
                 case CALLBACK_RELEASED: {
                     NetworkCallback callback = null;
-                    synchronized(mCallbackMap) {
+                    synchronized(sCallbacks) {
-                        callback = mCallbackMap.remove(request);
+                        callback = sCallbacks.remove(request);
                     }
                     if (callback != null) {
-                        synchronized(mRefCount) {
+                        synchronized(sCallbackRefCount) {
-                            if (mRefCount.decrementAndGet() == 0) {
+                            if (sCallbackRefCount.decrementAndGet() == 0) {
                                 getLooper().quit();
                             }
                         }
@@ -2809,14 +2798,14 @@ public class ConnectivityManager {
             }
         }
 
-        private Object getObject(Message msg, Class c) {
+        private <T> T getObject(Message msg, Class<T> c) {
-            return msg.getData().getParcelable(c.getSimpleName());
+            return (T) msg.getData().getParcelable(c.getSimpleName());
         }
 
         private NetworkCallback getCallback(NetworkRequest req, String name) {
             NetworkCallback callback;
-            synchronized(mCallbackMap) {
+            synchronized(sCallbacks) {
-                callback = mCallbackMap.get(req);
+                callback = sCallbacks.get(req);
             }
             if (callback == null) {
                 Log.e(TAG, "callback not found for " + name + " message");
@@ -2831,8 +2820,7 @@ public class ConnectivityManager {
                 // TODO: switch this to ConnectivityThread
                 HandlerThread callbackThread = new HandlerThread("ConnectivityManager");
                 callbackThread.start();
-                sCallbackHandler = new CallbackHandler(callbackThread.getLooper(),
+                sCallbackHandler = new CallbackHandler(callbackThread.getLooper());
-                        sNetworkCallback, sCallbackRefCount, this);
             }
         }
     }
@@ -2846,8 +2834,7 @@ public class ConnectivityManager {
         }
     }
 
-    static final HashMap<NetworkRequest, NetworkCallback> sNetworkCallback =
+    static final HashMap<NetworkRequest, NetworkCallback> sCallbacks = new HashMap<>();
-            new HashMap<NetworkRequest, NetworkCallback>();
     static final AtomicInteger sCallbackRefCount = new AtomicInteger(0);
     static CallbackHandler sCallbackHandler = null;
 
@@ -2863,25 +2850,32 @@ public class ConnectivityManager {
         if (need == null && action != REQUEST) {
             throw new IllegalArgumentException("null NetworkCapabilities");
         }
+        // TODO: throw an exception if networkCallback.networkRequest is not null.
+        // http://b/20701525
+        final NetworkRequest request;
         try {
             incCallbackHandlerRefCount();
-            synchronized(sNetworkCallback) {
+            synchronized(sCallbacks) {
+                Messenger messenger = new Messenger(sCallbackHandler);
+                Binder binder = new Binder();
                 if (action == LISTEN) {
-                    networkCallback.networkRequest = mService.listenForNetwork(need,
+                    request = mService.listenForNetwork(need, messenger, binder);
-                            new Messenger(sCallbackHandler), new Binder());
                 } else {
-                    networkCallback.networkRequest = mService.requestNetwork(need,
+                    request = mService.requestNetwork(
-                            new Messenger(sCallbackHandler), timeoutMs, new Binder(), legacyType);
+                            need, messenger, timeoutMs, binder, legacyType);
                 }
-                if (networkCallback.networkRequest != null) {
+                if (request != null) {
-                    sNetworkCallback.put(networkCallback.networkRequest, networkCallback);
+                    sCallbacks.put(request, networkCallback);
                 }
+                networkCallback.networkRequest = request;
             }
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         }
-        if (networkCallback.networkRequest == null) decCallbackHandlerRefCount();
+        if (request == null) {
-        return networkCallback.networkRequest;
+            decCallbackHandlerRefCount();
+        }
+        return request;
     }
 
     /**