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
diff --git a/src/com/android/telecomm/Call.java b/src/com/android/telecomm/Call.java
index 7d694de..a1e9cea 100644
--- a/src/com/android/telecomm/Call.java
+++ b/src/com/android/telecomm/Call.java
@@ -26,7 +26,6 @@
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 @@
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 @@
@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 @@
}
@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,13 +641,13 @@
}
@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);
+ clearConnectionService();
+ setDisconnectCause(code, msg);
+ CallsManager.getInstance().markCallAsDisconnected(this, code, msg);
+ if (mIsIncoming) {
for (Listener listener : mListeners) {
listener.onFailedIncomingCall(this);
}
@@ -660,26 +655,6 @@
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);
- }
- clearConnectionService();
}
}
@@ -735,7 +710,7 @@
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");
}
diff --git a/src/com/android/telecomm/CallsManager.java b/src/com/android/telecomm/CallsManager.java
index d13d140..75ad17b 100644
--- a/src/com/android/telecomm/CallsManager.java
+++ b/src/com/android/telecomm/CallsManager.java
@@ -158,12 +158,6 @@
}
@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");
setCallState(call, CallState.RINGING);
diff --git a/src/com/android/telecomm/ConnectionServiceWrapper.java b/src/com/android/telecomm/ConnectionServiceWrapper.java
index 21f03e9..f535267 100644
--- a/src/com/android/telecomm/ConnectionServiceWrapper.java
+++ b/src/com/android/telecomm/ConnectionServiceWrapper.java
@@ -25,6 +25,7 @@
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 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 @@
* {@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);
- }
+ ParcelableConnection connection = (ParcelableConnection) args.arg3;
+ handleCreateConnectionComplete(callId, request, connection);
} finally {
args.recycle();
}
@@ -391,52 +354,22 @@
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);
if (mCallIdMapper.isValidCallId(callId) || mCallIdMapper.isValidConferenceId(callId)) {
@@ -720,7 +653,7 @@
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 @@
@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 @@
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 @@
}
}
+ 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 @@
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();
diff --git a/src/com/android/telecomm/CreateConnectionProcessor.java b/src/com/android/telecomm/CreateConnectionProcessor.java
index cc7808e..3e6e544 100644
--- a/src/com/android/telecomm/CreateConnectionProcessor.java
+++ b/src/com/android/telecomm/CreateConnectionProcessor.java
@@ -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 @@
mCall.clearConnectionService();
}
if (response != null) {
- response.handleCreateConnectionCancelled();
+ response.handleCreateConnectionFailure(DisconnectCause.OUTGOING_CANCELED, null);
}
}
@@ -128,7 +127,7 @@
} 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 @@
}
@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;
- }
- }
}
}
diff --git a/src/com/android/telecomm/CreateConnectionResponse.java b/src/com/android/telecomm/CreateConnectionResponse.java
index 380f42c..b907e3a 100644
--- a/src/com/android/telecomm/CreateConnectionResponse.java
+++ b/src/com/android/telecomm/CreateConnectionResponse.java
@@ -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);
}