Merge changes Ib0c4a52e,Iff7d2981,Ie8af8b71,I7884faf8 into main

* changes:
  Rename mActiveTetheringRequests to mPendingTetheringRequests
  Give the two variants of ensureIpServerStarted different names.
  Move the TetheringRequest further up the stack.
  Un-nest legacyTether by moving lambda contents to new method.
This commit is contained in:
Quang Luong 2025-01-29 13:48:17 -08:00 • committed by Gerrit Code Review
commit 343429d013
2 changed files with 132 additions and 76 deletions

View file

@ -241,7 +241,7 @@ public class Tethering {
// Currently active tethering requests per tethering type. Only one of each type can be
// requested at a time. After a tethering type is requested, the map keeps tethering parameters
// to be used after the interface comes up asynchronously.
private final SparseArray<TetheringRequest> mActiveTetheringRequests =
private final SparseArray<TetheringRequest> mPendingTetheringRequests =
new SparseArray<>();
private final Context mContext;
@ -640,7 +640,7 @@ public class Tethering {
}
if (enabled) {
ensureIpServerStarted(iface);
ensureIpServerStartedForInterface(iface);
} else {
ensureIpServerStopped(iface);
}
@ -701,7 +701,7 @@ public class Tethering {
final IIntResultListener listener) {
mHandler.post(() -> {
final int type = request.getTetheringType();
final TetheringRequest unfinishedRequest = mActiveTetheringRequests.get(type);
final TetheringRequest unfinishedRequest = mPendingTetheringRequests.get(type);
// If tethering is already enabled with a different request,
// disable before re-enabling.
if (unfinishedRequest != null && !unfinishedRequest.equalsIgnoreUidPackage(request)) {
@ -709,7 +709,7 @@ public class Tethering {
unfinishedRequest.getInterfaceName(), null);
mEntitlementMgr.stopProvisioningIfNeeded(type);
}
mActiveTetheringRequests.put(type, request);
mPendingTetheringRequests.put(type, request);
if (request.isExemptFromEntitlementCheck()) {
mEntitlementMgr.setExemptedDownstreamType(type);
@ -728,7 +728,7 @@ public class Tethering {
});
}
void stopTetheringInternal(int type) {
mActiveTetheringRequests.remove(type);
mPendingTetheringRequests.remove(type);
enableTetheringInternal(type, false /* disabled */, null, null);
mEntitlementMgr.stopProvisioningIfNeeded(type);
@ -782,7 +782,7 @@ public class Tethering {
// If changing tethering fail, remove corresponding request
// no matter who trigger the start/stop.
if (result != TETHER_ERROR_NO_ERROR) {
mActiveTetheringRequests.remove(type);
mPendingTetheringRequests.remove(type);
mTetheringMetrics.updateErrorCode(type, result);
mTetheringMetrics.sendReport(type);
}
@ -944,7 +944,9 @@ public class Tethering {
public void onAvailable(String iface) {
if (this != mBluetoothCallback) return;
enableIpServing(TETHERING_BLUETOOTH, iface, getRequestedState(TETHERING_BLUETOOTH));
final TetheringRequest request = getPendingTetheringRequest(TETHERING_BLUETOOTH);
enableIpServing(request, TETHERING_BLUETOOTH, iface,
getRequestedState(TETHERING_BLUETOOTH));
mConfiguredBluetoothIface = iface;
}
@ -999,7 +1001,10 @@ public class Tethering {
// Ethernet callback arrived after Ethernet tethering stopped. Ignore.
return;
}
enableIpServing(TETHERING_ETHERNET, iface, getRequestedState(TETHERING_ETHERNET));
final TetheringRequest request = getPendingTetheringRequest(TETHERING_ETHERNET);
enableIpServing(request, TETHERING_ETHERNET, iface,
getRequestedState(TETHERING_ETHERNET));
mConfiguredEthernetIface = iface;
}
@ -1020,7 +1025,9 @@ public class Tethering {
} else {
mConfiguredVirtualIface = iface;
}
final TetheringRequest request = getPendingTetheringRequest(TETHERING_VIRTUAL);
enableIpServing(
request,
TETHERING_VIRTUAL,
mConfiguredVirtualIface,
getRequestedState(TETHERING_VIRTUAL));
@ -1064,7 +1071,7 @@ public class Tethering {
*/
@NonNull
private TetheringRequest getOrCreatePendingTetheringRequest(int type) {
TetheringRequest pending = mActiveTetheringRequests.get(type);
TetheringRequest pending = mPendingTetheringRequests.get(type);
if (pending != null) return pending;
Log.w(TAG, "No pending TetheringRequest for type " + type + " found, creating a placeholder"
@ -1074,6 +1081,59 @@ public class Tethering {
return placeholder;
}
private void handleLegacyTether(String iface, int requestedState,
final IIntResultListener listener) {
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.VANILLA_ICE_CREAM) {
// After V, the TetheringManager and ConnectivityManager tether and untether methods
// throw UnsupportedOperationException, so this cannot happen in normal use. Ensure
// that this code cannot run even if callers use raw binder calls or other
// unsupported methods.
return;
}
final int type = ifaceNameToType(iface);
if (type == TETHERING_INVALID) {
Log.e(TAG, "Ignoring call to legacy tether for unknown iface " + iface);
try {
listener.onResult(TETHER_ERROR_UNKNOWN_IFACE);
} catch (RemoteException e) { }
}
int result = tetherInternal(null, iface, requestedState);
switch (type) {
case TETHERING_WIFI:
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Legacy tether API called on Wifi iface " + iface,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI);
if (result == TETHER_ERROR_NO_ERROR) {
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Legacy tether API succeeded on Wifi iface " + iface,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_SUCCESS);
}
break;
case TETHERING_WIFI_P2P:
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Legacy tether API called on Wifi P2P iface " + iface,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P);
if (result == TETHER_ERROR_NO_ERROR) {
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Legacy tether API succeeded on Wifi P2P iface " + iface,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P_SUCCESS);
}
break;
default:
// Do nothing
break;
}
try {
listener.onResult(result);
} catch (RemoteException e) { }
}
/**
* Legacy tether API that starts tethering with CONNECTIVITY_SCOPE_GLOBAL on the given iface.
*
@ -1088,51 +1148,14 @@ public class Tethering {
* those broadcasts are disabled by OEM.
*/
void legacyTether(String iface, int requestedState, final IIntResultListener listener) {
if (Build.VERSION.SDK_INT > Build.VERSION_CODES.VANILLA_ICE_CREAM) {
// After V, the TetheringManager and ConnectivityManager tether and untether methods
// throw UnsupportedOperationException, so this cannot happen in normal use. Ensure
// that this code cannot run even if callers use raw binder calls or other
// unsupported methods.
return;
}
mHandler.post(() -> {
int result = tetherInternal(iface, requestedState);
switch (ifaceNameToType(iface)) {
case TETHERING_WIFI:
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Legacy tether API called on Wifi iface " + iface,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI);
if (result == TETHER_ERROR_NO_ERROR) {
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Legacy tether API succeeded on Wifi iface " + iface,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_SUCCESS);
}
break;
case TETHERING_WIFI_P2P:
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Legacy tether API called on Wifi P2P iface " + iface,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P);
if (result == TETHER_ERROR_NO_ERROR) {
TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
"Legacy tether API succeeded on Wifi P2P iface " + iface,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P_SUCCESS);
}
break;
default:
// Do nothing
break;
}
try {
listener.onResult(result);
} catch (RemoteException e) { }
});
mHandler.post(() -> handleLegacyTether(iface, requestedState, listener));
}
private int tetherInternal(String iface, int requestedState) {
// TODO: is it possible to make the request @NonNull here?
// Because the IpServer doesn't actually look at (almost anything in) the request, we can just
// make callers synthesize a reasonable-looking request and make this @NonNull.
private int tetherInternal(@Nullable TetheringRequest request, String iface,
int requestedState) {
if (DBG) Log.d(TAG, "Tethering " + iface);
TetherState tetherState = mTetherStates.get(iface);
if (tetherState == null) {
@ -1145,15 +1168,19 @@ public class Tethering {
Log.e(TAG, "Tried to Tether an unavailable iface: " + iface + ", ignoring");
return TETHER_ERROR_UNAVAIL_IFACE;
}
// NOTE: If a CMD_TETHER_REQUESTED message is already in the TISM's queue but not yet
// NOTE: If a CMD_TETHER_REQUESTED message is already in the IpServer's queue but not yet
// processed, this will be a no-op and it will not return an error.
//
// This code cannot race with untether() because they both run on the handler thread.
//
// TODO: once the request is @NonNull, it will be correct to fetch the type from it, because
// the type used to fetch (or synthesize) the request is the same as the type passed in at
// IpServer creation time.
// BUG: the connectivity scope in the request is ignored. This matches current behaviour.
// TODO: see if the connectivity scope can be made (or is already) correct, and fetch the
// requested state from there.
final int type = tetherState.ipServer.interfaceType();
final TetheringRequest request = mActiveTetheringRequests.get(type, null);
if (request != null) {
mActiveTetheringRequests.delete(type);
}
mPendingTetheringRequests.remove(type);
tetherState.ipServer.enable(requestedState, request);
return TETHER_ERROR_NO_ERROR;
}
@ -1213,8 +1240,9 @@ public class Tethering {
return mEntitlementMgr.isTetherProvisioningRequired(cfg);
}
// TODO: make this take a TetheringRequest instead.
private int getRequestedState(int type) {
final TetheringRequest request = mActiveTetheringRequests.get(type);
final TetheringRequest request = mPendingTetheringRequests.get(type);
// The request could have been deleted before we had a chance to complete it.
// If so, assume that the scope is the default scope for this tethering type.
@ -1529,8 +1557,8 @@ public class Tethering {
}
@VisibleForTesting
SparseArray<TetheringRequest> getActiveTetheringRequests() {
return mActiveTetheringRequests;
SparseArray<TetheringRequest> getPendingTetheringRequests() {
return mPendingTetheringRequests;
}
@VisibleForTesting
@ -1590,14 +1618,21 @@ public class Tethering {
}
}
private void enableIpServing(int tetheringType, String ifname, int ipServingMode) {
enableIpServing(tetheringType, ifname, ipServingMode, false /* isNcm */);
final TetheringRequest getPendingTetheringRequest(int type) {
return mPendingTetheringRequests.get(type, null);
}
private void enableIpServing(int tetheringType, String ifname, int ipServingMode,
boolean isNcm) {
ensureIpServerStarted(ifname, tetheringType, isNcm);
if (tetherInternal(ifname, ipServingMode) != TETHER_ERROR_NO_ERROR) {
// TODO: make the request @NonNull and move the tetheringType and ipServingMode into it.
private void enableIpServing(@Nullable TetheringRequest request, int tetheringType,
String ifname, int ipServingMode) {
enableIpServing(request, tetheringType, ifname, ipServingMode, false /* isNcm */);
}
// TODO: make the request @NonNull and move the tetheringType and ipServingMode into it.
private void enableIpServing(@Nullable TetheringRequest request, int tetheringType,
String ifname, int ipServingMode, boolean isNcm) {
ensureIpServerStartedForType(ifname, tetheringType, isNcm);
if (tetherInternal(request, ifname, ipServingMode) != TETHER_ERROR_NO_ERROR) {
Log.e(TAG, "unable start tethering on iface " + ifname);
}
}
@ -1649,7 +1684,9 @@ public class Tethering {
mLog.e(ifname + " is not a tetherable iface, ignoring");
return;
}
enableIpServing(type, ifname, IpServer.STATE_LOCAL_ONLY);
// No need to look for active requests. There can never be explicit requests for
// TETHERING_WIFI_P2P because enableTetheringInternal ignores that type.
enableIpServing(null, type, ifname, IpServer.STATE_LOCAL_ONLY);
}
private void disableWifiP2pIpServingIfNeeded(String ifname) {
@ -1659,17 +1696,35 @@ public class Tethering {
disableWifiIpServingCommon(TETHERING_WIFI_P2P, ifname);
}
// TODO: fold this in to enableWifiIpServing. We cannot do this at the moment because there
// are tests that send wifi AP broadcasts with a null interface. But if this can't happen on
// real devices, we should fix those tests to always pass in an interface.
private int maybeInferWifiTetheringType(String ifname) {
return SdkLevel.isAtLeastT() ? TETHERING_WIFI : ifaceNameToType(ifname);
}
private void enableWifiIpServing(String ifname, int wifiIpMode) {
mLog.log("request WiFi tethering - interface=" + ifname + " state=" + wifiIpMode);
// Map wifiIpMode values to IpServer.Callback serving states.
TetheringRequest request;
final int ipServingMode;
final int type;
switch (wifiIpMode) {
case IFACE_IP_MODE_TETHERED:
ipServingMode = IpServer.STATE_TETHERED;
type = maybeInferWifiTetheringType(ifname);
request = getPendingTetheringRequest(type);
break;
case IFACE_IP_MODE_LOCAL_ONLY:
ipServingMode = IpServer.STATE_LOCAL_ONLY;
type = maybeInferWifiTetheringType(ifname);
// BUG: this request is incorrect - instead of LOHS, it will reflect whatever
// request (if any) is being processed for TETHERING_WIFI. However, this is the
// historical behaviour. It mostly works because a) most of the time there is no
// such request b) tetherinternal doesn't look at the connectivity scope of the
// request, it takes the scope from requestedState.
request = getPendingTetheringRequest(type);
break;
default:
mLog.e("Cannot enable IP serving in unknown WiFi mode: " + wifiIpMode);
@ -1678,14 +1733,13 @@ public class Tethering {
// After T, tethering always trust the iface pass by state change intent. This allow
// tethering to deprecate tetherable wifi regexs after T.
final int type = SdkLevel.isAtLeastT() ? TETHERING_WIFI : ifaceNameToType(ifname);
if (!checkTetherableType(type)) {
mLog.e(ifname + " is not a tetherable iface, ignoring");
return;
}
if (!TextUtils.isEmpty(ifname)) {
enableIpServing(type, ifname, ipServingMode);
enableIpServing(request, type, ifname, ipServingMode);
} else {
mLog.e("Cannot enable IP serving on missing interface name");
}
@ -1715,10 +1769,11 @@ public class Tethering {
return;
}
final TetheringRequest request = getPendingTetheringRequest(tetheringType);
if (ifaces != null) {
for (String iface : ifaces) {
if (ifaceNameToType(iface) == tetheringType) {
enableIpServing(tetheringType, iface, requestedState, forNcmFunction);
enableIpServing(request, tetheringType, iface, requestedState, forNcmFunction);
return;
}
}
@ -2950,7 +3005,7 @@ public class Tethering {
return type != TETHERING_INVALID;
}
private void ensureIpServerStarted(final String iface) {
private void ensureIpServerStartedForInterface(final String iface) {
// If we don't care about this type of interface, ignore.
final int interfaceType = ifaceNameToType(iface);
if (!checkTetherableType(interfaceType)) {
@ -2959,10 +3014,11 @@ public class Tethering {
return;
}
ensureIpServerStarted(iface, interfaceType, false /* isNcm */);
ensureIpServerStartedForType(iface, interfaceType, false /* isNcm */);
}
private void ensureIpServerStarted(final String iface, int interfaceType, boolean isNcm) {
private void ensureIpServerStartedForType(final String iface, int interfaceType,
boolean isNcm) {
// If we have already started a TISM for this interface, skip.
if (mTetherStates.containsKey(iface)) {
mLog.log("active iface (" + iface + ") reported as added, ignoring");

View file

@ -957,8 +957,8 @@ public class TetheringTest {
mTethering.startTethering(request, TEST_CALLER_PKG, null);
mLooper.dispatchAll();
assertEquals(1, mTethering.getActiveTetheringRequests().size());
assertEquals(request, mTethering.getActiveTetheringRequests().get(TETHERING_USB));
assertEquals(1, mTethering.getPendingTetheringRequests().size());
assertEquals(request, mTethering.getPendingTetheringRequests().get(TETHERING_USB));
if (mTethering.getTetheringConfiguration().isUsingNcm()) {
verify(mUsbManager).setCurrentFunctions(UsbManager.FUNCTION_NCM);
@ -2170,7 +2170,7 @@ public class TetheringTest {
runUsbTethering(upstreamState);
assertContains(Arrays.asList(mTethering.getTetheredIfaces()), TEST_RNDIS_IFNAME);
assertTrue(mTethering.isTetheringActive());
assertEquals(0, mTethering.getActiveTetheringRequests().size());
assertEquals(0, mTethering.getPendingTetheringRequests().size());
final Tethering.UserRestrictionActionListener ural = makeUserRestrictionActionListener(
mTethering, false /* currentDisallow */, true /* nextDisallow */);