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

Commit fb5560d6 authored by Ihab Awad's avatar Ihab Awad
Browse files

ConnectionService API has only one completed callback (2/3) 

Refactor ConnectionService API so it has only one "completed"
callback, and connection state and failure codes indicates what
happened. Previous design where we had separate callbacks for failure,
cancellation and success was error prone because it was easy to forget
to implement one of them.

This particular change in this set of changes also makes one crucial
fix. The Call object, when it is notified of an unsuccessful attempt
to make a connection, previously told itself:

  setState(CallState.DISCONNECTED);

But that led to its DISCONNECTED state never being published to
anyone, while on the other hand it was now removed from the
CallIdMapper, so nobody could talk to it (symptom: InCall UI hangup
button becomes a no-op). The correct thing to do is:

  CallsManager.getInstance().
      markCallAsDisconnected(this, code, msg);

which goes through the CallsManager to set the appropriate states. I
think that, post-L, our code here is really ripe for a refactoring
that prioritizes encapsulation and more ubiquitous use of the
"listener" pattern, otherwise we're going to have a bunch more of
these kinds of bugs. :)

Bug: 16993846
Bug: 17070939

Change-Id: Ie79e585dfbc2a1b79a3721d749855704cd8270ed
parent 0ef3eee3

src/com/android/telecomm/Call.java

+7 −32
Original line number Diff line number Diff line
@@ -26,7 +26,6 @@ import android.telecomm.Connection; 
import android.telecomm.PhoneCapabilities;

import android.telecomm.PropertyPresentation;

import android.telecomm.CallState;

import android.telecomm.ConnectionRequest;

import android.telecomm.GatewayInfo;

import android.telecomm.ParcelableConnection;

