Make Tethering file NetworkRequests only if needed.
Currently, Tethering files NetworkRequests even when
config_tether_upstream_automatic is enabled. This is incorrect:
when the automatic upstream selection is enabled, the tethering
upstream should always follow the default network and there is
no need to file any requests.
These requests are harmful when tethering is not using cellular
as its upstream, because:
- If the device does not use mobile data always on, the request
causes the cellular network to be brought up, causing power
draw.
- Even if the device does use mobile data always on, the request
causes the cellular network to come to the foreground, which
allows all apps to access it, causing potential data usage.
Amend the existing testGetCurrentPreferredUpstream to cover these
changes, by making that test case always set automatic upstream
mode. This does not result in any loss of meaningful test
coverage, because getCurrentPreferredUpstream is only ever called
when chooseUpstreamAutomatically is enabled.
Bug: 173068192
Test: atest TetheringTests
Change-Id: I068a5338699a3ed04f24f97f785ea89ff5890e50
diff --git a/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java b/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
index eb22448..513f07c 100644
--- a/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
+++ b/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
@@ -199,17 +199,22 @@
private void reevaluateUpstreamRequirements(boolean tryCell, boolean autoUpstream,
boolean dunRequired) {
- final boolean mobileRequestWasRequired = mTryCell && (mDunRequired || !mAutoUpstream);
- final boolean mobileRequestIsRequired = tryCell && (dunRequired || !autoUpstream);
+ final boolean mobileRequestRequired = tryCell && (dunRequired || !autoUpstream);
final boolean dunRequiredChanged = (mDunRequired != dunRequired);
mTryCell = tryCell;
mDunRequired = dunRequired;
mAutoUpstream = autoUpstream;
- if (dunRequiredChanged && mobileNetworkRequested()) {
- releaseMobileNetworkRequest();
+ if (mobileRequestRequired && !mobileNetworkRequested()) {
registerMobileNetworkRequest();
+ } else if (mobileNetworkRequested() && !mobileRequestRequired) {
+ releaseMobileNetworkRequest();
+ } else if (mobileNetworkRequested() && dunRequiredChanged) {
+ releaseMobileNetworkRequest();
+ if (mobileRequestRequired) {
+ registerMobileNetworkRequest();
+ }
}
}
@@ -221,11 +226,6 @@
*/
public void setTryCell(boolean tryCell) {
reevaluateUpstreamRequirements(tryCell, mAutoUpstream, mDunRequired);
- if (tryCell) {
- registerMobileNetworkRequest();
- } else {
- releaseMobileNetworkRequest();
- }
}
/** Informs UpstreamNetworkMonitor of upstream configuration parameters. */
@@ -274,7 +274,9 @@
// 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);
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java
index 4376b61..43d32b5 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/UpstreamNetworkMonitorTest.java
@@ -41,7 +41,6 @@
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 @@
mCM = spy(new TestConnectivityManager(mContext, mCS, sDefaultRequest));
mSM = new TestStateMachine();
- mUNM = new UpstreamNetworkMonitor(
- (ConnectivityManager) mCM, mSM, mLog, EVENT_UNM_UPDATE);
+ mUNM = new UpstreamNetworkMonitor(mCM, mSM, mLog, EVENT_UNM_UPDATE);
}
@After public void tearDown() throws Exception {
@@ -373,38 +371,44 @@
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
- // triggered by code over in Tethering, but once that has been moved
- // into UNM we should test for this here.
+ assertEquals(1, mCM.requested.size());
+ assertTrue(isDunRequested());
// [6] DUN network arrives -> DUN selected
final TestNetworkAgent dunAgent = new TestNetworkAgent(mCM, CELL_CAPABILITIES);
@@ -412,10 +416,22 @@
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