Make Tethering file NetworkRequests only if needed. am: 2bcde0fd97

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1556199

Change-Id: Ieaedc7d8d7bea358b42dc4e8e5b4533c69c7c8e4

Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java

@@ -199,17 +199,22 @@ public class UpstreamNetworkMonitor {
 
     private void reevaluateUpstreamRequirements(boolean tryCell, boolean autoUpstream,
             boolean dunRequired) {
-        final boolean mobileRequestWasRequired = mTryCell && (mDunRequired || !mAutoUpstream);
+        final boolean mobileRequestRequired = tryCell && (dunRequired || !autoUpstream);
-        final boolean mobileRequestIsRequired = tryCell && (dunRequired || !autoUpstream);
         final boolean dunRequiredChanged = (mDunRequired != dunRequired);
 
         mTryCell = tryCell;
         mDunRequired = dunRequired;
         mAutoUpstream = autoUpstream;
 
-        if (dunRequiredChanged && mobileNetworkRequested()) {
+        if (mobileRequestRequired && !mobileNetworkRequested()) {
-            releaseMobileNetworkRequest();
             registerMobileNetworkRequest();
+        } else if (mobileNetworkRequested() && !mobileRequestRequired) {
+            releaseMobileNetworkRequest();
+        } else if (mobileNetworkRequested() && dunRequiredChanged) {
+            releaseMobileNetworkRequest();
+            if (mobileRequestRequired) {
+                registerMobileNetworkRequest();
+            }
         }
     }
 
@@ -221,11 +226,6 @@ public class UpstreamNetworkMonitor {
      */
     public void setTryCell(boolean tryCell) {
         reevaluateUpstreamRequirements(tryCell, mAutoUpstream, mDunRequired);
-        if (tryCell) {
-            registerMobileNetworkRequest();
-        } else {
-            releaseMobileNetworkRequest();
-        }
     }
 
     /** Informs UpstreamNetworkMonitor of upstream configuration parameters. */
@@ -274,7 +274,9 @@ public class UpstreamNetworkMonitor {
         // TODO: Change the timeout from 0 (no onUnavailable callback) to some
         // moderate callback timeout. This might be useful for updating some UI.
         // Additionally, we log a message to aid in any subsequent debugging.
-        mLog.i("requesting mobile upstream network: " + mobileUpstreamRequest);
+        mLog.i("requesting mobile upstream network: " + mobileUpstreamRequest
+                + " mTryCell=" + mTryCell + " mAutoUpstream=" + mAutoUpstream
+                + " mDunRequired=" + mDunRequired);
 
         cm().requestNetwork(mobileUpstreamRequest, 0, legacyType, mHandler,
                 mMobileNetworkCallback);

Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java

@@ -41,7 +41,6 @@ import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 import android.content.Context;
-import android.net.ConnectivityManager;
 import android.net.ConnectivityManager.NetworkCallback;
 import android.net.IConnectivityManager;
 import android.net.IpPrefix;
@@ -111,8 +110,7 @@ public class UpstreamNetworkMonitorTest {
 
         mCM = spy(new TestConnectivityManager(mContext, mCS, sDefaultRequest));
         mSM = new TestStateMachine();
-        mUNM = new UpstreamNetworkMonitor(
+        mUNM = new UpstreamNetworkMonitor(mCM, mSM, mLog, EVENT_UNM_UPDATE);
-                (ConnectivityManager) mCM, mSM, mLog, EVENT_UNM_UPDATE);
     }
 
     @After public void tearDown() throws Exception {
@@ -373,38 +371,44 @@ public class UpstreamNetworkMonitorTest {
     public void testGetCurrentPreferredUpstream() throws Exception {
         mUNM.startTrackDefaultNetwork(sDefaultRequest, mEntitleMgr);
         mUNM.startObserveAllNetworks();
-        mUNM.setUpstreamConfig(false /* autoUpstream */, false /* dunRequired */);
+        mUNM.setUpstreamConfig(true /* autoUpstream */, false /* dunRequired */);
+        mUNM.setTryCell(true);
 
         // [0] Mobile connects, DUN not required -> mobile selected.
         final TestNetworkAgent cellAgent = new TestNetworkAgent(mCM, CELL_CAPABILITIES);
         cellAgent.fakeConnect();
         mCM.makeDefaultNetwork(cellAgent);
         assertEquals(cellAgent.networkId, mUNM.getCurrentPreferredUpstream().network);
+        assertEquals(0, mCM.requested.size());
 
         // [1] Mobile connects but not permitted -> null selected
         when(mEntitleMgr.isCellularUpstreamPermitted()).thenReturn(false);
         assertEquals(null, mUNM.getCurrentPreferredUpstream());
         when(mEntitleMgr.isCellularUpstreamPermitted()).thenReturn(true);
+        assertEquals(0, mCM.requested.size());
 
         // [2] WiFi connects but not validated/promoted to default -> mobile selected.
         final TestNetworkAgent wifiAgent = new TestNetworkAgent(mCM, WIFI_CAPABILITIES);
         wifiAgent.fakeConnect();
         assertEquals(cellAgent.networkId, mUNM.getCurrentPreferredUpstream().network);
+        assertEquals(0, mCM.requested.size());
 
         // [3] WiFi validates and is promoted to the default network -> WiFi selected.
         mCM.makeDefaultNetwork(wifiAgent);
         assertEquals(wifiAgent.networkId, mUNM.getCurrentPreferredUpstream().network);
+        assertEquals(0, mCM.requested.size());
 
         // [4] DUN required, no other changes -> WiFi still selected
         mUNM.setUpstreamConfig(false /* autoUpstream */, true /* dunRequired */);
         assertEquals(wifiAgent.networkId, mUNM.getCurrentPreferredUpstream().network);
+        assertEquals(1, mCM.requested.size());
+        assertTrue(isDunRequested());
 
         // [5] WiFi no longer validated, mobile becomes default, DUN required -> null selected.
         mCM.makeDefaultNetwork(cellAgent);
         assertEquals(null, mUNM.getCurrentPreferredUpstream());
-        // TODO: make sure that a DUN request has been filed. This is currently
+        assertEquals(1, mCM.requested.size());
-        // triggered by code over in Tethering, but once that has been moved
+        assertTrue(isDunRequested());
-        // into UNM we should test for this here.
 
         // [6] DUN network arrives -> DUN selected
         final TestNetworkAgent dunAgent = new TestNetworkAgent(mCM, CELL_CAPABILITIES);
@@ -412,10 +416,22 @@ public class UpstreamNetworkMonitorTest {
         dunAgent.networkCapabilities.removeCapability(NET_CAPABILITY_INTERNET);
         dunAgent.fakeConnect();
         assertEquals(dunAgent.networkId, mUNM.getCurrentPreferredUpstream().network);
+        assertEquals(1, mCM.requested.size());
 
         // [7] Mobile is not permitted -> null selected
         when(mEntitleMgr.isCellularUpstreamPermitted()).thenReturn(false);
         assertEquals(null, mUNM.getCurrentPreferredUpstream());
+        assertEquals(1, mCM.requested.size());
+
+        // [7] Mobile is permitted again -> DUN selected
+        when(mEntitleMgr.isCellularUpstreamPermitted()).thenReturn(true);
+        assertEquals(dunAgent.networkId, mUNM.getCurrentPreferredUpstream().network);
+        assertEquals(1, mCM.requested.size());
+
+        // [8] DUN no longer required -> request is withdrawn
+        mUNM.setUpstreamConfig(true /* autoUpstream */, false /* dunRequired */);
+        assertEquals(0, mCM.requested.size());
+        assertFalse(isDunRequested());
     }
 
     @Test