Cleanup RadioOnHelper and supporting methods

Minor changes to comments and fixs possible bug
introduced in refactor: The phone type should
be saved before the radio is turned on in order
for the TelephonyConnectionService to switch
the phone type later if it changes when the
call is placed.

Test: Telephony Unit Tests
Merged-In: Id6c1c3299bacbb4335cb5bf62f76de7a7b90a298
Change-Id: Id6c1c3299bacbb4335cb5bf62f76de7a7b90a298
diff --git a/proguard.flags b/proguard.flags
index e8646eb..c707f76 100644
--- a/proguard.flags
+++ b/proguard.flags
@@ -7,4 +7,4 @@
 -keepclassmembers class * {
 @**.NeededForTesting *;
 }
--verbose
+-verbose
\ No newline at end of file
diff --git a/src/com/android/services/telephony/RadioOnHelper.java b/src/com/android/services/telephony/RadioOnHelper.java
index ce7f72a..daa7665 100644
--- a/src/com/android/services/telephony/RadioOnHelper.java
+++ b/src/com/android/services/telephony/RadioOnHelper.java
@@ -30,12 +30,11 @@
 import java.util.List;
 
 /**
- * Helper class that implements special behavior related to emergency calls or make phone calls when
- * radio is power off due to the device being on Bluetooth. Specifically, this class handles the
- * case of the user trying to dial an emergency number while the radio is off (i.e. the device is
- * in airplane mode) or a normal number while the radio is off (because of the device is on
- * Bluetooth), by forcibly turning the radio back on, waiting for it to come up, and then retrying
- * the call.
+ * Helper class that implements special behavior related to emergency calls or making phone calls
+ * when the radio is in the POWER_OFF STATE. Specifically, this class handles the case of the user
+ * trying to dial an emergency number while the radio is off (i.e. the device is in airplane mode)
+ * or a normal number while the radio is off (because of the device is on Bluetooth), by turning the
+ * radio back on, waiting for it to come up, and then retrying the call.
  */
 public class RadioOnHelper implements RadioOnStateListener.Callback {
 
@@ -65,7 +64,7 @@
      *
      * This method kicks off the following sequence:
      * - Power on the radio for each Phone
-     * - Listen for the service state change event telling us the radio has come up.
+     * - Listen for radio events telling us the radio has come up.
      * - Retry if we've gone a significant amount of time without any response from the radio.
      * - Finally, clean up any leftover state.
      *
@@ -73,7 +72,7 @@
      * RadioOnHelper's handler (thus ensuring that the rest of the sequence is entirely
      * serialized, and runs on the main looper.)
      */
