Track default upstream when system is ready

Start tracking default upstream from boot.This is useful for
entitlement refine in following change. EntitlementManager can
decide if it needs to process entitlement provisioning before
tethering started.

Test: -atest FrameworksNetTests
      -build, flash, booted
      -manually turnoff/on tethering with different upstream
bug: 111490073

Change-Id: I8fdbd64c52f26b5363693bb5bd8050930e8ea961

5 files changed, 97 insertions, 84 deletions

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index eda9fe15fe36..89194e43bf95 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -1968,6 +1968,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     void systemReady() {
         mProxyTracker.loadGlobalProxy();
         registerNetdEventCallback();
+        mTethering.systemReady();
 
         synchronized (this) {
             mSystemReady = true;
diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java
index d75601be23e3..9dfdddbea18a 100644
--- a/services/core/java/com/android/server/connectivity/Tethering.java
+++ b/services/core/java/com/android/server/connectivity/Tethering.java
@@ -1382,7 +1382,7 @@ public class Tethering extends BaseNetworkObserver {
                     return;
                 }
 
-                mUpstreamNetworkMonitor.start(mDeps.getDefaultNetworkRequest());
+                mUpstreamNetworkMonitor.startObserveAllNetworks();
 
                 // TODO: De-duplicate with updateUpstreamWanted() below.
                 if (upstreamWanted()) {
@@ -1658,6 +1658,10 @@ public class Tethering extends BaseNetworkObserver {
         }
     }
 
+    public void systemReady() {
+        mUpstreamNetworkMonitor.startTrackDefaultNetwork(mDeps.getDefaultNetworkRequest());
+    }
+
     @Override
     public void dump(FileDescriptor fd, PrintWriter writer, String[] args) {
         // Binder.java closes the resource for us.
diff --git a/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java b/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java
index 3e5d5aa6ca54..3ac311b3e13a 100644
--- a/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java
+++ b/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java
@@ -55,10 +55,13 @@ import java.util.Set;
  * A class to centralize all the network and link properties information
  * 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 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 owner of UNM gets it to register network callbacks by calling the
+ * following methods :
+ * Calling #startTrackDefaultNetwork() to track the system default network.
+ * Calling #startObserveAllNetworks() to observe all networks. Listening all
+ * networks is necessary while the expression of preferred upstreams remains
+ * a list of legacy connectivity types.  In future, this can be revisited.
+ * Calling #registerMobileNetworkRequest() to bring up mobile DUN/HIPRI network.
  *
  * The methods and data members of this class are only to be accessed and
  * modified from the tethering master state machine thread. Any other
@@ -119,33 +122,31 @@ public class UpstreamNetworkMonitor {
         mCM = cm;
     }
 
-    public void start(NetworkRequest defaultNetworkRequest) {
+    public void startTrackDefaultNetwork(NetworkRequest defaultNetworkRequest) {
+        // This is not really a "request", just a way of tracking the system default network.
+        // It's guaranteed not to actually bring up any networks because it's the same request
+        // as the ConnectivityService default request, and thus shares fate with it. We can't
+        // use registerDefaultNetworkCallback because it will not track the system default
+        // network if there is a VPN that applies to our UID.
+        if (mDefaultNetworkCallback == null) {
+            final NetworkRequest trackDefaultRequest = new NetworkRequest(defaultNetworkRequest);
+            mDefaultNetworkCallback = new UpstreamNetworkCallback(CALLBACK_DEFAULT_INTERNET);
+            cm().requestNetwork(trackDefaultRequest, mDefaultNetworkCallback, mHandler);
+        }
+    }
+
+    public void startObserveAllNetworks() {
         stop();
 
         final NetworkRequest listenAllRequest = new NetworkRequest.Builder()
                 .clearCapabilities().build();
         mListenAllCallback = new UpstreamNetworkCallback(CALLBACK_LISTEN_ALL);
         cm().registerNetworkCallback(listenAllRequest, mListenAllCallback, mHandler);
-
-        if (defaultNetworkRequest != null) {
-            // This is not really a "request", just a way of tracking the system default network.
-            // It's guaranteed not to actually bring up any networks because it's the same request
-            // as the ConnectivityService default request, and thus shares fate with it. We can't
-            // use registerDefaultNetworkCallback because it will not track the system default
-            // network if there is a VPN that applies to our UID.
-            final NetworkRequest trackDefaultRequest = new NetworkRequest(defaultNetworkRequest);
-            mDefaultNetworkCallback = new UpstreamNetworkCallback(CALLBACK_DEFAULT_INTERNET);
-            cm().requestNetwork(trackDefaultRequest, mDefaultNetworkCallback, mHandler);
-        }
     }
 
     public void stop() {
         releaseMobileNetworkRequest();
 
-        releaseCallback(mDefaultNetworkCallback);
-        mDefaultNetworkCallback = null;
-        mDefaultInternetNetwork = null;
-
         releaseCallback(mListenAllCallback);
         mListenAllCallback = null;
 
@@ -264,9 +265,7 @@ public class UpstreamNetworkMonitor {
         mNetworkMap.put(network, new NetworkState(null, null, null, network, null, null));
     }
 
-    private void handleNetCap(int callbackType, Network network, NetworkCapabilities newNc) {
-        if (callbackType == CALLBACK_DEFAULT_INTERNET) mDefaultInternetNetwork = network;
-
+    private void handleNetCap(Network network, NetworkCapabilities newNc) {
         final NetworkState prev = mNetworkMap.get(network);
         if (prev == null || newNc.equals(prev.networkCapabilities)) {
             // Ignore notifications about networks for which we have not yet
@@ -315,31 +314,25 @@ public class UpstreamNetworkMonitor {
         notifyTarget(EVENT_ON_LINKPROPERTIES, network);
     }
 
-    private void handleSuspended(int callbackType, Network network) {
-        if (callbackType != CALLBACK_LISTEN_ALL) return;
+    private void handleSuspended(Network network) {
         if (!network.equals(mTetheringUpstreamNetwork)) return;
         mLog.log("SUSPENDED current upstream: " + network);
     }
 
-    private void handleResumed(int callbackType, Network network) {
-        if (callbackType != CALLBACK_LISTEN_ALL) return;
+    private void handleResumed(Network network) {
         if (!network.equals(mTetheringUpstreamNetwork)) return;
         mLog.log("RESUMED current upstream: " + network);
     }
 
-    private void handleLost(int callbackType, Network network) {
-        if (network.equals(mDefaultInternetNetwork)) {
-            mDefaultInternetNetwork = null;
-            // There are few TODOs within ConnectivityService's rematching code
-            // pertaining to spurious onLost() notifications.
-            //
-            // TODO: simplify this, probably if favor of code that:
-            //     - selects a new upstream if mTetheringUpstreamNetwork has
-            //       been lost (by any callback)
-            //     - deletes the entry from the map only when the LISTEN_ALL
-            //       callback gets  notified.
-            if (callbackType == CALLBACK_DEFAULT_INTERNET) return;
-        }
+    private void handleLost(Network network) {
+        // There are few TODOs within ConnectivityService's rematching code
+        // pertaining to spurious onLost() notifications.
+        //
+        // TODO: simplify this, probably if favor of code that:
+        //     - selects a new upstream if mTetheringUpstreamNetwork has
+        //       been lost (by any callback)
+        //     - deletes the entry from the map only when the LISTEN_ALL
+        //       callback gets notified.
 
         if (!mNetworkMap.containsKey(network)) {
             // Ignore loss of networks about which we had not previously
@@ -393,11 +386,17 @@ public class UpstreamNetworkMonitor {
 
         @Override
         public void onCapabilitiesChanged(Network network, NetworkCapabilities newNc) {
-            handleNetCap(mCallbackType, network, newNc);
+            if (mCallbackType == CALLBACK_DEFAULT_INTERNET) {
+                mDefaultInternetNetwork = network;
+                return;
+            }
+            handleNetCap(network, newNc);
         }
 
         @Override
         public void onLinkPropertiesChanged(Network network, LinkProperties newLp) {
+            if (mCallbackType == CALLBACK_DEFAULT_INTERNET) return;
+
             handleLinkProp(network, newLp);
             // Any non-LISTEN_ALL callback will necessarily concern a network that will
             // also match the LISTEN_ALL callback by construction of the LISTEN_ALL callback.
@@ -409,17 +408,25 @@ public class UpstreamNetworkMonitor {
 
         @Override
         public void onNetworkSuspended(Network network) {
-            handleSuspended(mCallbackType, network);
+            if (mCallbackType == CALLBACK_LISTEN_ALL) {
+                handleSuspended(network);
+            }
         }
 
         @Override
         public void onNetworkResumed(Network network) {
-            handleResumed(mCallbackType, network);
+            if (mCallbackType == CALLBACK_LISTEN_ALL) {
+                handleResumed(network);
+            }
         }
 
         @Override
         public void onLost(Network network) {
-            handleLost(mCallbackType, network);
+            if (mCallbackType == CALLBACK_DEFAULT_INTERNET) {
+                mDefaultInternetNetwork = null;
+                return;
+            }
+            handleLost(network);
             // Any non-LISTEN_ALL callback will necessarily concern a network that will
             // also match the LISTEN_ALL callback by construction of the LISTEN_ALL callback.
             // So it's not useful to do this work for non-LISTEN_ALL callbacks.
diff --git a/tests/net/java/com/android/server/connectivity/TetheringTest.java b/tests/net/java/com/android/server/connectivity/TetheringTest.java
index 80818120fa3c..bca9be772704 100644
--- a/tests/net/java/com/android/server/connectivity/TetheringTest.java
+++ b/tests/net/java/com/android/server/connectivity/TetheringTest.java
@@ -71,7 +71,6 @@ import android.net.MacAddress;
 import android.net.Network;
 import android.net.NetworkCapabilities;
 import android.net.NetworkInfo;
-import android.net.NetworkRequest;
 import android.net.NetworkState;
 import android.net.NetworkUtils;
 import android.net.RouteInfo;
@@ -130,10 +129,6 @@ public class TetheringTest {
     private static final String TEST_USB_IFNAME = "test_rndis0";
     private static final String TEST_WLAN_IFNAME = "test_wlan0";
 
-    // Actual contents of the request don't matter for this test. The lack of
-    // any specific TRANSPORT_* is sufficient to identify this request.
-    private static final NetworkRequest mDefaultRequest = new NetworkRequest.Builder().build();
-
     @Mock private ApplicationInfo mApplicationInfo;
     @Mock private Context mContext;
     @Mock private INetworkManagementService mNMService;
@@ -257,11 +252,6 @@ public class TetheringTest {
             isTetheringSupportedCalls++;
             return true;
         }
-
-        @Override
-        public NetworkRequest getDefaultNetworkRequest() {
-            return mDefaultRequest;
-        }
     }
 
     private static NetworkState buildMobileUpstreamState(boolean withIPv4, boolean withIPv6,
@@ -496,7 +486,7 @@ public class TetheringTest {
                 TEST_WLAN_IFNAME, WifiManager.IFACE_IP_MODE_LOCAL_ONLY);
         verifyNoMoreInteractions(mWifiManager);
         verifyTetheringBroadcast(TEST_WLAN_IFNAME, EXTRA_ACTIVE_LOCAL_ONLY);
-        verify(mUpstreamNetworkMonitor, times(1)).start(any(NetworkRequest.class));
+        verify(mUpstreamNetworkMonitor, times(1)).startObserveAllNetworks();
         // TODO: Figure out why this isn't exactly once, for sendTetherStateChangedBroadcast().
         assertTrue(1 <= mTetheringDependencies.isTetheringSupportedCalls);
 
@@ -730,7 +720,7 @@ public class TetheringTest {
                 TEST_WLAN_IFNAME, WifiManager.IFACE_IP_MODE_TETHERED);
         verifyNoMoreInteractions(mWifiManager);
         verifyTetheringBroadcast(TEST_WLAN_IFNAME, EXTRA_ACTIVE_TETHER);
-        verify(mUpstreamNetworkMonitor, times(1)).start(any(NetworkRequest.class));
+        verify(mUpstreamNetworkMonitor, times(1)).startObserveAllNetworks();
         // In tethering mode, in the default configuration, an explicit request
         // for a mobile network is also made.
         verify(mUpstreamNetworkMonitor, times(1)).registerMobileNetworkRequest();
diff --git a/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java b/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java
index a22cbd4c95d8..0afd607d1457 100644
--- a/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java
+++ b/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java
@@ -24,6 +24,7 @@ import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
 import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR;
 import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -53,7 +54,6 @@ import android.net.NetworkCapabilities;
 import android.net.NetworkRequest;
 import android.net.NetworkState;
 import android.net.util.SharedLog;
-
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
 
@@ -65,7 +65,6 @@ import org.junit.Before;
 import org.junit.runner.RunWith;
 import org.junit.Test;
 import org.mockito.Mock;
-import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
 import java.util.ArrayList;
@@ -126,7 +125,7 @@ public class UpstreamNetworkMonitorTest {
     }
 
     @Test
-    public void testDoesNothingBeforeStarted() {
+    public void testDoesNothingBeforeTrackDefaultAndStarted() throws Exception {
         assertTrue(mCM.hasNoCallbacks());
         assertFalse(mUNM.mobileNetworkRequested());
 
@@ -138,37 +137,40 @@ public class UpstreamNetworkMonitorTest {
 
     @Test
     public void testDefaultNetworkIsTracked() throws Exception {
-        assertEquals(0, mCM.trackingDefault.size());
+        assertTrue(mCM.hasNoCallbacks());
+        mUNM.startTrackDefaultNetwork(mDefaultRequest);
 
-        mUNM.start(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
         assertEquals(1, mCM.trackingDefault.size());
 
         mUNM.stop();
-        assertTrue(mCM.hasNoCallbacks());
+        assertTrue(mCM.onlyHasDefaultCallbacks());
     }
 
     @Test
     public void testListensForAllNetworks() throws Exception {
         assertTrue(mCM.listening.isEmpty());
 
-        mUNM.start(mDefaultRequest);
+        mUNM.startTrackDefaultNetwork(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
         assertFalse(mCM.listening.isEmpty());
         assertTrue(mCM.isListeningForAll());
 
         mUNM.stop();
-        assertTrue(mCM.hasNoCallbacks());
+        assertTrue(mCM.onlyHasDefaultCallbacks());
     }
 
     @Test
     public void testCallbacksRegistered() {
-        mUNM.start(mDefaultRequest);
-        verify(mCM, times(1)).registerNetworkCallback(
-                any(NetworkRequest.class), any(NetworkCallback.class), any(Handler.class));
+        mUNM.startTrackDefaultNetwork(mDefaultRequest);
         verify(mCM, times(1)).requestNetwork(
                 eq(mDefaultRequest), any(NetworkCallback.class), any(Handler.class));
+        mUNM.startObserveAllNetworks();
+        verify(mCM, times(1)).registerNetworkCallback(
+                any(NetworkRequest.class), any(NetworkCallback.class), any(Handler.class));
 
         mUNM.stop();
-        verify(mCM, times(2)).unregisterNetworkCallback(any(NetworkCallback.class));
+        verify(mCM, times(1)).unregisterNetworkCallback(any(NetworkCallback.class));
     }
 
     @Test
@@ -176,7 +178,7 @@ public class UpstreamNetworkMonitorTest {
         assertFalse(mUNM.mobileNetworkRequested());
         assertEquals(0, mCM.requested.size());
 
-        mUNM.start(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
         assertFalse(mUNM.mobileNetworkRequested());
         assertEquals(0, mCM.requested.size());
 
@@ -199,11 +201,9 @@ public class UpstreamNetworkMonitorTest {
         assertFalse(mUNM.mobileNetworkRequested());
         assertEquals(0, mCM.requested.size());
 
-        mUNM.start(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
         verify(mCM, times(1)).registerNetworkCallback(
                 any(NetworkRequest.class), any(NetworkCallback.class), any(Handler.class));
-        verify(mCM, times(1)).requestNetwork(
-                eq(mDefaultRequest), any(NetworkCallback.class), any(Handler.class));
         assertFalse(mUNM.mobileNetworkRequested());
         assertEquals(0, mCM.requested.size());
 
@@ -227,7 +227,7 @@ public class UpstreamNetworkMonitorTest {
         assertTrue(mCM.isDunRequested());
 
         mUNM.stop();
-        verify(mCM, times(3)).unregisterNetworkCallback(any(NetworkCallback.class));
+        verify(mCM, times(2)).unregisterNetworkCallback(any(NetworkCallback.class));
 
         verifyNoMoreInteractions(mCM);
     }
@@ -237,7 +237,7 @@ public class UpstreamNetworkMonitorTest {
         assertFalse(mUNM.mobileNetworkRequested());
         assertEquals(0, mCM.requested.size());
 
-        mUNM.start(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
         assertFalse(mUNM.mobileNetworkRequested());
         assertEquals(0, mCM.requested.size());
 
@@ -257,7 +257,7 @@ public class UpstreamNetworkMonitorTest {
 
     @Test
     public void testUpdateMobileRequiresDun() throws Exception {
-        mUNM.start(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
 
         // Test going from no-DUN to DUN correctly re-registers callbacks.
         mUNM.updateMobileRequiresDun(false);
@@ -285,7 +285,8 @@ public class UpstreamNetworkMonitorTest {
         final Collection<Integer> preferredTypes = new ArrayList<>();
         preferredTypes.add(TYPE_WIFI);
 
-        mUNM.start(mDefaultRequest);
+        mUNM.startTrackDefaultNetwork(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
         // There are no networks, so there is nothing to select.
         assertSatisfiesLegacyType(TYPE_NONE, mUNM.selectPreferredUpstreamType(preferredTypes));
 
@@ -350,7 +351,8 @@ public class UpstreamNetworkMonitorTest {
 
     @Test
     public void testGetCurrentPreferredUpstream() throws Exception {
-        mUNM.start(mDefaultRequest);
+        mUNM.startTrackDefaultNetwork(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
         mUNM.updateMobileRequiresDun(false);
 
         // [0] Mobile connects, DUN not required -> mobile selected.
@@ -389,7 +391,8 @@ public class UpstreamNetworkMonitorTest {
 
     @Test
     public void testLocalPrefixes() throws Exception {
-        mUNM.start(mDefaultRequest);
+        mUNM.startTrackDefaultNetwork(mDefaultRequest);
+        mUNM.startObserveAllNetworks();
 
         // [0] Test minimum set of local prefixes.
         Set<IpPrefix> local = mUNM.getLocalPrefixes();
@@ -521,11 +524,19 @@ public class UpstreamNetworkMonitorTest {
         }
 
         boolean hasNoCallbacks() {
-            return allCallbacks.isEmpty() &&
-                   trackingDefault.isEmpty() &&
-                   listening.isEmpty() &&
-                   requested.isEmpty() &&
-                   legacyTypeMap.isEmpty();
+            return allCallbacks.isEmpty()
+                    && trackingDefault.isEmpty()
+                    && listening.isEmpty()
+                    && requested.isEmpty()
+                    && legacyTypeMap.isEmpty();
+        }
+
+        boolean onlyHasDefaultCallbacks() {
+            return (allCallbacks.size() == 1)
+                    && (trackingDefault.size() == 1)
+                    && listening.isEmpty()
+                    && requested.isEmpty()
+                    && legacyTypeMap.isEmpty();
         }
 
         boolean isListeningForAll() {