import android.telecomm.PhoneAccount;
@@ -67,7 +66,6 @@ final class Call implements CreateConnectionResponse { 
    interface Listener {

        void onSuccessfulOutgoingCall(Call call, int callState);

        void onFailedOutgoingCall(Call call, int errorCode, String errorMsg);

        void onCancelledOutgoingCall(Call call);

        void onSuccessfulIncomingCall(Call call);

        void onFailedIncomingCall(Call call);

        void onRequestingRingback(Call call, boolean requestingRingback);
@@ -96,8 +94,6 @@ final class Call implements CreateConnectionResponse { 
        @Override

        public void onFailedOutgoingCall(Call call, int errorCode, String errorMsg) {}

        @Override

        public void onCancelledOutgoingCall(Call call) {}

        @Override

        public void onSuccessfulIncomingCall(Call call) {}

        @Override

        public void onFailedIncomingCall(Call call) {}
@@ -612,8 +608,7 @@ final class Call implements CreateConnectionResponse { 
    }



    @Override

    public void handleCreateConnectionSuccessful(

            ConnectionRequest request, ParcelableConnection connection) {

    public void handleCreateConnectionSuccess(ParcelableConnection connection) {

        Log.v(this, "handleCreateConnectionSuccessful %s", connection);

        mCreateConnectionProcessor = null;

        setTargetPhoneAccount(connection.getPhoneAccount());
@@ -646,40 +641,20 @@ final class Call implements CreateConnectionResponse { 
    }



    @Override

    public void handleCreateConnectionFailed(int code, String msg) {

    public void handleCreateConnectionFailure(int code, String msg) {

        mCreateConnectionProcessor = null;

        if (mIsIncoming) {

        clearConnectionService();

            setDisconnectCause(code, null);

            setState(CallState.DISCONNECTED);

        setDisconnectCause(code, msg);

        CallsManager.getInstance().markCallAsDisconnected(this, code, msg);



            for (Listener listener : mListeners) {

                listener.onFailedIncomingCall(this);

            }

        } else {

            for (Listener listener : mListeners) {

                listener.onFailedOutgoingCall(this, code, msg);

            }

            clearConnectionService();

        }

    }



    @Override

    public void handleCreateConnectionCancelled() {

        Log.v(this, "handleCreateConnectionCancelled");

        mCreateConnectionProcessor = null;

        if (mIsIncoming) {

            clearConnectionService();

            setDisconnectCause(DisconnectCause.OUTGOING_CANCELED, null);



            for (Listener listener : mListeners) {

                listener.onFailedIncomingCall(this);

            }

        } else {

            for (Listener listener : mListeners) {

                listener.onCancelledOutgoingCall(this);

                listener.onFailedOutgoingCall(this, code, msg);

            }

            clearConnectionService();

        }

    }
@@ -735,7 +710,7 @@ final class Call implements CreateConnectionResponse { 
            mCreateConnectionProcessor.abort();

        } else if (mState == CallState.NEW || mState == CallState.PRE_DIAL_WAIT ||

                mState == CallState.CONNECTING) {

            handleCreateConnectionCancelled();

            handleCreateConnectionFailure(DisconnectCause.LOCAL, null);

        } else {

            Log.v(this, "Cannot abort a call which isn't either PRE_DIAL_WAIT or CONNECTING");

        }

src/com/android/telecomm/CallsManager.java

+0 −6
Original line number Diff line number Diff line
@@ -157,12 +157,6 @@ public final class CallsManager extends Call.ListenerBase { 
        markCallAsDisconnected(call, errorCode, errorMsg);

    }



    @Override

    public void onCancelledOutgoingCall(Call call) {

        Log.v(this, "onCancelledOutgoingCall, call: %s", call);

        markCallAsDisconnected(call, DisconnectCause.OUTGOING_CANCELED, null);

    }



    @Override

    public void onSuccessfulIncomingCall(Call call) {

        Log.d(this, "onSuccessfulIncomingCall");

src/com/android/telecomm/ConnectionServiceWrapper.java

+63 −99
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.os.IBinder; 
import android.os.Message;

import android.os.RemoteException;

import android.telecomm.AudioState;

import android.telecomm.Connection;

import android.telecomm.ConnectionRequest;

import android.telecomm.ConnectionService;

import android.telecomm.GatewayInfo;
@@ -46,7 +47,6 @@ import com.google.common.base.Preconditions; 
import java.util.ArrayList;

import java.util.Collections;

import java.util.HashMap;

import java.util.HashSet;

import java.util.List;

import java.util.Map;

import java.util.Set;
@@ -59,77 +59,40 @@ import java.util.concurrent.ConcurrentHashMap; 
 * {@link IConnectionService}.

 */

final class ConnectionServiceWrapper extends ServiceBinder<IConnectionService> {

    private static final int MSG_HANDLE_CREATE_CONNECTION_SUCCESSFUL = 1;

    private static final int MSG_HANDLE_CREATE_CONNECTION_FAILED = 2;

    private static final int MSG_HANDLE_CREATE_CONNECTION_CANCELLED = 3;

    private static final int MSG_SET_ACTIVE = 4;

    private static final int MSG_SET_RINGING = 5;

    private static final int MSG_SET_DIALING = 6;

    private static final int MSG_SET_DISCONNECTED = 7;

    private static final int MSG_SET_ON_HOLD = 8;

    private static final int MSG_SET_REQUESTING_RINGBACK = 9;

    private static final int MSG_SET_CALL_CAPABILITIES = 10;

    private static final int MSG_SET_IS_CONFERENCED = 11;

    private static final int MSG_ADD_CONFERENCE_CALL = 12;

    private static final int MSG_REMOVE_CALL = 13;

    private static final int MSG_ON_POST_DIAL_WAIT = 14;

    private static final int MSG_QUERY_REMOTE_CALL_SERVICES = 15;

    private static final int MSG_SET_VIDEO_PROVIDER = 16;

    private static final int MSG_SET_AUDIO_MODE_IS_VOIP = 17;

    private static final int MSG_SET_STATUS_HINTS = 18;

    private static final int MSG_SET_HANDLE = 19;

    private static final int MSG_SET_CALLER_DISPLAY_NAME = 20;

    private static final int MSG_SET_VIDEO_STATE = 21;

    private static final int MSG_SET_CONFERENCEABLE_CONNECTIONS = 22;

    private static final int MSG_START_ACTIVITY_FROM_IN_CALL = 23;

    private static final int MSG_HANDLE_CREATE_CONNECTION_COMPLETE = 1;

    private static final int MSG_SET_ACTIVE = 2;

    private static final int MSG_SET_RINGING = 3;

    private static final int MSG_SET_DIALING = 4;

    private static final int MSG_SET_DISCONNECTED = 5;

    private static final int MSG_SET_ON_HOLD = 6;

    private static final int MSG_SET_REQUESTING_RINGBACK = 7;

    private static final int MSG_SET_CALL_CAPABILITIES = 8;

    private static final int MSG_SET_IS_CONFERENCED = 9;

    private static final int MSG_ADD_CONFERENCE_CALL = 10;

    private static final int MSG_REMOVE_CALL = 11;

    private static final int MSG_ON_POST_DIAL_WAIT = 12;

    private static final int MSG_QUERY_REMOTE_CALL_SERVICES = 13;

    private static final int MSG_SET_VIDEO_PROVIDER = 14;

    private static final int MSG_SET_AUDIO_MODE_IS_VOIP = 15;

    private static final int MSG_SET_STATUS_HINTS = 16;

    private static final int MSG_SET_HANDLE = 17;

    private static final int MSG_SET_CALLER_DISPLAY_NAME = 18;

    private static final int MSG_SET_VIDEO_STATE = 19;

    private static final int MSG_SET_CONFERENCEABLE_CONNECTIONS = 20;

    private static final int MSG_START_ACTIVITY_FROM_IN_CALL = 21;



    private final Handler mHandler = new Handler() {

        @Override

        public void handleMessage(Message msg) {

            Call call;

            switch (msg.what) {

                case MSG_HANDLE_CREATE_CONNECTION_SUCCESSFUL: {

                case MSG_HANDLE_CREATE_CONNECTION_COMPLETE: {

                    SomeArgs args = (SomeArgs) msg.obj;

                    try {

                        String callId = (String) args.arg1;

                        ConnectionRequest request = (ConnectionRequest) args.arg2;

                        if (mPendingResponses.containsKey(callId)) {

                        ParcelableConnection connection = (ParcelableConnection) args.arg3;

                            mPendingResponses.remove(callId).

                                    handleCreateConnectionSuccessful(request, connection);

                        }

                    } finally {

                        args.recycle();

                    }

                    break;

                }

                case MSG_HANDLE_CREATE_CONNECTION_FAILED: {

                    SomeArgs args = (SomeArgs) msg.obj;

                    try {

                        String callId = (String) args.arg1;

                        ConnectionRequest request = (ConnectionRequest) args.arg2;

                        int statusCode = args.argi1;

                        String statusMsg = (String) args.arg3;

                        removeCall(

                                mCallIdMapper.getCall(callId),

                                statusCode,

                                statusMsg);

                    } finally {

                        args.recycle();

                    }

                    break;

                }

                case MSG_HANDLE_CREATE_CONNECTION_CANCELLED: {

                    SomeArgs args = (SomeArgs) msg.obj;

                    try {

                        String callId = (String) args.arg1;

                        ConnectionRequest request = (ConnectionRequest) args.arg2;

                        if (mPendingResponses.containsKey(callId)) {

                            mPendingResponses.remove(callId)

                                    .handleCreateConnectionCancelled();

                        } else {

                            //Log.w(this, "handleCreateConnectionCancelled, unknown call: %s", callId);

                        }

                        handleCreateConnectionComplete(callId, request, connection);

                    } finally {

                        args.recycle();

                    }
@@ -391,51 +354,21 @@ final class ConnectionServiceWrapper extends ServiceBinder<IConnectionService> { 
    private final class Adapter extends IConnectionServiceAdapter.Stub {



        @Override

        public void handleCreateConnectionSuccessful(

        public void handleCreateConnectionComplete(

                String callId,

                ConnectionRequest request,

                ParcelableConnection connection) {

            logIncoming("handleCreateConnectionSuccessful %s", request);

            logIncoming("handleCreateConnectionComplete %s", request);

            if (mCallIdMapper.isValidCallId(callId)) {

                SomeArgs args = SomeArgs.obtain();

                args.arg1 = callId;

                args.arg2 = request;

                args.arg3 = connection;

                mHandler.obtainMessage(MSG_HANDLE_CREATE_CONNECTION_SUCCESSFUL, args)

                mHandler.obtainMessage(MSG_HANDLE_CREATE_CONNECTION_COMPLETE, args)

                        .sendToTarget();

            }

        }



        @Override

        public void handleCreateConnectionFailed(

                String callId,

                ConnectionRequest request,

                int errorCode,

                String errorMsg) {

            logIncoming("handleCreateConnectionFailed %s %d %s", request, errorCode, errorMsg);

            if (mCallIdMapper.isValidCallId(callId)) {

                SomeArgs args = SomeArgs.obtain();

                args.arg1 = callId;

                args.arg2 = request;

                args.argi1 = errorCode;

                args.arg3 = errorMsg;

                mHandler.obtainMessage(MSG_HANDLE_CREATE_CONNECTION_FAILED, args).sendToTarget();

            }

        }



        @Override

        public void handleCreateConnectionCancelled(

                String callId,

                ConnectionRequest request) {

            logIncoming("handleCreateConnectionCancelled %s", request);

            if (mCallIdMapper.isValidCallId(callId)) {

                SomeArgs args = SomeArgs.obtain();

                args.arg1 = callId;

                args.arg2 = request;

                mHandler.obtainMessage(MSG_HANDLE_CREATE_CONNECTION_CANCELLED, args).sendToTarget();

            }

        }



        @Override

        public void setActive(String callId) {

            logIncoming("setActive %s", callId);
@@ -720,7 +653,7 @@ final class ConnectionServiceWrapper extends ServiceBinder<IConnectionService> { 
                            call.isIncoming());

                } catch (RemoteException e) {

                    Log.e(this, e, "Failure to createConnection -- %s", getComponentName());

                    mPendingResponses.remove(callId).handleCreateConnectionFailed(

                    mPendingResponses.remove(callId).handleCreateConnectionFailure(

                            DisconnectCause.OUTGOING_FAILURE, e.toString());

                }

            }
@@ -728,7 +661,7 @@ final class ConnectionServiceWrapper extends ServiceBinder<IConnectionService> { 
            @Override

            public void onFailure() {

                Log.e(this, new Exception(), "Failure to call %s", getComponentName());

                response.handleCreateConnectionFailed(DisconnectCause.OUTGOING_FAILURE, null);

                response.handleCreateConnectionFailure(DisconnectCause.OUTGOING_FAILURE, null);

            }

        };
@@ -866,10 +799,19 @@ final class ConnectionServiceWrapper extends ServiceBinder<IConnectionService> { 
        removeCall(call, DisconnectCause.ERROR_UNSPECIFIED, null);

    }



    void removeCall(String callId, int disconnectCause, String disconnectMessage) {

        CreateConnectionResponse response = mPendingResponses.remove(callId);

        if (response != null) {

            response.handleCreateConnectionFailure(disconnectCause, disconnectMessage);

        }



        mCallIdMapper.removeCall(callId);

    }



    void removeCall(Call call, int disconnectCause, String disconnectMessage) {

        CreateConnectionResponse response = mPendingResponses.remove(mCallIdMapper.getCallId(call));

        if (response != null) {

            response.handleCreateConnectionFailed(disconnectCause, disconnectMessage);

            response.handleCreateConnectionFailure(disconnectCause, disconnectMessage);

        }



        mCallIdMapper.removeCall(call);
@@ -937,6 +879,28 @@ final class ConnectionServiceWrapper extends ServiceBinder<IConnectionService> { 
        }

    }



    private void handleCreateConnectionComplete(

            String callId,

            ConnectionRequest request,

            ParcelableConnection connection) {

        // TODO: Note we are not using parameter "request", which is a side effect of our tacit

        // assumption that we have at most one outgoing connection attempt per ConnectionService.

        // This may not continue to be the case.

        if (connection.getState() == Connection.STATE_DISCONNECTED) {

            // A connection that begins in the DISCONNECTED state is an indication of

            // failure to connect; we handle all failures uniformly

            removeCall(

                    callId,

                    connection.getFailureCode(),

                    connection.getFailureMessage());

        } else {

            // Successful connection

            if (mPendingResponses.containsKey(callId)) {

                mPendingResponses.remove(callId).handleCreateConnectionSuccess(connection);

            }

        }

    }



    /**

     * Called when the associated connection service dies.

     */
@@ -946,7 +910,7 @@ final class ConnectionServiceWrapper extends ServiceBinder<IConnectionService> { 
                    new CreateConnectionResponse[mPendingResponses.values().size()]);

            mPendingResponses.clear();

            for (int i = 0; i < responses.length; i++) {

                responses[i].handleCreateConnectionFailed(DisconnectCause.ERROR_UNSPECIFIED, null);

                responses[i].handleCreateConnectionFailure(DisconnectCause.ERROR_UNSPECIFIED, null);

            }

        }

        mCallIdMapper.clear();

src/com/android/telecomm/CreateConnectionProcessor.java

+11 −16
Original line number Diff line number Diff line
@@ -16,7 +16,6 @@ 


package com.android.telecomm;



import android.telecomm.ConnectionRequest;

import android.telecomm.ParcelableConnection;

import android.telecomm.PhoneAccount;

import android.telecomm.PhoneAccountHandle;
@@ -102,7 +101,7 @@ final class CreateConnectionProcessor { 
            mCall.clearConnectionService();

        }

        if (response != null) {

            response.handleCreateConnectionCancelled();

            response.handleCreateConnectionFailure(DisconnectCause.OUTGOING_CANCELED, null);

        }

    }
@@ -128,7 +127,7 @@ final class CreateConnectionProcessor { 
        } else {

            Log.v(this, "attemptNextPhoneAccount, no more accounts, failing");

            if (mResponse != null) {

                mResponse.handleCreateConnectionFailed(mLastErrorCode, mLastErrorMsg);

                mResponse.handleCreateConnectionFailure(mLastErrorCode, mLastErrorMsg);

                mResponse = null;

                mCall.clearConnectionService();

            }
@@ -211,30 +210,26 @@ final class CreateConnectionProcessor { 
        }



        @Override

        public void handleCreateConnectionSuccessful(

                ConnectionRequest request, ParcelableConnection connection) {

        public void handleCreateConnectionSuccess(ParcelableConnection connection) {

            if (mResponse == null) {

                // Nobody is listening for this connection attempt any longer; ask the responsible

                // ConnectionService to tear down any resources associated with the call

                mService.abort(mCall);

            } else {

                mResponse.handleCreateConnectionSuccessful(request, connection);

                // Success -- share the good news and remember that we are no longer interested

                // in hearing about any more attempts

                mResponse.handleCreateConnectionSuccess(connection);

                mResponse = null;

            }

        }



        @Override

        public void handleCreateConnectionFailed(int code, String msg) {

        public void handleCreateConnectionFailure(int code, String msg) {

            // Failure of some sort; record the reasons for failure and try again if possible

            Log.d(CreateConnectionProcessor.this, "Connection failed: %d (%s)", code, msg);

            mLastErrorCode = code;

            mLastErrorMsg = msg;

            Log.d(CreateConnectionProcessor.this, "Connection failed: %d (%s)", code, msg);

            attemptNextPhoneAccount();

        }



        @Override

        public void handleCreateConnectionCancelled() {

            if (mResponse != null) {

                mResponse.handleCreateConnectionCancelled();

                mResponse = null;

            }

        }

    }

}

src/com/android/telecomm/CreateConnectionResponse.java

+2 −5
Original line number Diff line number Diff line
@@ -16,15 +16,12 @@ 


package com.android.telecomm;



import android.telecomm.ConnectionRequest;

import android.telecomm.ParcelableConnection;



/**

 * A callback for providing the result of creating a connection.

 */

interface CreateConnectionResponse {

    void handleCreateConnectionSuccessful(

            ConnectionRequest request, ParcelableConnection connection);

    void handleCreateConnectionFailed(int code, String msg);

    void handleCreateConnectionCancelled();

    void handleCreateConnectionSuccess(ParcelableConnection connection);

    void handleCreateConnectionFailure(int code, String message);

}