Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cf7020b7 authored by Sailesh Nepal's avatar Sailesh Nepal
Browse files

Clean up and fix bugs in ConnectionService.createConnection 

This CL fixes serveral issues with the createConnection code:
  - it uses failureCode/failureMessage which were never set.
    Renamed to disconnectCode and disconnectMessage and set
    those fields in Connection.setDisconnected
  - Connection.CANCELED_CONNECTION was static and it caused
    lots of log spew which was confusing. Changed to create
    a new connection every time, same as failure
  - moved sNullConnection from Connection to ConnectionService
  - made FailureSignalingConnection private and removed type
    checks for it. Using disconnect code is better, this is
    already what ConnectionServiceWrapper does

Note, the current code still expects connections to be cancelled
or failed in synchronously. This bug is being tracked separately.

Bug: 17156304
Change-Id: I0b13a78b738c4bf37a69de9fd5dcd17be0c45c14
parent 1553a528

api/current.txt

+2 −2
Original line number Diff line number Diff line
@@ -28802,8 +28802,8 @@ package android.telecomm { 
    method public final int getCallerDisplayNamePresentation();

    method public final android.telecomm.Conference getConference();

    method public final java.util.List<android.telecomm.Connection> getConferenceableConnections();

    method public final int getFailureCode();

    method public final java.lang.String getFailureMessage();

    method public final int getDisconnectCause();

    method public final java.lang.String getDisconnectMessage();

    method public final android.net.Uri getHandle();

    method public final int getHandlePresentation();

    method public final int getState();

telecomm/java/android/telecomm/Connection.java

+20 −30
Original line number Diff line number Diff line
@@ -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() {

telecomm/java/android/telecomm/ConnectionService.java

+25 −23
Original line number Diff line number Diff line
@@ -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();

        }

        connectionCreated(callId, request, createdConnection);

        Log.d(this, "createConnection, connection: %s", connection);

        if (connection == null) {

            connection = Connection.createFailedConnection(DisconnectCause.OUTGOING_FAILURE, null);

        }



    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;

    }

}

telecomm/java/android/telecomm/ParcelableConnection.java

+12 −12
Original line number Diff line number Diff line
@@ -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);

    }

}