Merge changes Ic349e0b0,I061733d7 into main

* changes:
  Fix LOHS stealing pending tethered SoftAP requests
  Pre-emptively fail pending pan requests upon opposite request
This commit is contained in:
Quang Luong 2025-02-12 19:08:19 -08:00 • committed by Gerrit Code Review
commit 5817d88cf5
2 changed files with 46 additions and 41 deletions

View file

@ -132,7 +132,6 @@ import android.telephony.TelephonyManager;
import android.text.TextUtils;
import android.util.ArrayMap;
import android.util.Log;
import android.util.Pair;
import android.util.SparseArray;
import androidx.annotation.NonNull;
@ -292,7 +291,7 @@ public class Tethering {
private SettingsObserver mSettingsObserver;
private BluetoothPan mBluetoothPan;
private PanServiceListener mBluetoothPanListener;
private ArrayList<Pair<Boolean, IIntResultListener>> mPendingPanRequests;
private final ArrayList<IIntResultListener> mPendingPanRequestListeners;
// AIDL doesn't support Set<Integer>. Maintain a int bitmap here. When the bitmap is passed to
// TetheringManager, TetheringManager would convert it to a set of Integer types.
// mSupportedTypeBitmap should always be updated inside tethering internal thread but it may be
@ -312,7 +311,7 @@ public class Tethering {
// This is intended to ensrure that if something calls startTethering(bluetooth) just after
// bluetooth is enabled. Before onServiceConnected is called, store the calls into this
// list and handle them as soon as onServiceConnected is called.
mPendingPanRequests = new ArrayList<>();
mPendingPanRequestListeners = new ArrayList<>();
mTetherStates = new ArrayMap<>();
mConnectedClientsTracker = new ConnectedClientsTracker();
@ -862,12 +861,19 @@ public class Tethering {
return;
}
// The reference of IIntResultListener should only exist when application want to start
// tethering but tethering is not bound to pan service yet. Even if the calling process
// dies, the referenice of IIntResultListener would still keep in mPendingPanRequests. Once
// tethering bound to pan service (onServiceConnected) or bluetooth just crash
// (onServiceDisconnected), all the references from mPendingPanRequests would be cleared.
mPendingPanRequests.add(new Pair(enable, listener));
if (!enable) {
// The service is not connected. If disabling tethering, there's no point starting
// the service just to stop tethering since tethering is not started. Just remove
// any pending requests to enable tethering, and notify them that they have failed.
for (IIntResultListener pendingListener : mPendingPanRequestListeners) {
sendTetherResult(pendingListener, TETHER_ERROR_SERVICE_UNAVAIL,
TETHERING_BLUETOOTH);
}
mPendingPanRequestListeners.clear();
sendTetherResult(listener, TETHER_ERROR_NO_ERROR, TETHERING_BLUETOOTH);
return;
}
mPendingPanRequestListeners.add(listener);
// Bluetooth tethering is not a popular feature. To avoid bind to bluetooth pan service all
// the time but user never use bluetooth tethering. mBluetoothPanListener is created first
@ -893,10 +899,11 @@ public class Tethering {
mBluetoothPan = (BluetoothPan) proxy;
mIsConnected = true;
for (Pair<Boolean, IIntResultListener> request : mPendingPanRequests) {
setBluetoothTetheringSettings(mBluetoothPan, request.first, request.second);
for (IIntResultListener pendingListener : mPendingPanRequestListeners) {
setBluetoothTetheringSettings(mBluetoothPan, true /* enable */,
pendingListener);
}
mPendingPanRequests.clear();
mPendingPanRequestListeners.clear();
});
}
@ -907,11 +914,11 @@ public class Tethering {
// reachable before next onServiceConnected.
mIsConnected = false;
for (Pair<Boolean, IIntResultListener> request : mPendingPanRequests) {
sendTetherResult(request.second, TETHER_ERROR_SERVICE_UNAVAIL,
for (IIntResultListener pendingListener : mPendingPanRequestListeners) {
sendTetherResult(pendingListener, TETHER_ERROR_SERVICE_UNAVAIL,
TETHERING_BLUETOOTH);
}
mPendingPanRequests.clear();
mPendingPanRequestListeners.clear();
mBluetoothIfaceRequest = null;
mBluetoothCallback = null;
maybeDisableBluetoothIpServing();
@ -1730,27 +1737,7 @@ public class Tethering {
break;
case IFACE_IP_MODE_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);
if (request == null) {
request = createImplicitLocalOnlyTetheringRequest(TETHERING_WIFI);
} else {
// If we've taken this request from the pending requests, then force the
// connectivity scope to local so we start IpServer in local-only mode (this
// matches historical behavior). This should be OK since the connectivity scope
// is only used to start IpServer in the correct mode.
// TODO: This will break fuzzy-matching logic for start/stop tethering in the
// future. Figure out how to reconcile that with this forced scope.
// Possibly ignore the connectivity scope for wifi if both requests are
// explicit, since explicit Wifi requests may only have
// CONNECTIVITY_SCOPE_GLOBAL. Or possibly, don't add any edge case and
// treat it as a different request entirely.
request.getParcel().connectivityScope = CONNECTIVITY_SCOPE_LOCAL;
}
request = createImplicitLocalOnlyTetheringRequest(type);
break;
default:
mLog.e("Cannot enable IP serving in unknown WiFi mode: " + wifiIpMode);

View file

@ -93,7 +93,6 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.notNull;
@ -2787,6 +2786,10 @@ public class TetheringTest {
public void assertHasResult() {
if (!mHasResult) fail("No callback result");
}
public void assertDoesNotHaveResult() {
if (mHasResult) fail("Has callback result");
}
}
@Test
@ -3395,10 +3398,9 @@ public class TetheringTest {
}
@Test
@IgnoreUpTo(Build.VERSION_CODES.S_V2)
public void testBluetoothTethering() throws Exception {
initTetheringOnTestThread();
// Switch to @IgnoreUpTo(Build.VERSION_CODES.S_V2) when it is available for AOSP.
assumeTrue(isAtLeastT());
final ResultListener result = new ResultListener(TETHER_ERROR_NO_ERROR);
mockBluetoothSettings(true /* bluetoothOn */, true /* tetheringOn */);
@ -3432,10 +3434,9 @@ public class TetheringTest {
}
@Test
@IgnoreAfter(Build.VERSION_CODES.S_V2)
public void testBluetoothTetheringBeforeT() throws Exception {
initTetheringOnTestThread();
// Switch to @IgnoreAfter(Build.VERSION_CODES.S_V2) when it is available for AOSP.
assumeFalse(isAtLeastT());
final ResultListener result = new ResultListener(TETHER_ERROR_NO_ERROR);
mockBluetoothSettings(true /* bluetoothOn */, true /* tetheringOn */);
@ -3515,6 +3516,23 @@ public class TetheringTest {
verifyNetdCommandForBtTearDown();
}
@Test
public void testPendingPanEnableRequestFailedUponDisableRequest() throws Exception {
initTetheringOnTestThread();
mockBluetoothSettings(true /* bluetoothOn */, true /* tetheringOn */);
final ResultListener failedEnable = new ResultListener(TETHER_ERROR_SERVICE_UNAVAIL);
mTethering.startTethering(createTetheringRequest(TETHERING_BLUETOOTH),
TEST_CALLER_PKG, failedEnable);
mLooper.dispatchAll();
failedEnable.assertDoesNotHaveResult();
// Stop tethering before the pan service connects. This should fail the enable request.
mTethering.stopTethering(TETHERING_BLUETOOTH);
mLooper.dispatchAll();
failedEnable.assertHasResult();
}
private void mockBluetoothSettings(boolean bluetoothOn, boolean tetheringOn) {
when(mBluetoothAdapter.isEnabled()).thenReturn(bluetoothOn);
when(mBluetoothPan.isTetheringOn()).thenReturn(tetheringOn);