-    public void enableRadioOnCalling(RadioOnStateListener.Callback callback) {
+    public void triggerRadioOnAndListen(RadioOnStateListener.Callback callback) {
         setupListeners();
         mCallback = callback;
         mInProgressListeners.clear();
@@ -109,8 +108,8 @@
 
             // Post the broadcast intend for change in airplane mode
             // TODO: We really should not be in charge of sending this broadcast.
-            //     If changing the setting is sufficent to trigger all of the rest of the logic,
-            //     then that should also trigger the broadcast intent.
+            // If changing the setting is sufficient to trigger all of the rest of the logic,
+            // then that should also trigger the broadcast intent.
             Intent intent = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
             intent.putExtra("state", false);
             mContext.sendBroadcastAsUser(intent, UserHandle.ALL);
diff --git a/src/com/android/services/telephony/RadioOnStateListener.java b/src/com/android/services/telephony/RadioOnStateListener.java
index 7bfa9c6..91a7d77 100644
--- a/src/com/android/services/telephony/RadioOnStateListener.java
+++ b/src/com/android/services/telephony/RadioOnStateListener.java
@@ -24,22 +24,24 @@
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.os.SomeArgs;
-import com.android.internal.telephony.CommandsInterface;
 import com.android.internal.telephony.Phone;
-import com.android.internal.telephony.PhoneConstants;
-import com.android.internal.telephony.SubscriptionController;
 
 /**
- * Helper class that listens to a Phone's radio state and sends a callback when the radio state of
- * that Phone is either "in service" or ("emergency calls only." if is emergency).
+ * Helper class that listens to a Phone's radio state and sends an onComplete callback when we
+ * return true for isOkToCall.
  */
 public class RadioOnStateListener {
 
-    /**
-     * Receives the result of the RadioOnStateListener's attempt to turn on the radio.
-     */
     interface Callback {
+        /**
+         * Receives the result of the RadioOnStateListener's attempt to turn on the radio.
+         */
         void onComplete(RadioOnStateListener listener, boolean isRadioReady);
+
+        /**
+         * Given the Phone and the new service state of that phone, return whether or not this
+         * phone is ok to call. If it is, onComplete will be called shortly after.
+         */
         boolean isOkToCall(Phone phone, int serviceState);
     }
 
@@ -140,8 +142,9 @@
     }
 
     /**
-     * Handles the SERVICE_STATE_CHANGED event. Normally this event tells us that the radio has
-     * finally come up. In that case, it's now safe to actually place the RadioOn call.
+     * Handles the SERVICE_STATE_CHANGED event. This event tells us that the radio state has changed
+     * and is probably coming up. We can now check to see if the conditions are met to place the
+     * call with {@link Callback#isOkToCall}
      */
     private void onServiceStateChanged(ServiceState state) {
         Log.d(this, "onServiceStateChanged(), new state = %s, Phone = %s", state,
@@ -151,7 +154,7 @@
         // - STATE_IN_SERVICE        // Normal operation
         // - STATE_OUT_OF_SERVICE    // Still searching for an operator to register to,
         //                           // or no radio signal
-        // - STATE_EMERGENCY_ONLY    // Phone is locked; only emergency numbers are allowed
+        // - STATE_EMERGENCY_ONLY    // Only emergency numbers are allowed; currently not used
         // - STATE_POWER_OFF         // Radio is explicitly powered off (airplane mode)
 
         if (isOkToCall(state.getState())) {
@@ -167,9 +170,7 @@
     }
 
     /**
-     * We currently only look to make sure that the radio is on before dialing. We should be able to
-     * make emergency calls at any time after the radio has been powered on and isn't in the
-     * UNAVAILABLE state, even if it is reporting the OUT_OF_SERVICE state.
+     * Callback to see if it is okay to call yet, given the current conditions.
      */
     private boolean isOkToCall(int serviceState) {
         return (mCallback == null) ? false : mCallback.isOkToCall(mPhone, serviceState);
diff --git a/src/com/android/services/telephony/TelephonyConnectionService.java b/src/com/android/services/telephony/TelephonyConnectionService.java
index 13fef03..53bfd68 100644
--- a/src/com/android/services/telephony/TelephonyConnectionService.java
+++ b/src/com/android/services/telephony/TelephonyConnectionService.java
@@ -118,6 +118,7 @@
                     mTelephonyConnectionServiceProxy);
 
     private ComponentName mExpectedComponentName = null;
+    private RadioOnHelper mRadioOnHelper;
     private EmergencyTonePlayer mEmergencyTonePlayer;
 
     // Contains one TelephonyConnection that has placed a call and a memory of which Phones it has
@@ -354,37 +355,45 @@
         final boolean isEmergencyNumber =
                 PhoneNumberUtils.isLocalEmergencyNumber(this, numberToDial);
 
+
         final boolean isAirplaneModeOn = Settings.Global.getInt(getContentResolver(),
                 Settings.Global.AIRPLANE_MODE_ON, 0) > 0;
 
-        if ((isEmergencyNumber && (!isRadioOn() || isAirplaneModeOn))
-                || isRadioPowerDownOnBluetooth()) {
+        boolean needToTurnOnRadio = (isEmergencyNumber && (!isRadioOn() || isAirplaneModeOn))
+                || isRadioPowerDownOnBluetooth();
+
+        if (needToTurnOnRadio) {
             final Uri resultHandle = handle;
             // By default, Connection based on the default Phone, since we need to return to Telecom
             // now.
+            final int originalPhoneType = PhoneFactory.getDefaultPhone().getPhoneType();
             final Connection resultConnection = getTelephonyConnection(request, numberToDial,
                     isEmergencyNumber, resultHandle, PhoneFactory.getDefaultPhone());
-            RadioOnHelper radioOnHelper = new RadioOnHelper(this);
-            radioOnHelper.enableRadioOnCalling(new RadioOnStateListener.Callback() {
+            if (mRadioOnHelper == null) {
+                mRadioOnHelper = new RadioOnHelper(this);
+            }
+            mRadioOnHelper.triggerRadioOnAndListen(new RadioOnStateListener.Callback() {
                 @Override
-                public void onComplete(RadioOnStateListener listener,
-                        boolean isRadioReady) {
-                    handleOnComplete(isRadioReady,
-                            isEmergencyNumber,
-                            resultConnection,
-                            request,
-                            numberToDial,
-                            resultHandle);
+                public void onComplete(RadioOnStateListener listener, boolean isRadioReady) {
+                    handleOnComplete(isRadioReady, isEmergencyNumber, resultConnection, request,
+                            numberToDial, resultHandle, originalPhoneType);
                 }
 
                 @Override
                 public boolean isOkToCall(Phone phone, int serviceState) {
                     if (isEmergencyNumber) {
+                        // We currently only look to make sure that the radio is on before dialing.
+                        // We should be able to make emergency calls at any time after the radio has
+                        // been powered on and isn't in the UNAVAILABLE state, even if it is
+                        // reporting the OUT_OF_SERVICE state.
                         return (phone.getState() == PhoneConstants.State.OFFHOOK)
-                                || phone.getServiceStateTracker().isRadioOn();
+                            || phone.getServiceStateTracker().isRadioOn();
                     } else {
+                        // It is not an emergency number, so wait until we are in service and ready
+                        // to make calls. This can happen when we power down the radio on bluetooth
+                        // to save power on watches.
                         return (phone.getState() == PhoneConstants.State.OFFHOOK)
-                                || serviceState == ServiceState.STATE_IN_SERVICE;
+                            || serviceState == ServiceState.STATE_IN_SERVICE;
                     }
                 }
             });
@@ -432,16 +441,13 @@
     /**
      * Handle the onComplete callback of RadioOnStateListener.
      */
-    private void handleOnComplete(boolean isRadioReady,
-            boolean isEmergencyNumber,
-            Connection originalConnection,
-            ConnectionRequest request,
-            String numberToDial,
-            Uri handle) {
+    private void handleOnComplete(boolean isRadioReady, boolean isEmergencyNumber,
+            Connection originalConnection, ConnectionRequest request, String numberToDial,
+            Uri handle, int originalPhoneType) {
         // Make sure the Call has not already been canceled by the user.
         if (originalConnection.getState() == Connection.STATE_DISCONNECTED) {
-            Log.i(this, "Emergency call disconnected before the outgoing call was "
-                    + "placed. Skipping emergency call placement.");
+            Log.i(this, "Call disconnected before the outgoing call was placed. Skipping call "
+                    + "placement.");
             return;
         }
         if (isRadioReady) {
@@ -449,23 +455,20 @@
             // successfully.
             final Phone phone = getPhoneForAccount(request.getAccountHandle(),
                     isEmergencyNumber);
-            // If the PhoneType of the Phone being used is different than the Default
-            // Phone, then we need create a new Connection using that PhoneType and
-            // replace it in Telecom.
-            if (phone.getPhoneType() != PhoneFactory.getDefaultPhone().getPhoneType()) {
+            // If the PhoneType of the Phone being used is different than the Default Phone, then we
+            // need create a new Connection using that PhoneType and replace it in Telecom.
+            if (phone.getPhoneType() != originalPhoneType) {
                 Connection repConnection = getTelephonyConnection(request, numberToDial,
                         isEmergencyNumber, handle, phone);
                 // If there was a failure, the resulting connection will not be a
                 // TelephonyConnection, so don't place the call, just return!
                 if (repConnection instanceof TelephonyConnection) {
-                    placeOutgoingConnection((TelephonyConnection) repConnection, phone,
-                            request);
+                    placeOutgoingConnection((TelephonyConnection) repConnection, phone, request);
                 }
                 // Notify Telecom of the new Connection type.
                 // TODO: Switch out the underlying connection instead of creating a new
                 // one and causing UI Jank.
-                addExistingConnection(PhoneUtils.makePstnPhoneAccountHandle(phone),
-                        repConnection);
+                addExistingConnection(PhoneUtils.makePstnPhoneAccountHandle(phone), repConnection);
                 // Remove the old connection from Telecom after.
                 originalConnection.setDisconnected(
                         DisconnectCauseUtil.toTelecomDisconnectCause(
@@ -473,8 +476,7 @@
                                 "Reconnecting outgoing Emergency Call."));
                 originalConnection.destroy();
             } else {
-                placeOutgoingConnection((TelephonyConnection) originalConnection,
-                        phone, request);
+                placeOutgoingConnection((TelephonyConnection) originalConnection, phone, request);
             }
         } else {
             Log.w(this, "onCreateOutgoingConnection, failed to turn on radio");
diff --git a/tests/src/com/android/services/telephony/RadioOnStateListenerTest.java b/tests/src/com/android/services/telephony/RadioOnStateListenerTest.java
index abac699..fb214cc 100644
--- a/tests/src/com/android/services/telephony/RadioOnStateListenerTest.java
+++ b/tests/src/com/android/services/telephony/RadioOnStateListenerTest.java
@@ -34,7 +34,8 @@
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 
-import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Matchers.isNull;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
@@ -54,7 +55,6 @@
     private static final long TIMEOUT_MS = 100;
 
     @Mock Phone mMockPhone;
-    @Mock ServiceStateTracker mMockServiceStateTracker;
     @Mock RadioOnStateListener.Callback mCallback;
     RadioOnStateListener mListener;
 
@@ -86,22 +86,16 @@
     }
 
     /**
-     * Prerequisites:
-     *  - Phone is IN_SERVICE
-     *  - Radio is on
-     *
-     * Test: Send SERVICE_STATE_CHANGED message
-     *
-     * Result: callback's onComplete is called with the isRadioReady=true
+     * {@link RadioOnStateListener.Callback#isOkToCall(int)} returns true, so we are expecting
+     * {@link RadioOnStateListener.Callback#onComplete(boolean)} to return true.
      */
     @Test
     @SmallTest
-    public void testPhoneChangeState_InService() {
+    public void testPhoneChangeState_OkToCallTrue() {
         ServiceState state = new ServiceState();
         state.setState(ServiceState.STATE_IN_SERVICE);
         when(mMockPhone.getState()).thenReturn(PhoneConstants.State.IDLE);
-        when(mMockPhone.getServiceStateTracker()).thenReturn(mMockServiceStateTracker);
-        when(mMockServiceStateTracker.isRadioOn()).thenReturn(true);
+        when(mCallback.isOkToCall(eq(mMockPhone), anyInt())).thenReturn(true);
         mListener.waitForRadioOn(mMockPhone, mCallback);
         waitForHandlerAction(mListener.getHandler(), TIMEOUT_MS);
 
@@ -113,24 +107,17 @@
     }
 
     /**
-     * Prerequisites:
-     *  - Phone is OUT_OF_SERVICE (emergency calls only)
-     *  - Radio is on
-     *
-     * Test: Send SERVICE_STATE_CHANGED message
-     *
-     * Result: callback's onComplete is called with the isRadioReady=true
+     * We never receive a {@link RadioOnStateListener.Callback#onComplete(boolean)} because
+     * {@link RadioOnStateListener.Callback#isOkToCall(int)} returns false.
      */
     @Test
     @SmallTest
-    public void testPhoneChangeState_EmergencyCalls() {
+    public void testPhoneChangeState_NoOkToCall_Timeout() {
         ServiceState state = new ServiceState();
         state.setState(ServiceState.STATE_OUT_OF_SERVICE);
-        state.setEmergencyOnly(true);
         when(mMockPhone.getState()).thenReturn(PhoneConstants.State.IDLE);
+        when(mCallback.isOkToCall(eq(mMockPhone), anyInt())).thenReturn(false);
         when(mMockPhone.getServiceState()).thenReturn(state);
-        when(mMockPhone.getServiceStateTracker()).thenReturn(mMockServiceStateTracker);
-        when(mMockServiceStateTracker.isRadioOn()).thenReturn(true);
         mListener.waitForRadioOn(mMockPhone, mCallback);
         waitForHandlerAction(mListener.getHandler(), TIMEOUT_MS);
 
@@ -138,91 +125,22 @@
                 new AsyncResult(null, state, null)).sendToTarget();
 
         waitForHandlerAction(mListener.getHandler(), TIMEOUT_MS);
-        verify(mCallback).onComplete(eq(mListener), eq(true));
+        verify(mCallback, never()).onComplete(any(RadioOnStateListener.class), anyBoolean());
     }
 
     /**
-     * Prerequisites:
-     *  - Phone is OUT_OF_SERVICE
-     *  - Radio is on
-     *
-     * Test: Send SERVICE_STATE_CHANGED message
-     *
-     * Result: callback's onComplete is called with the isRadioReady=true. Even though the radio is
-     * not reporting emergency calls only, we still send onComplete so that the radio can trigger
-     * the emergency call.
-     */
-    @Test
-    @SmallTest
-    public void testPhoneChangeState_OutOfService() {
-        ServiceState state = new ServiceState();
-        state.setState(ServiceState.STATE_OUT_OF_SERVICE);
-        when(mMockPhone.getState()).thenReturn(PhoneConstants.State.IDLE);
-        when(mMockPhone.getServiceState()).thenReturn(state);
-        when(mMockPhone.getServiceStateTracker()).thenReturn(mMockServiceStateTracker);
-        when(mMockServiceStateTracker.isRadioOn()).thenReturn(true);
-        mListener.waitForRadioOn(mMockPhone, mCallback);
-        waitForHandlerAction(mListener.getHandler(), TIMEOUT_MS);
-
-        // Still expect an answer because we will be sending the onComplete message as soon as the
-        // radio is confirmed to be on, whether or not it is out of service or not.
-        mListener.getHandler().obtainMessage(RadioOnStateListener.MSG_SERVICE_STATE_CHANGED,
-                new AsyncResult(null, state, null)).sendToTarget();
-
-        waitForHandlerAction(mListener.getHandler(), TIMEOUT_MS);
-        verify(mCallback).onComplete(eq(mListener), eq(true));
-    }
-
-    /**
-     * Prerequisites:
-     *  - Phone is OUT_OF_SERVICE (emergency calls only)
-     *  - Radio is on
-     *
-     * Test: Wait for retry timer to complete (don't send ServiceState changed message)
-     *
-     * Result: callback's onComplete is called with the isRadioReady=true.
+     * Tests {@link RadioOnStateListener.Callback#isOkToCall(int)} returning false and hitting the
+     * max number of retries. This should result in
+     * {@link RadioOnStateListener.Callback#onComplete(boolean)} returning false.
      */
     @Test
     @FlakyTest
-    @SmallTest
-    public void testTimeout_EmergencyCalls() {
-        ServiceState state = new ServiceState();
-        state.setState(ServiceState.STATE_OUT_OF_SERVICE);
-        state.setEmergencyOnly(true);
-        when(mMockPhone.getState()).thenReturn(PhoneConstants.State.IDLE);
-        when(mMockPhone.getServiceState()).thenReturn(state);
-        when(mMockPhone.getServiceStateTracker()).thenReturn(mMockServiceStateTracker);
-        when(mMockServiceStateTracker.isRadioOn()).thenReturn(true);
-        mListener.setTimeBetweenRetriesMillis(100);
-
-        // Wait for the timer to expire and check state manually in onRetryTimeout
-        mListener.waitForRadioOn(mMockPhone, mCallback);
-        waitForHandlerActionDelayed(mListener.getHandler(), TIMEOUT_MS, 500);
-
-        verify(mCallback).onComplete(eq(mListener), eq(true));
-    }
-
-    /**
-     * Prerequisites:
-     *  - Phone is OUT_OF_SERVICE
-     *  - Radio is off
-     *
-     * Test: Wait for retry timer to complete, no ServiceState changed messages received.
-     *
-     * Result:
-     * - callback's onComplete is called with the isRadioReady=false.
-     * - setRadioPower was send twice (tried to turn on the radio)
-     */
-    @Test
-    @FlakyTest
-    @SmallTest
     public void testTimeout_RetryFailure() {
         ServiceState state = new ServiceState();
         state.setState(ServiceState.STATE_POWER_OFF);
         when(mMockPhone.getState()).thenReturn(PhoneConstants.State.IDLE);
         when(mMockPhone.getServiceState()).thenReturn(state);
-        when(mMockPhone.getServiceStateTracker()).thenReturn(mMockServiceStateTracker);
-        when(mMockServiceStateTracker.isRadioOn()).thenReturn(false);
+        when(mCallback.isOkToCall(eq(mMockPhone), anyInt())).thenReturn(false);
         mListener.setTimeBetweenRetriesMillis(50);
         mListener.setMaxNumRetries(2);