Merge "Address the comment of aosp/1288493"

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

@@ -37,6 +37,7 @@ import android.content.Intent;
 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.telephony.CarrierConfigManager;
 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 @@ public class EntitlementManager {
 
     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 @@ public class EntitlementManager {
     // {@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 @@ public class EntitlementManager {
     private OnUiEntitlementFailedListener mListener;
     private TetheringConfigurationFetcher mFetcher;
 
-    public EntitlementManager(Context ctx, StateMachine tetherMasterSM, SharedLog log,
+    public EntitlementManager(Context ctx, Handler h, SharedLog log,
-            int permissionChangeMessageCode) {
+            Runnable callback) {
         mContext = ctx;
         mLog = log.forSubComponent(TAG);
         mCurrentDownstreams = new BitSet();
         mCurrentEntitlementResults = new SparseIntArray();
         mEntitlementCacheValue = new SparseIntArray();
-        mTetherMasterSM = tetherMasterSM;
+        mPermissionChangeCallback = callback;
-        mPermissionChangeMessageCode = permissionChangeMessageCode;
+        mHandler = h;
-        mHandler = tetherMasterSM.getHandler();
         mContext.registerReceiver(mReceiver, new IntentFilter(ACTION_PROVISIONING_ALARM),
                 null, mHandler);
         mSilentProvisioningService = ComponentName.unflattenFromString(
@@ -409,25 +408,25 @@ public class EntitlementManager {
     }
 
     private void evaluateCellularPermission(final TetheringConfiguration config) {
-        final boolean oldPermitted = mLastCellularUpstreamPermitted;
+        final boolean permitted = isCellularUpstreamPermitted(config);
-        mLastCellularUpstreamPermitted = isCellularUpstreamPermitted(config);
 
         if (DBG) {
-            mLog.i("Cellular permission change from " + oldPermitted
+            mLog.i("Cellular permission change from " + mLastCellularUpstreamPermitted
-                    + " to " + mLastCellularUpstreamPermitted);
+                    + " to " + permitted);
         }
 
-        if (mLastCellularUpstreamPermitted != oldPermitted) {
+        if (mLastCellularUpstreamPermitted != permitted) {
-            mLog.log("Cellular permission change: " + mLastCellularUpstreamPermitted);
+            mLog.log("Cellular permission change: " + permitted);
-            mTetherMasterSM.sendMessage(mPermissionChangeMessageCode);
+            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 @@ public class EntitlementManager {
      * @param pw {@link PrintWriter} is used to print formatted
      */
     public void dump(PrintWriter pw) {
-        pw.print("isCellularUpstreamPermitted: ");
+        final ConditionVariable mWaiting = new ConditionVariable();
-        pw.println(isCellularUpstreamPermitted());
+        mHandler.post(() -> {
-        for (int type = mCurrentDownstreams.nextSetBit(0); type >= 0;
+            pw.print("isCellularUpstreamPermitted: ");
-                type = mCurrentDownstreams.nextSetBit(type + 1)) {
+            pw.println(isCellularUpstreamPermitted());
-            pw.print("Type: ");
+            for (int type = mCurrentDownstreams.nextSetBit(0); type >= 0;
-            pw.print(typeString(type));
+                    type = mCurrentDownstreams.nextSetBit(type + 1)) {
-            if (mCurrentEntitlementResults.indexOfKey(type) > -1) {
+                pw.print("Type: ");
-                pw.print(", Value: ");
+                pw.print(typeString(type));
-                pw.println(errorString(mCurrentEntitlementResults.get(type)));
+                if (mCurrentEntitlementResults.indexOfKey(type) > -1) {
-            } else {
+                    pw.print(", Value: ");
-                pw.println(", Value: empty");
+                    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");
         }
     }
 

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

@@ -284,8 +284,9 @@ public class Tethering {
         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,
+        mEntitlementMgr = mDeps.getEntitlementManager(mContext, mHandler, mLog,
-                TetherMasterSM.EVENT_UPSTREAM_PERMISSION_CHANGED);
+                () -> mTetherMasterSM.sendMessage(
+                TetherMasterSM.EVENT_UPSTREAM_PERMISSION_CHANGED));
         mEntitlementMgr.setOnUiEntitlementFailedListener((int downstream) -> {
             mLog.log("OBSERVED UiEnitlementFailed");
             stopTethering(downstream);

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

@@ -96,9 +96,9 @@ public abstract class TetheringDependencies {
     /**
      * Get a reference to the EntitlementManager to be used by tethering.
      */
-    public EntitlementManager getEntitlementManager(Context ctx, StateMachine target,
+    public EntitlementManager getEntitlementManager(Context ctx, Handler h, SharedLog log,
-            SharedLog log, int what) {
+            Runnable callback) {
-        return new EntitlementManager(ctx, target, log, what);
+        return new EntitlementManager(ctx, h, log, callback);
     }
 
     /**

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

@@ -37,6 +37,8 @@ import static org.mockito.Matchers.anyBoolean;
 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.Context;
 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 android.telephony.CarrierConfigManager;
 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 @@ public final class EntitlementManagerTest {
     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 final class EntitlementManagerTest {
         public int uiProvisionCount = 0;
         public int silentProvisionCount = 0;
 
-        public WrappedEntitlementManager(Context ctx, StateMachine target,
+        public WrappedEntitlementManager(Context ctx, Handler h, SharedLog log,
-                SharedLog log, int what) {
+                Runnable callback) {
-            super(ctx, target, log, what);
+            super(ctx, h, log, callback);
         }
 
         public void reset() {
@@ -169,8 +167,9 @@ public final class EntitlementManagerTest {
         when(mLog.forSubComponent(anyString())).thenReturn(mLog);
 
         mMockContext = new MockContext(mContext);
-        mSM = new TestStateMachine();
+        mPermissionChangeCallback = spy(() -> { });
-        mEnMgr = new WrappedEntitlementManager(mMockContext, mSM, mLog, EVENT_EM_UPDATE);
+        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 @@ public final class EntitlementManagerTest {
 
     @After
     public void tearDown() throws Exception {
-        if (mSM != null) {
-            mSM.quit();
-            mSM = null;
-        }
         mMockingSession.finishMocking();
     }
 
@@ -350,68 +345,105 @@ public final class EntitlementManagerTest {
         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 @@ public final class EntitlementManagerTest {
         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 @@ public final class EntitlementManagerTest {
         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 @@ public final class EntitlementManagerTest {
         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();
-        }
-    }
 }

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

@@ -367,9 +367,9 @@ public class TetheringTest {
         }
 
         @Override
-        public EntitlementManager getEntitlementManager(Context ctx, StateMachine target,
+        public EntitlementManager getEntitlementManager(Context ctx, Handler h, SharedLog log,
-                SharedLog log, int what) {
+                Runnable callback) {
-            mEntitleMgr = spy(super.getEntitlementManager(ctx, target, log, what));
+            mEntitleMgr = spy(super.getEntitlementManager(ctx, h, log, callback));
             return mEntitleMgr;
         }
 
@@ -1754,10 +1754,12 @@ public class TetheringTest {
         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