From 0b42baf68a7e52e1924277d8e2361bd93975a0ba Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 6 Jul 2016 22:53:17 +0900 Subject: [PATCH] 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 Change-Id: I5004da5b91498e6ff7f8b05057a9e24b975bb56e --- .../java/android/net/ConnectivityManager.java | 68 +++++++++---------- 1 file changed, 31 insertions(+), 37 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 9e5aaf5977..db341246c0 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -2709,24 +2709,17 @@ public class ConnectivityManager { } private class CallbackHandler extends Handler { - private final HashMapmCallbackMap; - private final AtomicInteger mRefCount; private static final String TAG = "ConnectivityManager.CallbackHandler"; - private final ConnectivityManager mCm; private static final boolean DBG = false; - CallbackHandler(Looper looper, HashMapcallbackMap, - AtomicInteger refCount, ConnectivityManager cm) { + CallbackHandler(Looper looper) { super(looper); - mCallbackMap = callbackMap; - mRefCount = refCount; - mCm = cm; } @Override public void handleMessage(Message message) { - NetworkRequest request = (NetworkRequest) getObject(message, NetworkRequest.class); - Network network = (Network) getObject(message, Network.class); + NetworkRequest request = getObject(message, NetworkRequest.class); + Network network = getObject(message, Network.class); if (DBG) { Log.d(TAG, whatToString(message.what) + " for network " + network); } @@ -2769,9 +2762,7 @@ public class ConnectivityManager { case CALLBACK_CAP_CHANGED: { NetworkCallback callback = getCallback(request, "CAP_CHANGED"); if (callback != null) { - NetworkCapabilities cap = (NetworkCapabilities)getObject(message, - NetworkCapabilities.class); - + NetworkCapabilities cap = getObject(message, NetworkCapabilities.class); callback.onCapabilitiesChanged(network, cap); } break; @@ -2779,9 +2770,7 @@ public class ConnectivityManager { case CALLBACK_IP_CHANGED: { NetworkCallback callback = getCallback(request, "IP_CHANGED"); if (callback != null) { - LinkProperties lp = (LinkProperties)getObject(message, - LinkProperties.class); - + LinkProperties lp = getObject(message, LinkProperties.class); callback.onLinkPropertiesChanged(network, lp); } break; @@ -2802,12 +2791,12 @@ public class ConnectivityManager { } case CALLBACK_RELEASED: { NetworkCallback callback = null; - synchronized(mCallbackMap) { - callback = mCallbackMap.remove(request); + synchronized(sCallbacks) { + callback = sCallbacks.remove(request); } if (callback != null) { - synchronized(mRefCount) { - if (mRefCount.decrementAndGet() == 0) { + synchronized(sCallbackRefCount) { + if (sCallbackRefCount.decrementAndGet() == 0) { getLooper().quit(); } } @@ -2828,14 +2817,14 @@ public class ConnectivityManager { } } - private Object getObject(Message msg, Class c) { - return msg.getData().getParcelable(c.getSimpleName()); + private T getObject(Message msg, Class c) { + return (T) msg.getData().getParcelable(c.getSimpleName()); } private NetworkCallback getCallback(NetworkRequest req, String name) { NetworkCallback callback; - synchronized(mCallbackMap) { - callback = mCallbackMap.get(req); + synchronized(sCallbacks) { + callback = sCallbacks.get(req); } if (callback == null) { Log.e(TAG, "callback not found for " + name + " message"); @@ -2850,8 +2839,7 @@ public class ConnectivityManager { // TODO: switch this to ConnectivityThread HandlerThread callbackThread = new HandlerThread("ConnectivityManager"); callbackThread.start(); - sCallbackHandler = new CallbackHandler(callbackThread.getLooper(), - sNetworkCallback, sCallbackRefCount, this); + sCallbackHandler = new CallbackHandler(callbackThread.getLooper()); } } } @@ -2865,8 +2853,7 @@ public class ConnectivityManager { } } - static final HashMap sNetworkCallback = - new HashMap(); + static final HashMap sCallbacks = new HashMap<>(); static final AtomicInteger sCallbackRefCount = new AtomicInteger(0); static CallbackHandler sCallbackHandler = null; @@ -2882,25 +2869,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, - new Messenger(sCallbackHandler), new Binder()); + request = mService.listenForNetwork(need, messenger, binder); } else { - networkCallback.networkRequest = mService.requestNetwork(need, - new Messenger(sCallbackHandler), timeoutMs, new Binder(), legacyType); + request = mService.requestNetwork( + need, messenger, timeoutMs, binder, legacyType); } - if (networkCallback.networkRequest != null) { - sNetworkCallback.put(networkCallback.networkRequest, networkCallback); + if (request != null) { + sCallbacks.put(request, networkCallback); } + networkCallback.networkRequest = request; } } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } - if (networkCallback.networkRequest == null) decCallbackHandlerRefCount(); - return networkCallback.networkRequest; + if (request == null) { + decCallbackHandlerRefCount(); + } + return request; } /**