Merge "Fix concurrency issues with ringtone playback." into qt-r1-dev
diff --git a/src/com/android/server/telecom/AsyncRingtonePlayer.java b/src/com/android/server/telecom/AsyncRingtonePlayer.java
index 3745bc5..8f11882 100644
--- a/src/com/android/server/telecom/AsyncRingtonePlayer.java
+++ b/src/com/android/server/telecom/AsyncRingtonePlayer.java
@@ -26,6 +26,7 @@
import android.os.HandlerThread;
import android.os.Message;
import android.telecom.Log;
+import android.telecom.Logging.Session;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.os.SomeArgs;
@@ -107,6 +108,7 @@
args.arg2 = incomingCall;
args.arg3 = volumeShaperConfig;
args.arg4 = isVibrationEnabled;
+ args.arg5 = Log.createSubsession();
postMessage(EVENT_PLAY, true /* shouldCreateHandler */, args);
return mHapticsFuture;
}
@@ -173,73 +175,87 @@
Call incomingCall = (Call) args.arg2;
VolumeShaper.Configuration volumeShaperConfig = (VolumeShaper.Configuration) args.arg3;
boolean isVibrationEnabled = (boolean) args.arg4;
+ Session session = (Session) args.arg5;
args.recycle();
- // don't bother with any of this if there is an EVENT_STOP waiting.
- if (mHandler.hasMessages(EVENT_STOP)) {
- mHapticsFuture.complete(false /* ringtoneHasHaptics */);
- mHapticsFuture = null;
- return;
- }
- // If the Ringtone Uri is EMPTY, then the "None" Ringtone has been selected. Do not play
- // anything.
- if(Uri.EMPTY.equals(incomingCall.getRingtone())) {
- mRingtone = null;
- mHapticsFuture.complete(false /* ringtoneHasHaptics */);
- mHapticsFuture = null;
- return;
- }
-
- ThreadUtil.checkNotOnMainThread();
- Log.i(this, "handlePlay: Play ringtone.");
-
- if (mRingtone == null) {
- mRingtone = factory.getRingtone(incomingCall, volumeShaperConfig);
- if (mRingtone == null) {
- Uri ringtoneUri = incomingCall.getRingtone();
- String ringtoneUriString = (ringtoneUri == null) ? "null" :
- ringtoneUri.toSafeString();
- Log.addEvent(null, LogUtils.Events.ERROR_LOG, "Failed to get ringtone from " +
- "factory. Skipping ringing. Uri was: " + ringtoneUriString);
- mHapticsFuture.complete(false /* ringtoneHasHaptics */);
- mHapticsFuture = null;
- return;
- }
-
- // With the ringtone to play now known, we can determine if it has haptic channels or
- // not; we will complete the haptics future so the default vibration code in Ringer
- // can know whether to trigger the vibrator.
- if (mHapticsFuture != null && !mHapticsFuture.isDone()) {
- boolean hasHaptics = factory.hasHapticChannels(mRingtone);
-
- Log.i(this, "handlePlay: hasHaptics=%b, isVibrationEnabled=%b", hasHaptics,
- isVibrationEnabled);
- if (hasHaptics) {
- AudioAttributes attributes = mRingtone.getAudioAttributes();
- Log.d(this, "handlePlay: %s haptic channel",
- (isVibrationEnabled ? "unmuting" : "muting"));
- mRingtone.setAudioAttributes(
- new AudioAttributes.Builder(attributes)
- .setHapticChannelsMuted(!isVibrationEnabled)
- .build());
+ Log.continueSession(session, "ARP.hP");
+ try {
+ // don't bother with any of this if there is an EVENT_STOP waiting.
+ if (mHandler.hasMessages(EVENT_STOP)) {
+ if (mHapticsFuture != null) {
+ mHapticsFuture.complete(false /* ringtoneHasHaptics */);
+ mHapticsFuture = null;
}
- mHapticsFuture.complete(hasHaptics);
- mHapticsFuture = null;
- }
- }
-
- if (mShouldPauseBetweenRepeat) {
- // We're trying to pause between repeats, so the ringtone will not intentionally loop.
- // Instead, we'll use a handler message to perform repeats.
- handleRepeat();
- } else {
- mRingtone.setLooping(true);
- if (mRingtone.isPlaying()) {
- Log.d(this, "Ringtone already playing.");
return;
}
- mRingtone.play();
- Log.i(this, "Play ringtone, looping.");
+
+ // If the Ringtone Uri is EMPTY, then the "None" Ringtone has been selected. Do not play
+ // anything.
+ if (Uri.EMPTY.equals(incomingCall.getRingtone())) {
+ mRingtone = null;
+ if (mHapticsFuture != null) {
+ mHapticsFuture.complete(false /* ringtoneHasHaptics */);
+ mHapticsFuture = null;
+ }
+
+ return;
+ }
+
+ ThreadUtil.checkNotOnMainThread();
+ Log.i(this, "handlePlay: Play ringtone.");
+
+ if (mRingtone == null) {
+ mRingtone = factory.getRingtone(incomingCall, volumeShaperConfig);
+ if (mRingtone == null) {
+ Uri ringtoneUri = incomingCall.getRingtone();
+ String ringtoneUriString = (ringtoneUri == null) ? "null" :
+ ringtoneUri.toSafeString();
+ Log.addEvent(null, LogUtils.Events.ERROR_LOG, "Failed to get ringtone from " +
+ "factory. Skipping ringing. Uri was: " + ringtoneUriString);
+ if (mHapticsFuture != null) {
+ mHapticsFuture.complete(false /* ringtoneHasHaptics */);
+ mHapticsFuture = null;
+ }
+ return;
+ }
+
+ // With the ringtone to play now known, we can determine if it has haptic channels or
+ // not; we will complete the haptics future so the default vibration code in Ringer
+ // can know whether to trigger the vibrator.
+ if (mHapticsFuture != null && !mHapticsFuture.isDone()) {
+ boolean hasHaptics = factory.hasHapticChannels(mRingtone);
+
+ Log.i(this, "handlePlay: hasHaptics=%b, isVibrationEnabled=%b", hasHaptics,
+ isVibrationEnabled);
+ if (hasHaptics) {
+ AudioAttributes attributes = mRingtone.getAudioAttributes();
+ Log.d(this, "handlePlay: %s haptic channel",
+ (isVibrationEnabled ? "unmuting" : "muting"));
+ mRingtone.setAudioAttributes(
+ new AudioAttributes.Builder(attributes)
+ .setHapticChannelsMuted(!isVibrationEnabled)
+ .build());
+ }
+ mHapticsFuture.complete(hasHaptics);
+ mHapticsFuture = null;
+ }
+ }
+
+ if (mShouldPauseBetweenRepeat) {
+ // We're trying to pause between repeats, so the ringtone will not intentionally loop.
+ // Instead, we'll use a handler message to perform repeats.
+ handleRepeat();
+ } else {
+ mRingtone.setLooping(true);
+ if (mRingtone.isPlaying()) {
+ Log.d(this, "Ringtone already playing.");
+ return;
+ }
+ mRingtone.play();
+ Log.i(this, "Play ringtone, looping.");
+ }
+ } finally {
+ Log.cancelSubsession(session);
}
}
diff --git a/src/com/android/server/telecom/CallAudioManager.java b/src/com/android/server/telecom/CallAudioManager.java
index 7e4c3ba..3345fa4 100644
--- a/src/com/android/server/telecom/CallAudioManager.java
+++ b/src/com/android/server/telecom/CallAudioManager.java
@@ -431,35 +431,45 @@
}
void silenceRingers() {
- for (Call call : mRingingCalls) {
- call.silence();
- }
+ synchronized (mCallsManager.getLock()) {
+ for (Call call : mRingingCalls) {
+ call.silence();
+ }
- mRinger.stopRinging();
- mRinger.stopCallWaiting();
+ mRinger.stopRinging();
+ mRinger.stopCallWaiting();
+ }
}
@VisibleForTesting
public boolean startRinging() {
- return mRinger.startRinging(mForegroundCall,
- mCallAudioRouteStateMachine.isHfpDeviceAvailable());
+ synchronized (mCallsManager.getLock()) {
+ return mRinger.startRinging(mForegroundCall,
+ mCallAudioRouteStateMachine.isHfpDeviceAvailable());
+ }
}
@VisibleForTesting
public void startCallWaiting(String reason) {
- if (mRingingCalls.size() == 1) {
- mRinger.startCallWaiting(mRingingCalls.iterator().next(), reason);
+ synchronized (mCallsManager.getLock()) {
+ if (mRingingCalls.size() == 1) {
+ mRinger.startCallWaiting(mRingingCalls.iterator().next(), reason);
+ }
}
}
@VisibleForTesting
public void stopRinging() {
- mRinger.stopRinging();
+ synchronized (mCallsManager.getLock()) {
+ mRinger.stopRinging();
+ }
}
@VisibleForTesting
public void stopCallWaiting() {
- mRinger.stopCallWaiting();
+ synchronized (mCallsManager.getLock()) {
+ mRinger.stopCallWaiting();
+ }
}
@VisibleForTesting
@@ -801,10 +811,12 @@
private void maybeStopRingingAndCallWaitingForAnsweredOrRejectedCall(Call call) {
// Check to see if the call being answered/rejected is the only ringing call, since this
// will be called before the connection service acknowledges the state change.
- if (mRingingCalls.size() == 0 ||
- (mRingingCalls.size() == 1 && call == mRingingCalls.iterator().next())) {
- mRinger.stopRinging();
- mRinger.stopCallWaiting();
+ synchronized (mCallsManager.getLock()) {
+ if (mRingingCalls.size() == 0 ||
+ (mRingingCalls.size() == 1 && call == mRingingCalls.iterator().next())) {
+ mRinger.stopRinging();
+ mRinger.stopCallWaiting();
+ }
}
}
diff --git a/src/com/android/server/telecom/Ringer.java b/src/com/android/server/telecom/Ringer.java
index 2909b72..1dceb01 100644
--- a/src/com/android/server/telecom/Ringer.java
+++ b/src/com/android/server/telecom/Ringer.java
@@ -326,7 +326,7 @@
}
if (hapticsFuture != null) {
- mVibrateFuture = hapticsFuture.thenAccept(isUsingAudioCoupledHaptics -> {
+ mVibrateFuture = hapticsFuture.thenAccept(isUsingAudioCoupledHaptics -> {
if (!isUsingAudioCoupledHaptics || !mIsHapticPlaybackSupportedByDevice) {
Log.i(this, "startRinging: fileHasHaptics=%b, hapticsSupported=%b",
isUsingAudioCoupledHaptics, mIsHapticPlaybackSupportedByDevice);
diff --git a/tests/src/com/android/server/telecom/tests/CallAudioManagerTest.java b/tests/src/com/android/server/telecom/tests/CallAudioManagerTest.java
index 01add22..3f70453 100644
--- a/tests/src/com/android/server/telecom/tests/CallAudioManagerTest.java
+++ b/tests/src/com/android/server/telecom/tests/CallAudioManagerTest.java
@@ -31,6 +31,7 @@
import com.android.server.telecom.InCallTonePlayer;
import com.android.server.telecom.RingbackPlayer;
import com.android.server.telecom.Ringer;
+import com.android.server.telecom.TelecomSystem;
import com.android.server.telecom.bluetooth.BluetoothStateReceiver;
import org.junit.Before;
@@ -66,6 +67,7 @@
@Mock private RingbackPlayer mRingbackPlayer;
@Mock private DtmfLocalTonePlayer mDtmfLocalTonePlayer;
@Mock private BluetoothStateReceiver mBluetoothStateReceiver;
+ @Mock private TelecomSystem.SyncRoot mLock;
private CallAudioManager mCallAudioManager;
@@ -81,6 +83,7 @@
}).when(mockInCallTonePlayer).startTone();
return mockInCallTonePlayer;
}).when(mPlayerFactory).createPlayer(anyInt());
+ when(mCallsManager.getLock()).thenReturn(mLock);
mCallAudioManager = new CallAudioManager(
mCallAudioRouteStateMachine,
mCallsManager,
