OSDN Git Service

Switch to listening for all network changes.
authorErik Kline <ek@google.com>
Tue, 24 Jan 2017 15:53:04 +0000 (00:53 +0900)
committerErik Kline <ek@google.com>
Thu, 26 Jan 2017 11:50:51 +0000 (20:50 +0900)
This is for use while preferred upstreams are expressed as legacy types.

Test: as follows
    - built (bullhead)
    - flashed
    - booted
    - runtest frameworks-net passes
    - USB tethering to WiFi and DUN works
Bug: 32163131
Change-Id: I76e7b6c95eb1b54e926096b2791163617bb0a818

services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java
tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java

index 23481dc..08a3332 100644 (file)
@@ -40,7 +40,9 @@ import java.util.HashMap;
  * pertaining to the current and any potential upstream network.
  *
  * Calling #start() registers two callbacks: one to track the system default
- * network and a second to specifically observe TYPE_MOBILE_DUN networks.
+ * network and a second to observe all networks.  The latter is necessary
+ * while the expression of preferred upstreams remains a list of legacy
+ * connectivity types.  In future, this can be revisited.
  *
  * The methods and data members of this class are only to be accessed and
  * modified from the tethering master state machine thread. Any other
@@ -48,6 +50,10 @@ import java.util.HashMap;
  *
  * TODO: Move upstream selection logic here.
  *
