Add synchronized lock for ringback playing start/stop request to avoid
race conditions.
Bug: 233772586
Test: atest RingbackPlayerTest
Change-Id: I143081557957f961a58fd3c1e0518f6a4af738d9
Merged-In: I143081557957f961a58fd3c1e0518f6a4af738d9
diff --git a/src/com/android/server/telecom/RingbackPlayer.java b/src/com/android/server/telecom/RingbackPlayer.java
index a8af3ac..e0c6136 100644
--- a/src/com/android/server/telecom/RingbackPlayer.java
+++ b/src/com/android/server/telecom/RingbackPlayer.java
@@ -19,6 +19,7 @@
import static com.android.server.telecom.LogUtils.Events.START_RINBACK;
import static com.android.server.telecom.LogUtils.Events.STOP_RINGBACK;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.Preconditions;
import android.telecom.Log;
@@ -42,8 +43,12 @@
*/
private InCallTonePlayer mTonePlayer;
- RingbackPlayer(InCallTonePlayer.Factory playerFactory) {
+ private final Object mLock;
+
+ @VisibleForTesting
+ public RingbackPlayer(InCallTonePlayer.Factory playerFactory) {
mPlayerFactory = playerFactory;
+ mLock = new Object();
}
/**
@@ -52,25 +57,27 @@
* @param call The call for which to ringback.
*/
public void startRingbackForCall(Call call) {
- Preconditions.checkState(call.getState() == CallState.DIALING);
+ synchronized (mLock) {
+ Preconditions.checkState(call.getState() == CallState.DIALING);
- if (mCall == call) {
- Log.w(this, "Ignoring duplicate requests to ring for %s.", call);
- return;
- }
+ if (mCall == call) {
+ Log.w(this, "Ignoring duplicate requests to ring for %s.", call);
+ return;
+ }
- if (mCall != null) {
- // We only get here for the foreground call so, there's no reason why there should
- // exist a current dialing call.
- Log.wtf(this, "Ringback player thinks there are two foreground-dialing calls.");
- }
+ if (mCall != null) {
+ // We only get here for the foreground call so, there's no reason why there should
+ // exist a current dialing call.
+ Log.wtf(this, "Ringback player thinks there are two foreground-dialing calls.");
+ }
- mCall = call;
- if (mTonePlayer == null) {
- Log.i(this, "Playing the ringback tone for %s.", call);
- Log.addEvent(call, START_RINBACK);
- mTonePlayer = mPlayerFactory.createPlayer(InCallTonePlayer.TONE_RING_BACK);
- mTonePlayer.startTone();
+ mCall = call;
+ if (mTonePlayer == null) {
+ Log.i(this, "Playing the ringback tone for %s.", call);
+ Log.addEvent(call, START_RINBACK);
+ mTonePlayer = mPlayerFactory.createPlayer(InCallTonePlayer.TONE_RING_BACK);
+ mTonePlayer.startTone();
+ }
}
}
@@ -80,19 +87,27 @@
* @param call The call for which to stop ringback.
*/
public void stopRingbackForCall(Call call) {
- if (mCall == call) {
- // The foreground call is no longer dialing or is no longer the foreground call. In
- // either case, stop the ringback tone.
- mCall = null;
+ synchronized (mLock) {
+ if (mCall == call) {
+ // The foreground call is no longer dialing or is no longer the foreground call. In
+ // either case, stop the ringback tone.
+ mCall = null;
- if (mTonePlayer == null) {
- Log.w(this, "No player found to stop.");
- } else {
- Log.i(this, "Stopping the ringback tone for %s.", call);
- Log.addEvent(call, STOP_RINGBACK);
- mTonePlayer.stopTone();
- mTonePlayer = null;
+ if (mTonePlayer == null) {
+ Log.w(this, "No player found to stop.");
+ } else {
+ Log.i(this, "Stopping the ringback tone for %s.", call);
+ Log.addEvent(call, STOP_RINGBACK);
+ mTonePlayer.stopTone();
+ mTonePlayer = null;
+ }
}
}
}
+
+ public boolean isRingbackPlaying() {
+ synchronized (mLock) {
+ return mTonePlayer != null;
+ }
+ }
}
\ No newline at end of file
diff --git a/tests/src/com/android/server/telecom/tests/RingbackPlayerTest.java b/tests/src/com/android/server/telecom/tests/RingbackPlayerTest.java
new file mode 100644
index 0000000..f69d532
--- /dev/null
+++ b/tests/src/com/android/server/telecom/tests/RingbackPlayerTest.java
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.telecom.tests;
+
+import static org.junit.Assert.assertFalse;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.when;
+
+import android.test.suitebuilder.annotation.SmallTest;
+
+import com.android.server.telecom.Call;
+import com.android.server.telecom.CallState;
+import com.android.server.telecom.InCallTonePlayer;
+import com.android.server.telecom.RingbackPlayer;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mock;
+
+import java.util.concurrent.CountDownLatch;
+
+@RunWith(JUnit4.class)
+public class RingbackPlayerTest extends TelecomTestCase {
+ @Mock InCallTonePlayer.Factory mFactory;
+ @Mock Call mCall;
+ @Mock InCallTonePlayer mTonePlayer;
+
+ private RingbackPlayer mRingbackPlayer;
+
+ @Before
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ when(mFactory.createPlayer(anyInt())).thenReturn(mTonePlayer);
+ mRingbackPlayer = new RingbackPlayer(mFactory);
+ }
+
+ @SmallTest
+ @Test
+ public void testPlayerSync() {
+ // make sure InCallTonePlayer try to start playing the tone after RingbackPlayer receives
+ // stop tone request.
+ CountDownLatch latch = new CountDownLatch(1);
+ doReturn(CallState.DIALING).when(mCall).getState();
+ doAnswer(x -> {
+ new Thread(() -> {
+ try {
+ latch.wait();
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ }).start();
+ return true;
+ }).when(mTonePlayer).startTone();
+
+ mRingbackPlayer.startRingbackForCall(mCall);
+ mRingbackPlayer.stopRingbackForCall(mCall);
+ assertFalse(mRingbackPlayer.isRingbackPlaying());
+ latch.countDown();
+ }
+}