Merge "Clean up and fix bugs in ConnectionService.createConnection" into lmp-dev

3 files changed, 57 insertions, 65 deletions

diff --git a/telecomm/java/android/telecomm/Connection.java b/telecomm/java/android/telecomm/Connection.java
index c1d5715..c307a25 100644
--- a/telecomm/java/android/telecomm/Connection.java
+++ b/telecomm/java/android/telecomm/Connection.java
@@ -65,8 +65,6 @@ public abstract class Connection {
     // Flag controlling whether PII is emitted into the logs
     private static final boolean PII_DEBUG = Log.isLoggable(android.util.Log.DEBUG);
 
-    private static Connection sNullConnection;
-
     /** @hide */
     public abstract static class Listener {
         public void onStateChanged(Connection c, int state) {}
@@ -476,9 +474,8 @@ public abstract class Connection {
     private boolean mAudioModeIsVoip;
     private StatusHints mStatusHints;
     private int mVideoState;
-    private int mFailureCode;
-    private String mFailureMessage;
-    private boolean mIsCanceled;
+    private int mDisconnectCause;
+    private String mDisconnectMessage;
     private Conference mConference;
     private ConnectionService mConnectionService;
 
@@ -604,17 +601,17 @@ public abstract class Connection {
     }
 
     /**
-     * @return The failure code ({@see DisconnectCause}) associated with this failed connection.
+     * @return The {@link DisconnectCause} for this connection.
      */
-    public final int getFailureCode() {
-        return mFailureCode;
+    public final int getDisconnectCause() {
+        return mDisconnectCause;
     }
 
     /**
-     * @return The reason for the connection failure. This will not be displayed to the user.
+     * @return The disconnect message for this connection.
      */
-    public final String getFailureMessage() {
-        return mFailureMessage;
+    public final String getDisconnectMessage() {
+        return mDisconnectMessage;
     }
 
     /**
@@ -778,6 +775,8 @@ public abstract class Connection {
      * @param message Optional call-service-provided message about the disconnect.
      */
     public final void setDisconnected(int cause, String message) {
+        mDisconnectCause = cause;
+        mDisconnectMessage = message;
         setState(STATE_DISCONNECTED);
         Log.d(this, "Disconnected with cause %d message %s", cause, message);
         for (Listener l : mListeners) {
@@ -1060,13 +1059,6 @@ public abstract class Connection {
         return builder.toString();
     }
 
-    static synchronized Connection getNullConnection() {
-        if (sNullConnection == null) {
-            sNullConnection = new Connection() {};
-        }
-        return sNullConnection;
-    }
-
     private void setState(int state) {
         if (mState == STATE_DISCONNECTED && mState != state) {
             Log.d(this, "Connection already DISCONNECTED; cannot transition out of this state.");
@@ -1082,31 +1074,29 @@ public abstract class Connection {
         }
     }
 
-    static class FailureSignalingConnection extends Connection {
-        public FailureSignalingConnection(int code, String message) {
-            setDisconnected(code, message);
+    private static class FailureSignalingConnection extends Connection {
+        public FailureSignalingConnection(int cause, String message) {
+            setDisconnected(cause, message);
         }
     }
 
     /**
      * Return a {@code Connection} which represents a failed connection attempt. The returned
-     * {@code Connection} will have a {@link #getFailureCode()} and {@link #getFailureMessage()}
-     * as specified, a {@link #getState()} of {@link #STATE_DISCONNECTED}.
+     * {@code Connection} will have a {@link #getDisconnectCause()} and
+     * {@link #getDisconnectMessage()} as specified, and a {@link #getState()} of
+     * {@link #STATE_DISCONNECTED}.
      * <p>
      * The returned {@code Connection} can be assumed to {@link #destroy()} itself when appropriate,
      * so users of this method need not maintain a reference to its return value to destroy it.
      *
-     * @param code The failure code ({@see DisconnectCause}).
+     * @param cause The disconnect cause, ({@see DisconnectCause}).
      * @param message A reason for why the connection failed (not intended to be shown to the user).
      * @return A {@code Connection} which indicates failure.
      */
-    public static Connection createFailedConnection(final int code, final String message) {
-        return new FailureSignalingConnection(code, message);
+    public static Connection createFailedConnection(int cause, String message) {
+        return new FailureSignalingConnection(cause, message);
     }
 
-    private static final Connection CANCELED_CONNECTION =
-            new FailureSignalingConnection(DisconnectCause.OUTGOING_CANCELED, null);
-
     /**
      * Return a {@code Connection} which represents a canceled connection attempt. The returned
      * {@code Connection} will have state {@link #STATE_DISCONNECTED}, and cannot be moved out of
@@ -1119,7 +1109,7 @@ public abstract class Connection {
      * @return A {@code Connection} which indicates that the underlying call should be canceled.
      */
     public static Connection createCanceledConnection() {
-        return CANCELED_CONNECTION;
+        return new FailureSignalingConnection(DisconnectCause.OUTGOING_CANCELED, null);
     }
 
     private final void  fireOnConferenceableConnectionsChanged() {
diff --git a/telecomm/java/android/telecomm/ConnectionService.java b/telecomm/java/android/telecomm/ConnectionService.java
index 03b6c7b..c805978 100644
--- a/telecomm/java/android/telecomm/ConnectionService.java
+++ b/telecomm/java/android/telecomm/ConnectionService.java
@@ -73,6 +73,8 @@ public abstract class ConnectionService extends Service {
     private static final int MSG_ON_PHONE_ACCOUNT_CLICKED = 15;
     private static final int MSG_REMOVE_CONNECTION_SERVICE_ADAPTER = 16;
 
+    private static Connection sNullConnection;
+
     private final Map<String, Connection> mConnectionById = new HashMap<>();
     private final Map<Connection, String> mIdByConnection = new HashMap<>();
     private final Map<String, Conference> mConferenceById = new HashMap<>();
@@ -498,36 +500,29 @@ public abstract class ConnectionService extends Service {
             final String callId,
             final ConnectionRequest request,
             boolean isIncoming) {
-        Log.d(this, "call %s", request);
+        Log.d(this, "createConnection, callManagerAccount: %s, callId: %s, request: %s, " +
+                "isIncoming: %b", callManagerAccount, callId, request, isIncoming);
 
-        Connection createdConnection = isIncoming
+        Connection connection = isIncoming
                 ? onCreateIncomingConnection(callManagerAccount, request)
                 : onCreateOutgoingConnection(callManagerAccount, request);
-
-        if (createdConnection == null) {
-            Log.d(this, "adapter handleCreateConnectionComplete CANCELED %s", callId);
-            // Tell telecomm to try a different service.
-            createdConnection = Connection.createCanceledConnection();
+        Log.d(this, "createConnection, connection: %s", connection);
+        if (connection == null) {
+            connection = Connection.createFailedConnection(DisconnectCause.OUTGOING_FAILURE, null);
         }
-        connectionCreated(callId, request, createdConnection);
-    }
 
-    private void connectionCreated(
-            String callId,
-            ConnectionRequest request,
-            Connection connection) {
-        if (!(connection instanceof Connection.FailureSignalingConnection)) {
+        if (connection.getState() != Connection.STATE_DISCONNECTED) {
             addConnection(callId, connection);
         }
 
         Uri handle = connection.getHandle();
         String number = handle == null ? "null" : handle.getSchemeSpecificPart();
-        Log.v(this, "connectionCreated, parcelableconnection: %s, %d, %s",
+        Log.v(this, "createConnection, number: %s, state: %s, capabilities: %s",
                 Connection.toLogSafePhoneNumber(number),
-                connection.getState(),
+                Connection.stateToString(connection.getState()),
                 PhoneCapabilities.toString(connection.getCallCapabilities()));
 
-        Log.d(this, "adapter handleCreateConnectionSuccessful %s", callId);
+        Log.d(this, "createConnection, calling handleCreateConnectionSuccessful %s", callId);
         mAdapter.handleCreateConnectionComplete(
                 callId,
                 request,
@@ -545,8 +540,8 @@ public abstract class ConnectionService extends Service {
                         connection.isRequestingRingback(),
                         connection.getAudioModeIsVoip(),
                         connection.getStatusHints(),
-                        connection.getFailureCode(),
-                        connection.getFailureMessage()));
+                        connection.getDisconnectCause(),
+                        connection.getDisconnectMessage()));
     }
 
     private void abort(String callId) {
@@ -598,13 +593,13 @@ public abstract class ConnectionService extends Service {
         Log.d(this, "conference %s, %s", callId1, callId2);
 
         Connection connection1 = findConnectionForAction(callId1, "conference");
-        if (connection1 == Connection.getNullConnection()) {
+        if (connection1 == getNullConnection()) {
             Log.w(this, "Connection1 missing in conference request %s.", callId1);
             return;
         }
 
         Connection connection2 = findConnectionForAction(callId2, "conference");
-        if (connection2 == Connection.getNullConnection()) {
+        if (connection2 == getNullConnection()) {
             Log.w(this, "Connection2 missing in conference request %s.", callId2);
             return;
         }
@@ -616,7 +611,7 @@ public abstract class ConnectionService extends Service {
         Log.d(this, "splitFromConference(%s)", callId);
 
         Connection connection = findConnectionForAction(callId, "splitFromConference");
-        if (connection == Connection.getNullConnection()) {
+        if (connection == getNullConnection()) {
             Log.w(this, "Connection missing in conference request %s.", callId);
             return;
         }
@@ -881,6 +876,13 @@ public abstract class ConnectionService extends Service {
             return mConnectionById.get(callId);
         }
         Log.w(this, "%s - Cannot find Connection %s", action, callId);
-        return Connection.getNullConnection();
+        return getNullConnection();
+    }
+
+    static synchronized Connection getNullConnection() {
+        if (sNullConnection == null) {
+            sNullConnection = new Connection() {};
+        }
+        return sNullConnection;
     }
 }
diff --git a/telecomm/java/android/telecomm/ParcelableConnection.java b/telecomm/java/android/telecomm/ParcelableConnection.java
index 30ff5be..812ee55 100644
--- a/telecomm/java/android/telecomm/ParcelableConnection.java
+++ b/telecomm/java/android/telecomm/ParcelableConnection.java
@@ -41,8 +41,8 @@ public final class ParcelableConnection implements Parcelable {
     private boolean mRequestingRingback;
     private boolean mAudioModeIsVoip;
     private StatusHints mStatusHints;
-    private int mFailureCode;
-    private String mFailureMessage;
+    private int mDisconnectCause;
+    private String mDisconnectMessage;
 
     /** @hide */
     public ParcelableConnection(
@@ -58,8 +58,8 @@ public final class ParcelableConnection implements Parcelable {
             boolean requestingRingback,
             boolean audioModeIsVoip,
             StatusHints statusHints,
-            int failureCode,
-            String failureMessage) {
+            int disconnectCause,
+            String disconnectMessage) {
         mPhoneAccount = phoneAccount;
         mState = state;
         mCapabilities = capabilities;
@@ -72,8 +72,8 @@ public final class ParcelableConnection implements Parcelable {
         mRequestingRingback = requestingRingback;
         mAudioModeIsVoip = audioModeIsVoip;
         mStatusHints = statusHints;
-        mFailureCode = failureCode;
-        mFailureMessage = failureMessage;
+        mDisconnectCause = disconnectCause;
+        mDisconnectMessage = disconnectMessage;
     }
 
     public PhoneAccountHandle getPhoneAccount() {
@@ -125,12 +125,12 @@ public final class ParcelableConnection implements Parcelable {
         return mStatusHints;
     }
 
-    public final int getFailureCode() {
-        return mFailureCode;
+    public final int getDisconnectCause() {
+        return mDisconnectCause;
     }
 
-    public final String getFailureMessage() {
-        return mFailureMessage;
+    public final String getDisconnectMessage() {
+        return mDisconnectMessage;
     }
 
     @Override
@@ -212,7 +212,7 @@ public final class ParcelableConnection implements Parcelable {
         destination.writeByte((byte) (mRequestingRingback ? 1 : 0));
         destination.writeByte((byte) (mAudioModeIsVoip ? 1 : 0));
         destination.writeParcelable(mStatusHints, 0);
-        destination.writeInt(mFailureCode);
-        destination.writeString(mFailureMessage);
+        destination.writeInt(mDisconnectCause);
+        destination.writeString(mDisconnectMessage);
     }
 }