+ * All callback methods are run on the same thread as the specified target
+ * state machine.  This class does not require locking when accessed from this
+ * thread.  Access from other threads is not advised.
+ *
  * @hide
  */
 public class UpstreamNetworkMonitor {
@@ -60,15 +66,20 @@ public class UpstreamNetworkMonitor {
     public static final int EVENT_ON_LINKPROPERTIES = 3;
     public static final int EVENT_ON_LOST           = 4;
 
+    private static final int LISTEN_ALL = 1;
+    private static final int TRACK_DEFAULT = 2;
+    private static final int MOBILE_REQUEST = 3;
+
     private final Context mContext;
     private final StateMachine mTarget;
     private final int mWhat;
     private final HashMap<Network, NetworkState> mNetworkMap = new HashMap<>();
     private ConnectivityManager mCM;
+    private NetworkCallback mListenAllCallback;
     private NetworkCallback mDefaultNetworkCallback;
-    private NetworkCallback mDunTetheringCallback;
     private NetworkCallback mMobileNetworkCallback;
     private boolean mDunRequired;
+    private Network mCurrentDefault;
 
     public UpstreamNetworkMonitor(Context ctx, StateMachine tgt, int what) {
         mContext = ctx;
@@ -85,16 +96,13 @@ public class UpstreamNetworkMonitor {
     public void start() {
         stop();
 
-        mDefaultNetworkCallback = new UpstreamNetworkCallback();
-        cm().registerDefaultNetworkCallback(mDefaultNetworkCallback);
+        final NetworkRequest listenAllRequest = new NetworkRequest.Builder()
+                .clearCapabilities().build();
+        mListenAllCallback = new UpstreamNetworkCallback(LISTEN_ALL);
+        cm().registerNetworkCallback(listenAllRequest, mListenAllCallback);
 
-        final NetworkRequest dunTetheringRequest = new NetworkRequest.Builder()
-                .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
-                .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED)
-                .addCapability(NetworkCapabilities.NET_CAPABILITY_DUN)
-                .build();
-        mDunTetheringCallback = new UpstreamNetworkCallback();
-        cm().registerNetworkCallback(dunTetheringRequest, mDunTetheringCallback);
+        mDefaultNetworkCallback = new UpstreamNetworkCallback(TRACK_DEFAULT);
+        cm().registerDefaultNetworkCallback(mDefaultNetworkCallback);
     }
 
     public void stop() {
@@ -103,8 +111,8 @@ public class UpstreamNetworkMonitor {
         releaseCallback(mDefaultNetworkCallback);
         mDefaultNetworkCallback = null;
 
-        releaseCallback(mDunTetheringCallback);
-        mDunTetheringCallback = null;
+        releaseCallback(mListenAllCallback);
+        mListenAllCallback = null;
 
         mNetworkMap.clear();
     }
@@ -140,7 +148,7 @@ public class UpstreamNetworkMonitor {
 
         // The existing default network and DUN callbacks will be notified.
         // Therefore, to avoid duplicate notifications, we only register a no-op.
-        mMobileNetworkCallback = new NetworkCallback();
+        mMobileNetworkCallback = new UpstreamNetworkCallback(MOBILE_REQUEST);
 
         // TODO: Change the timeout from 0 (no onUnavailable callback) to some
         // moderate callback timeout. This might be useful for updating some UI.
@@ -165,86 +173,117 @@ public class UpstreamNetworkMonitor {
         return (network != null) ? mNetworkMap.get(network) : null;
     }
 
-    private void handleAvailable(Network network) {
-        if (VDBG) {
-            Log.d(TAG, "EVENT_ON_AVAILABLE for " + network);
-        }
+    private void handleAvailable(int callbackType, Network network) {
+        if (VDBG) Log.d(TAG, "EVENT_ON_AVAILABLE for " + network);
+
         if (!mNetworkMap.containsKey(network)) {
             mNetworkMap.put(network,
                     new NetworkState(null, null, null, network, null, null));
         }
 
-        final ConnectivityManager cm = cm();
-
-        if (mDefaultNetworkCallback != null) {
+        // Always request whatever extra information we can, in case this
+        // was already up when start() was called, in which case we would
+        // not have been notified of any information that had not changed.
+        final NetworkCallback cb =
+                (callbackType == TRACK_DEFAULT) ? mDefaultNetworkCallback :
+                (callbackType == MOBILE_REQUEST) ? mMobileNetworkCallback : null;
+        if (cb != null) {
+            final ConnectivityManager cm = cm();
             cm.requestNetworkCapabilities(mDefaultNetworkCallback);
             cm.requestLinkProperties(mDefaultNetworkCallback);
         }
 
-        // Requesting updates for mDunTetheringCallback is not
-        // necessary. Because it's a listen, it will already have
-        // heard all NetworkCapabilities and LinkProperties updates
-        // since UpstreamNetworkMonitor was started. Because we
-        // start UpstreamNetworkMonitor before chooseUpstreamType()
-        // is ever invoked (it can register a DUN request) this is
-        // mostly safe. However, if a DUN network is already up for
-        // some reason (unlikely, because DUN is restricted and,
-        // unless the DUN network is shared with another APN, only
-        // the system can request it and this is the only part of
-        // the system that requests it) we won't know its
-        // LinkProperties or NetworkCapabilities.
+        if (callbackType == TRACK_DEFAULT) {
+            mCurrentDefault = network;
+        }
 
+        // Requesting updates for mListenAllCallback is not currently possible
+        // because it's a "listen". Two possible solutions to getting updates
+        // about networks without waiting for a change (which might never come)
+        // are:
+        //
+        //     [1] extend request{NetworkCapabilities,LinkProperties}() to
+        //         take a Network argument and have ConnectivityService do
+        //         what's required (if the network satisfies the request)
+        //
+        //     [2] explicitly file a NetworkRequest for each connectivity type
+        //         listed as a preferred upstream and wait for these callbacks
+        //         to be notified (requires tracking many more callbacks).
+        //
+        // Until this is addressed, networks that exist prior to the "listen"
+        // registration and which do not subsequently change will not cause
+        // us to learn their NetworkCapabilities nor their LinkProperties.
+
+        // TODO: If sufficient information is available to select a more
+        // preferable upstream, do so now and notify the target.
         notifyTarget(EVENT_ON_AVAILABLE, network);
     }
 
     private void handleNetCap(Network network, NetworkCapabilities newNc) {
-        if (!mNetworkMap.containsKey(network)) {
-            // Ignore updates for networks for which we have not yet
-            // received onAvailable() - which should never happen -
-            // or for which we have already received onLost().
+        final NetworkState prev = mNetworkMap.get(network);
+        if (prev == null || newNc.equals(prev.networkCapabilities)) {
+            // Ignore notifications about networks for which we have not yet
+            // received onAvailable() (should never happen) and any duplicate
+            // notifications (e.g. matching more than one of our callbacks).
             return;
         }
+
         if (VDBG) {
             Log.d(TAG, String.format("EVENT_ON_CAPABILITIES for %s: %s",
                     network, newNc));
         }
 
-        final NetworkState prev = mNetworkMap.get(network);
-        mNetworkMap.put(network,
-                new NetworkState(null, prev.linkProperties, newNc,
-                                 network, null, null));
+        mNetworkMap.put(network, new NetworkState(
+                null, prev.linkProperties, newNc, network, null, null));
+        // TODO: If sufficient information is available to select a more
+        // preferable upstream, do so now and notify the target.
         notifyTarget(EVENT_ON_CAPABILITIES, network);
     }
 
     private void handleLinkProp(Network network, LinkProperties newLp) {
-        if (!mNetworkMap.containsKey(network)) {
-            // Ignore updates for networks for which we have not yet
-            // received onAvailable() - which should never happen -
-            // or for which we have already received onLost().
+        final NetworkState prev = mNetworkMap.get(network);
+        if (prev == null || newLp.equals(prev.linkProperties)) {
+            // Ignore notifications about networks for which we have not yet
+            // received onAvailable() (should never happen) and any duplicate
+            // notifications (e.g. matching more than one of our callbacks).
             return;
         }
+
         if (VDBG) {
             Log.d(TAG, String.format("EVENT_ON_LINKPROPERTIES for %s: %s",
                     network, newLp));
         }
 
-        final NetworkState prev = mNetworkMap.get(network);
-        mNetworkMap.put(network,
-                new NetworkState(null, newLp, prev.networkCapabilities,
-                                 network, null, null));
+        mNetworkMap.put(network, new NetworkState(
+                null, newLp, prev.networkCapabilities, network, null, null));
+        // TODO: If sufficient information is available to select a more
+        // preferable upstream, do so now and notify the target.
         notifyTarget(EVENT_ON_LINKPROPERTIES, network);
     }
 
-    private void handleLost(Network network) {
-        if (!mNetworkMap.containsKey(network)) {
-            // Ignore updates for networks for which we have not yet
-            // received onAvailable() - which should never happen -
-            // or for which we have already received onLost().
+    private void handleLost(int callbackType, Network network) {
+        if (callbackType == TRACK_DEFAULT) {
+            mCurrentDefault = null;
+            // Receiving onLost() for a default network does not necessarily
+            // mean the network is gone.  We wait for a separate notification
+            // on either the LISTEN_ALL or MOBILE_REQUEST callbacks before
+            // clearing all state.
             return;
         }
-        if (VDBG) {
-            Log.d(TAG, "EVENT_ON_LOST for " + network);
+
+        if (!mNetworkMap.containsKey(network)) {
+            // Ignore loss of networks about which we had not previously
+            // learned any information or for which we have already processed
+            // an onLost() notification.
+            return;
         }
+
+        if (VDBG) Log.d(TAG, "EVENT_ON_LOST for " + network);
+
+        // TODO: If sufficient information is available to select a more
+        // preferable upstream, do so now and notify the target.  Likewise,
+        // if the current upstream network is gone, notify the target of the
+        // fact that we now have no upstream at all.
         notifyTarget(EVENT_ON_LOST, mNetworkMap.remove(network));
     }
 
@@ -261,9 +300,15 @@ public class UpstreamNetworkMonitor {
      * tethering master state machine thread for subsequent processing.
      */
     private class UpstreamNetworkCallback extends NetworkCallback {
+        private final int mCallbackType;
+
+        UpstreamNetworkCallback(int callbackType) {
+            mCallbackType = callbackType;
+        }
+
         @Override
         public void onAvailable(Network network) {
-            mTarget.getHandler().post(() -> handleAvailable(network));
+            mTarget.getHandler().post(() -> handleAvailable(mCallbackType, network));
         }
 
         @Override
@@ -278,7 +323,7 @@ public class UpstreamNetworkMonitor {
 
         @Override
         public void onLost(Network network) {
-            mTarget.getHandler().post(() -> handleLost(network));
+            mTarget.getHandler().post(() -> handleLost(mCallbackType, network));
         }
     }
 
index 1e67769..b8c739b 100644 (file)
@@ -91,12 +91,12 @@ public class UpstreamNetworkMonitorTest {
     }
 
     @Test
-    public void testListensForDunNetworks() throws Exception {
+    public void testListensForAllNetworks() throws Exception {
         assertTrue(mCM.listening.isEmpty());
 
         mUNM.start();
         assertFalse(mCM.listening.isEmpty());
-        assertTrue(mCM.isListeningForDun());
+        assertTrue(mCM.isListeningForAll());
 
         mUNM.stop();
         assertTrue(mCM.hasNoCallbacks());
@@ -197,9 +197,12 @@ public class UpstreamNetworkMonitorTest {
                    legacyTypeMap.isEmpty();
         }
 
-        boolean isListeningForDun() {
+        boolean isListeningForAll() {
+            final NetworkCapabilities empty = new NetworkCapabilities();
+            empty.clearAll();
+
             for (NetworkRequest req : listening.values()) {
-                if (req.networkCapabilities.hasCapability(NET_CAPABILITY_DUN)) {
+                if (req.networkCapabilities.equalRequestableCapabilities(empty)) {
                     return true;
                 }
             }