Address the comment of aosp/1288493
Bug: 141256482
Test: atest TetheringTests
Change-Id: I0cf337625cee31a47879c59e9b18657ea7624eb4
diff --git a/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java b/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java
index d978541..23b8be1 100644
--- a/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java
+++ b/Tethering/src/com/android/networkstack/tethering/EntitlementManager.java
@@ -37,6 +37,7 @@
import android.content.IntentFilter;
import android.net.util.SharedLog;
import android.os.Bundle;
+import android.os.ConditionVariable;
import android.os.Handler;
import android.os.Parcel;
import android.os.PersistableBundle;
@@ -48,7 +49,6 @@
import android.util.SparseIntArray;
import com.android.internal.annotations.VisibleForTesting;
-import com.android.internal.util.StateMachine;
import java.io.PrintWriter;
import java.util.BitSet;
@@ -73,6 +73,7 @@
private final ComponentName mSilentProvisioningService;
private static final int MS_PER_HOUR = 60 * 60 * 1000;
+ private static final int DUMP_TIMEOUT = 10_000;
// The BitSet is the bit map of each enabled downstream types, ex:
// {@link TetheringManager.TETHERING_WIFI}
@@ -80,14 +81,13 @@
// {@link TetheringManager.TETHERING_BLUETOOTH}
private final BitSet mCurrentDownstreams;
private final Context mContext;
- private final int mPermissionChangeMessageCode;
private final SharedLog mLog;
private final SparseIntArray mEntitlementCacheValue;
private final Handler mHandler;
- private final StateMachine mTetherMasterSM;
// Key: TetheringManager.TETHERING_*(downstream).
// Value: TetheringManager.TETHER_ERROR_{NO_ERROR or PROVISION_FAILED}(provisioning result).
private final SparseIntArray mCurrentEntitlementResults;
+ private final Runnable mPermissionChangeCallback;
private PendingIntent mProvisioningRecheckAlarm;
private boolean mLastCellularUpstreamPermitted = true;
private boolean mUsingCellularAsUpstream = false;
@@ -95,16 +95,15 @@
private OnUiEntitlementFailedListener mListener;
private TetheringConfigurationFetcher mFetcher;
- public EntitlementManager(Context ctx, StateMachine tetherMasterSM, SharedLog log,
- int permissionChangeMessageCode) {
+ public EntitlementManager(Context ctx, Handler h, SharedLog log,
+ Runnable callback) {
mContext = ctx;
mLog = log.forSubComponent(TAG);
mCurrentDownstreams = new BitSet();
mCurrentEntitlementResults = new SparseIntArray();
mEntitlementCacheValue = new SparseIntArray();
- mTetherMasterSM = tetherMasterSM;
- mPermissionChangeMessageCode = permissionChangeMessageCode;
- mHandler = tetherMasterSM.getHandler();
+ mPermissionChangeCallback = callback;
+ mHandler = h;
mContext.registerReceiver(mReceiver, new IntentFilter(ACTION_PROVISIONING_ALARM),
null, mHandler);
mSilentProvisioningService = ComponentName.unflattenFromString(
@@ -409,25 +408,25 @@
}
private void evaluateCellularPermission(final TetheringConfiguration config) {
- final boolean oldPermitted = mLastCellularUpstreamPermitted;
- mLastCellularUpstreamPermitted = isCellularUpstreamPermitted(config);
+ final boolean permitted = isCellularUpstreamPermitted(config);
if (DBG) {
- mLog.i("Cellular permission change from " + oldPermitted
- + " to " + mLastCellularUpstreamPermitted);
+ mLog.i("Cellular permission change from " + mLastCellularUpstreamPermitted
+ + " to " + permitted);
}
- if (mLastCellularUpstreamPermitted != oldPermitted) {
- mLog.log("Cellular permission change: " + mLastCellularUpstreamPermitted);
- mTetherMasterSM.sendMessage(mPermissionChangeMessageCode);
+ if (mLastCellularUpstreamPermitted != permitted) {
+ mLog.log("Cellular permission change: " + permitted);
+ mPermissionChangeCallback.run();
}
// Only schedule periodic re-check when tether is provisioned
// and the result is ok.
- if (mLastCellularUpstreamPermitted && mCurrentEntitlementResults.size() > 0) {
+ if (permitted && mCurrentEntitlementResults.size() > 0) {
scheduleProvisioningRechecks(config);
} else {
cancelTetherProvisioningRechecks();
}
+ mLastCellularUpstreamPermitted = permitted;
}
/**
@@ -486,18 +485,25 @@
* @param pw {@link PrintWriter} is used to print formatted
*/
public void dump(PrintWriter pw) {
- pw.print("isCellularUpstreamPermitted: ");
- pw.println(isCellularUpstreamPermitted());
- for (int type = mCurrentDownstreams.nextSetBit(0); type >= 0;
- type = mCurrentDownstreams.nextSetBit(type + 1)) {
- pw.print("Type: ");
- pw.print(typeString(type));
- if (mCurrentEntitlementResults.indexOfKey(type) > -1) {
- pw.print(", Value: ");
- pw.println(errorString(mCurrentEntitlementResults.get(type)));
- } else {
- pw.println(", Value: empty");
+ final ConditionVariable mWaiting = new ConditionVariable();
+ mHandler.post(() -> {
+ pw.print("isCellularUpstreamPermitted: ");
+ pw.println(isCellularUpstreamPermitted());
+ for (int type = mCurrentDownstreams.nextSetBit(0); type >= 0;
+ type = mCurrentDownstreams.nextSetBit(type + 1)) {
+ pw.print("Type: ");
+ pw.print(typeString(type));
+ if (mCurrentEntitlementResults.indexOfKey(type) > -1) {
+ pw.print(", Value: ");
+ pw.println(errorString(mCurrentEntitlementResults.get(type)));
+ } else {
+ pw.println(", Value: empty");
+ }
}
+ mWaiting.open();
+ });
+ if (!mWaiting.block(DUMP_TIMEOUT)) {
+ pw.println("... dump timed out after " + DUMP_TIMEOUT + "ms");
}
}
diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java
index 2282467..55e9c90 100644
--- a/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -284,8 +284,9 @@
filter.addAction(ACTION_CARRIER_CONFIG_CHANGED);
// EntitlementManager will send EVENT_UPSTREAM_PERMISSION_CHANGED when cellular upstream
// permission is changed according to entitlement check result.
- mEntitlementMgr = mDeps.getEntitlementManager(mContext, mTetherMasterSM, mLog,
- TetherMasterSM.EVENT_UPSTREAM_PERMISSION_CHANGED);
+ mEntitlementMgr = mDeps.getEntitlementManager(mContext, mHandler, mLog,
+ () -> mTetherMasterSM.sendMessage(
+ TetherMasterSM.EVENT_UPSTREAM_PERMISSION_CHANGED));
mEntitlementMgr.setOnUiEntitlementFailedListener((int downstream) -> {
mLog.log("OBSERVED UiEnitlementFailed");
stopTethering(downstream);
diff --git a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
index 802f2ac..ce546c7 100644
--- a/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
+++ b/Tethering/src/com/android/networkstack/tethering/TetheringDependencies.java
@@ -96,9 +96,9 @@
/**
* Get a reference to the EntitlementManager to be used by tethering.
*/
- public EntitlementManager getEntitlementManager(Context ctx, StateMachine target,
- SharedLog log, int what) {
- return new EntitlementManager(ctx, target, log, what);
+ public EntitlementManager getEntitlementManager(Context ctx, Handler h, SharedLog log,
+ Runnable callback) {
+ return new EntitlementManager(ctx, h, log, callback);
}
/**
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java
index 8bd0edc..a692935 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/EntitlementManagerTest.java
@@ -37,6 +37,8 @@
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -45,7 +47,7 @@
import android.content.res.Resources;
import android.net.util.SharedLog;
import android.os.Bundle;
-import android.os.Message;
+import android.os.Handler;
import android.os.PersistableBundle;
import android.os.ResultReceiver;
import android.os.SystemProperties;
@@ -56,26 +58,22 @@
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
-import com.android.internal.util.State;
-import com.android.internal.util.StateMachine;
import com.android.internal.util.test.BroadcastInterceptingContext;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.MockitoSession;
import org.mockito.quality.Strictness;
-import java.util.ArrayList;
-
@RunWith(AndroidJUnit4.class)
@SmallTest
public final class EntitlementManagerTest {
- private static final int EVENT_EM_UPDATE = 1;
private static final String[] PROVISIONING_APP_NAME = {"some", "app"};
private static final String PROVISIONING_NO_UI_APP_NAME = "no_ui_app";
@@ -90,8 +88,8 @@
private final PersistableBundle mCarrierConfig = new PersistableBundle();
private final TestLooper mLooper = new TestLooper();
private Context mMockContext;
+ private Runnable mPermissionChangeCallback;
- private TestStateMachine mSM;
private WrappedEntitlementManager mEnMgr;
private TetheringConfiguration mConfig;
private MockitoSession mMockingSession;
@@ -112,9 +110,9 @@
public int uiProvisionCount = 0;
public int silentProvisionCount = 0;
- public WrappedEntitlementManager(Context ctx, StateMachine target,
- SharedLog log, int what) {
- super(ctx, target, log, what);
+ public WrappedEntitlementManager(Context ctx, Handler h, SharedLog log,
+ Runnable callback) {
+ super(ctx, h, log, callback);
}
public void reset() {
@@ -169,8 +167,9 @@
when(mLog.forSubComponent(anyString())).thenReturn(mLog);
mMockContext = new MockContext(mContext);
- mSM = new TestStateMachine();
- mEnMgr = new WrappedEntitlementManager(mMockContext, mSM, mLog, EVENT_EM_UPDATE);
+ mPermissionChangeCallback = spy(() -> { });
+ mEnMgr = new WrappedEntitlementManager(mMockContext, new Handler(mLooper.getLooper()), mLog,
+ mPermissionChangeCallback);
mEnMgr.setOnUiEntitlementFailedListener(mEntitlementFailedListener);
mConfig = new TetheringConfiguration(mMockContext, mLog, INVALID_SUBSCRIPTION_ID);
mEnMgr.setTetheringConfigurationFetcher(() -> {
@@ -180,10 +179,6 @@
@After
public void tearDown() throws Exception {
- if (mSM != null) {
- mSM.quit();
- mSM = null;
- }
mMockingSession.finishMocking();
}
@@ -350,68 +345,105 @@
mEnMgr.reset();
}
+ private void assertPermissionChangeCallback(InOrder inOrder) {
+ inOrder.verify(mPermissionChangeCallback, times(1)).run();
+ }
+
+ private void assertNoPermissionChange(InOrder inOrder) {
+ inOrder.verifyNoMoreInteractions();
+ }
+
@Test
public void verifyPermissionResult() {
+ final InOrder inOrder = inOrder(mPermissionChangeCallback);
setupForRequiredProvisioning();
mEnMgr.notifyUpstream(true);
mEnMgr.fakeEntitlementResult = TETHER_ERROR_PROVISIONING_FAILED;
mEnMgr.startProvisioningIfNeeded(TETHERING_WIFI, true);
mLooper.dispatchAll();
+ // Permitted: true -> false
+ assertPermissionChangeCallback(inOrder);
assertFalse(mEnMgr.isCellularUpstreamPermitted());
+
mEnMgr.stopProvisioningIfNeeded(TETHERING_WIFI);
mLooper.dispatchAll();
+ // Permitted: false -> false
+ assertNoPermissionChange(inOrder);
+
mEnMgr.fakeEntitlementResult = TETHER_ERROR_NO_ERROR;
mEnMgr.startProvisioningIfNeeded(TETHERING_WIFI, true);
mLooper.dispatchAll();
+ // Permitted: false -> true
+ assertPermissionChangeCallback(inOrder);
assertTrue(mEnMgr.isCellularUpstreamPermitted());
}
@Test
public void verifyPermissionIfAllNotApproved() {
+ final InOrder inOrder = inOrder(mPermissionChangeCallback);
setupForRequiredProvisioning();
mEnMgr.notifyUpstream(true);
mEnMgr.fakeEntitlementResult = TETHER_ERROR_PROVISIONING_FAILED;
mEnMgr.startProvisioningIfNeeded(TETHERING_WIFI, true);
mLooper.dispatchAll();
+ // Permitted: true -> false
+ assertPermissionChangeCallback(inOrder);
assertFalse(mEnMgr.isCellularUpstreamPermitted());
+
mEnMgr.fakeEntitlementResult = TETHER_ERROR_PROVISIONING_FAILED;
mEnMgr.startProvisioningIfNeeded(TETHERING_USB, true);
mLooper.dispatchAll();
+ // Permitted: false -> false
+ assertNoPermissionChange(inOrder);
assertFalse(mEnMgr.isCellularUpstreamPermitted());
+
mEnMgr.fakeEntitlementResult = TETHER_ERROR_PROVISIONING_FAILED;
mEnMgr.startProvisioningIfNeeded(TETHERING_BLUETOOTH, true);
mLooper.dispatchAll();
+ // Permitted: false -> false
+ assertNoPermissionChange(inOrder);
assertFalse(mEnMgr.isCellularUpstreamPermitted());
}
@Test
public void verifyPermissionIfAnyApproved() {
+ final InOrder inOrder = inOrder(mPermissionChangeCallback);
setupForRequiredProvisioning();
mEnMgr.notifyUpstream(true);
mEnMgr.fakeEntitlementResult = TETHER_ERROR_NO_ERROR;
mEnMgr.startProvisioningIfNeeded(TETHERING_WIFI, true);
mLooper.dispatchAll();
+ // Permitted: true -> true
+ assertNoPermissionChange(inOrder);
assertTrue(mEnMgr.isCellularUpstreamPermitted());
- mLooper.dispatchAll();
+
mEnMgr.fakeEntitlementResult = TETHER_ERROR_PROVISIONING_FAILED;
mEnMgr.startProvisioningIfNeeded(TETHERING_USB, true);
mLooper.dispatchAll();
+ // Permitted: true -> true
+ assertNoPermissionChange(inOrder);
assertTrue(mEnMgr.isCellularUpstreamPermitted());
+
mEnMgr.stopProvisioningIfNeeded(TETHERING_WIFI);
mLooper.dispatchAll();
+ // Permitted: true -> false
+ assertPermissionChangeCallback(inOrder);
assertFalse(mEnMgr.isCellularUpstreamPermitted());
-
}
@Test
public void verifyPermissionWhenProvisioningNotStarted() {
+ final InOrder inOrder = inOrder(mPermissionChangeCallback);
assertTrue(mEnMgr.isCellularUpstreamPermitted());
+ assertNoPermissionChange(inOrder);
setupForRequiredProvisioning();
assertFalse(mEnMgr.isCellularUpstreamPermitted());
+ assertNoPermissionChange(inOrder);
}
@Test
public void testRunTetherProvisioning() {
+ final InOrder inOrder = inOrder(mPermissionChangeCallback);
setupForRequiredProvisioning();
// 1. start ui provisioning, upstream is mobile
mEnMgr.fakeEntitlementResult = TETHER_ERROR_NO_ERROR;
@@ -421,16 +453,22 @@
mLooper.dispatchAll();
assertEquals(1, mEnMgr.uiProvisionCount);
assertEquals(0, mEnMgr.silentProvisionCount);
+ // Permitted: true -> true
+ assertNoPermissionChange(inOrder);
assertTrue(mEnMgr.isCellularUpstreamPermitted());
mEnMgr.reset();
+
// 2. start no-ui provisioning
mEnMgr.fakeEntitlementResult = TETHER_ERROR_NO_ERROR;
mEnMgr.startProvisioningIfNeeded(TETHERING_WIFI, false);
mLooper.dispatchAll();
assertEquals(0, mEnMgr.uiProvisionCount);
assertEquals(1, mEnMgr.silentProvisionCount);
+ // Permitted: true -> true
+ assertNoPermissionChange(inOrder);
assertTrue(mEnMgr.isCellularUpstreamPermitted());
mEnMgr.reset();
+
// 3. tear down mobile, then start ui provisioning
mEnMgr.notifyUpstream(false);
mLooper.dispatchAll();
@@ -438,44 +476,58 @@
mLooper.dispatchAll();
assertEquals(0, mEnMgr.uiProvisionCount);
assertEquals(0, mEnMgr.silentProvisionCount);
+ assertNoPermissionChange(inOrder);
mEnMgr.reset();
+
// 4. switch upstream back to mobile
mEnMgr.fakeEntitlementResult = TETHER_ERROR_NO_ERROR;
mEnMgr.notifyUpstream(true);
mLooper.dispatchAll();
assertEquals(1, mEnMgr.uiProvisionCount);
assertEquals(0, mEnMgr.silentProvisionCount);
+ // Permitted: true -> true
+ assertNoPermissionChange(inOrder);
assertTrue(mEnMgr.isCellularUpstreamPermitted());
mEnMgr.reset();
+
// 5. tear down mobile, then switch SIM
mEnMgr.notifyUpstream(false);
mLooper.dispatchAll();
mEnMgr.reevaluateSimCardProvisioning(mConfig);
assertEquals(0, mEnMgr.uiProvisionCount);
assertEquals(0, mEnMgr.silentProvisionCount);
+ assertNoPermissionChange(inOrder);
mEnMgr.reset();
+
// 6. switch upstream back to mobile again
mEnMgr.fakeEntitlementResult = TETHER_ERROR_PROVISIONING_FAILED;
mEnMgr.notifyUpstream(true);
mLooper.dispatchAll();
assertEquals(0, mEnMgr.uiProvisionCount);
assertEquals(3, mEnMgr.silentProvisionCount);
+ // Permitted: true -> false
+ assertPermissionChangeCallback(inOrder);
assertFalse(mEnMgr.isCellularUpstreamPermitted());
mEnMgr.reset();
+
// 7. start ui provisioning, upstream is mobile, downstream is ethernet
mEnMgr.fakeEntitlementResult = TETHER_ERROR_NO_ERROR;
mEnMgr.startProvisioningIfNeeded(TETHERING_ETHERNET, true);
mLooper.dispatchAll();
assertEquals(1, mEnMgr.uiProvisionCount);
assertEquals(0, mEnMgr.silentProvisionCount);
+ // Permitted: false -> true
+ assertPermissionChangeCallback(inOrder);
assertTrue(mEnMgr.isCellularUpstreamPermitted());
mEnMgr.reset();
+
// 8. downstream is invalid
mEnMgr.fakeEntitlementResult = TETHER_ERROR_NO_ERROR;
mEnMgr.startProvisioningIfNeeded(TETHERING_WIFI_P2P, true);
mLooper.dispatchAll();
assertEquals(0, mEnMgr.uiProvisionCount);
assertEquals(0, mEnMgr.silentProvisionCount);
+ assertNoPermissionChange(inOrder);
mEnMgr.reset();
}
@@ -491,32 +543,4 @@
assertEquals(1, mEnMgr.uiProvisionCount);
verify(mEntitlementFailedListener, times(1)).onUiEntitlementFailed(TETHERING_WIFI);
}
-
- public class TestStateMachine extends StateMachine {
- public final ArrayList<Message> messages = new ArrayList<>();
- private final State
- mLoggingState = new EntitlementManagerTest.TestStateMachine.LoggingState();
-
- class LoggingState extends State {
- @Override public void enter() {
- messages.clear();
- }
-
- @Override public void exit() {
- messages.clear();
- }
-
- @Override public boolean processMessage(Message msg) {
- messages.add(msg);
- return false;
- }
- }
-
- public TestStateMachine() {
- super("EntitlementManagerTest.TestStateMachine", mLooper.getLooper());
- addState(mLoggingState);
- setInitialState(mLoggingState);
- super.start();
- }
- }
}
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
index 65d9f47..a67a4b6 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -367,9 +367,9 @@
}
@Override
- public EntitlementManager getEntitlementManager(Context ctx, StateMachine target,
- SharedLog log, int what) {
- mEntitleMgr = spy(super.getEntitlementManager(ctx, target, log, what));
+ public EntitlementManager getEntitlementManager(Context ctx, Handler h, SharedLog log,
+ Runnable callback) {
+ mEntitleMgr = spy(super.getEntitlementManager(ctx, h, log, callback));
return mEntitleMgr;
}
@@ -1754,10 +1754,12 @@
final FileDescriptor mockFd = mock(FileDescriptor.class);
final PrintWriter mockPw = mock(PrintWriter.class);
runUsbTethering(null);
+ mLooper.startAutoDispatch();
mTethering.dump(mockFd, mockPw, new String[0]);
verify(mConfig).dump(any());
verify(mEntitleMgr).dump(any());
verify(mOffloadCtrl).dump(any());
+ mLooper.stopAutoDispatch();
}
// TODO: Test that a request for hotspot mode doesn't interfere with an