replace the Handler with an Executor in AsyncRingtonePlayer (82725b30) · Commits · e / os / android_packages_services_Telecomm

src/com/android/server/telecom/AsyncRingtonePlayer.java

+173 −12

Original line number	Diff line number	Diff line
		@@ -32,9 +32,12 @@ import android.util.Pair;
		import com.android.internal.annotations.VisibleForTesting;
		import com.android.internal.os.SomeArgs;
		import com.android.internal.util.Preconditions;
		import com.android.server.telecom.flags.FeatureFlags;

		import java.util.ArrayList;
		import java.util.concurrent.CountDownLatch;
		import java.util.concurrent.ExecutorService;
		import java.util.concurrent.Executors;
		import java.util.concurrent.TimeUnit;
		import java.util.function.BiConsumer;
		import java.util.function.Supplier;
		@@ -55,14 +58,22 @@ public class AsyncRingtonePlayer {
		/** Handler running on the ringtone thread. */
		private Handler mHandler;

		/**
		* The ExecutorService used for handling asynchronous ringtone operations.
		* Employing ExecutorService aligns with modern Java concurrency best practices
		* and is the recommended approach for managing asynchronous tasks, offering better
		* resource control and flexibility over direct thread management.
		*/
		private ExecutorService mExecutor;

		/** The current ringtone. Only used by the ringtone thread. */
		private Ringtone mRingtone;

		/**
		* Set to true if we are setting up to play or are currently playing. False if we are stopping
		* or have stopped playing.
		* or have stopped playing. Made volatile for thread safety.
		*/
		private boolean mIsPlaying = false;
		private volatile boolean mIsPlaying = false;

		/**
		* Set to true if BT HFP is active and audio is connected.
		@@ -75,8 +86,10 @@ public class AsyncRingtonePlayer {
		*/
		private final ArrayList<CountDownLatch> mPendingRingingLatches = new ArrayList<>();

		public AsyncRingtonePlayer() {
		// Empty
		private final FeatureFlags mFeatureFlags;

		public AsyncRingtonePlayer(FeatureFlags featureFlags) {
		mFeatureFlags = featureFlags;
		}

		/**
		@@ -85,7 +98,8 @@ public class AsyncRingtonePlayer {
		* the volume of the ringtone as it plays.
		*
		* @param ringtoneInfoSupplier The {@link Ringtone} factory.
		* @param ringtoneConsumer The {@link Ringtone} post-creation callback (to start the vibration).
		* @param ringtoneConsumer The {@link Ringtone} post-creation callback (to start the
		* vibration).
		* @param isHfpDeviceConnected True if there is a HFP BT device connected, false otherwise.
		*/
		public void play(@NonNull Supplier<Pair<Uri, Ringtone>> ringtoneInfoSupplier,
		@@ -98,14 +112,23 @@ public class AsyncRingtonePlayer {
		args.arg2 = ringtoneConsumer;
		args.arg3 = Log.createSubsession();
		args.arg4 = prepareRingingReadyLatch(isHfpDeviceConnected);

		if (mFeatureFlags.resolveHiddenDependenciesTwo()) {
		postTask(() -> handlePlayExecutor(args));
		} else {
		postMessage(EVENT_PLAY, true /* shouldCreateHandler */, args);
		}
		}

		/** Stops playing the ringtone. */
		public void stop() {
		Log.d(this, "Posting stop.");
		mIsPlaying = false;
		if (mFeatureFlags.resolveHiddenDependenciesTwo()) {
		postTask(this::handleStopExecutor);
		} else {
		postMessage(EVENT_STOP, false /* shouldCreateHandler */, null);
		}
		// Clear any pending ringing latches so that we do not have to wait for its timeout to pass
		// before calling stop.
		clearPendingRingingLatches();
		@@ -113,6 +136,7 @@ public class AsyncRingtonePlayer {

		/**
		* Called when the BT HFP profile active state changes.
		*
		* @param isBtActive A BT device is connected and audio is active.
		*/
		public void updateBtActiveState(boolean isBtActive) {
		@@ -126,6 +150,7 @@ public class AsyncRingtonePlayer {
		/**
		* Prepares a new ringing ready latch and tracks it in a list. Once the ready latch has been
		* used, {@link #removePendingRingingReadyLatch(CountDownLatch)} must be called on this latch.
		*
		* @param isHfpDeviceConnected true if there is a HFP device connected.
		* @return the newly prepared CountDownLatch
		*/
		@@ -146,6 +171,7 @@ public class AsyncRingtonePlayer {

		/**
		* Remove a ringing ready latch that has been used and is no longer pending.
		*
		* @param l The latch to remove.
		*/
		private void removePendingRingingReadyLatch(CountDownLatch l) {
		@@ -328,4 +354,139 @@ public class AsyncRingtonePlayer {
		}
		mRingtone = ringtone;
		}

		/**
		* Submits a task to the executor. Creates the executor if it doesn't exist or is terminated.
		* (New method for Executor logic)
		*/
		private void postTask(Runnable task) {
		synchronized (this) {
		if (mExecutor == null \|\| mExecutor.isShutdown() \|\| mExecutor.isTerminated()) {
		Log.v(this, "Creating new single thread executor.");
		mExecutor = Executors.newSingleThreadExecutor();
		}
		mExecutor.execute(task);
		}
		}

		/**
		* Starts the actual playback of the ringtone using Executor logic.
		* Executes on the executor thread.
		*/
		@SuppressWarnings("unchecked")
		private void handlePlayExecutor(SomeArgs args) {
		Supplier<Pair<Uri, Ringtone>> ringtoneInfoSupplier =
		(Supplier<Pair<Uri, Ringtone>>) args.arg1;
		BiConsumer<Pair<Uri, Ringtone>, Boolean> ringtoneConsumer =
		(BiConsumer<Pair<Uri, Ringtone>, Boolean>) args.arg2;
		Session session = (Session) args.arg3;
		CountDownLatch ringingReadyLatch = (CountDownLatch) args.arg4;

		Log.continueSession(session, "ARP.hPE");
		try {
		// Check if stop() was called (mIsPlaying becomes false)
		if (!mIsPlaying) {
		Log.i(this,
		"handlePlay: skipping play early (path 1) as mIsPlaying "
		+ "is false.");
		removePendingRingingReadyLatch(ringingReadyLatch);
		ringtoneConsumer.accept(null, /* stopped= */ true);
		return;
		}

		Ringtone localRingtoneInstance = null;
		Uri localRingtoneUri = null;
		boolean stoppedMidProcessing = false;

		try {
		try {
		Log.i(this, "handlePlay: delay ring for ready signal...");
		if (ringingReadyLatch != null) {
		boolean reachedZero = ringingReadyLatch.await(PLAY_DELAY_TIMEOUT_MS,
		TimeUnit.MILLISECONDS);
		Log.i(this, "handlePlay: ringing ready, timeout="
		+ !reachedZero);
		}
		} catch (InterruptedException e) {
		Log.w(this, "handlePlay: latch exception: " + e);
		}

		if (ringtoneInfoSupplier != null) {
		Pair<Uri, Ringtone> info = ringtoneInfoSupplier.get();
		if (info != null) {
		localRingtoneUri = info.first;
		localRingtoneInstance = info.second;
		}
		}

		if (!mIsPlaying) {
		Log.i(this,
		"handlePlay: skipping play (path 2) after ringtone "
		+ "supply as mIsPlaying is false.");
		stoppedMidProcessing = true;
		if (localRingtoneInstance != null) {
		localRingtoneInstance.stop();
		}
		return;
		}
		// setRingtone even if null - it also stops any current ringtone to be consistent
		// with the overall state.
		setRingtone(localRingtoneInstance);
		if (mRingtone == null) {
		// The ringtoneConsumer can still vibrate at this stage.
		Log.w(this, "No ringtone was found bail out from playing.");
		return;
		}

		String uriString = localRingtoneUri != null ? localRingtoneUri.toSafeString() : "";
		Log.i(this, "handlePlay: Play ringtone. Uri: " + uriString);
		mRingtone.setLooping(true);
		if (mRingtone.isPlaying()) {
		Log.d(this, "Ringtone already playing (mRingtone).");
		return;
		}
		mRingtone.play();
		Log.i(this, "Play ringtone, looping (mRingtone).");

		} finally {
		removePendingRingingReadyLatch(ringingReadyLatch);
		ringtoneConsumer.accept(new Pair<>(localRingtoneUri, localRingtoneInstance),
		stoppedMidProcessing);
		}
		} finally {
		Log.cancelSubsession(session);
		if (args != null) {
		args.recycle();
		}
		}
		}

		/**
		* Stops the playback of the ringtone using Executor logic.
		* Executes on the executor thread.
		*/
		private void handleStopExecutor() {
		ThreadUtil.checkNotOnMainThread();
		Log.i(this, "[ExecutorLogic] Stop ringtone task executing.");
		setRingtone(null);

		synchronized (this) {
		// If mIsPlaying is true here, it means a play() was called after the stop()
		// that initiated this StopRunnable, but before this StopRunnable executed.
		// In this case, we shouldn't shut down the executor.
		if (!mIsPlaying) { // If still in a "globally" stopped state
		if (mExecutor != null && !mExecutor.isShutdown()) {
		Log.v(this, "[ExecutorLogic] Executor shutting down.");
		mExecutor.shutdown(); // Allows submitted tasks to complete
		// Set to null so it can be recreated by postTask if needed again.
		mExecutor = null;
		Log.v(this, "[ExecutorLogic] Executor cleared after shutdown.");
		}
		} else {
		Log.v(this,
		"[ExecutorLogic] Executor kept alive because mIsPlaying is" +
		" true (new play likely enqueued).");
		}
		}
		}
		}

src/com/android/server/telecom/components/TelecomService.java

+1 −1

Original line number	Diff line number	Diff line
		@@ -249,7 +249,7 @@ public class TelecomService extends Service implements TelecomSystem.Component {
		},
		ConnectionServiceFocusManager::new,
		new Timeouts.Adapter(),
		new AsyncRingtonePlayer(),
		new AsyncRingtonePlayer(featureFlags),
		new PhoneNumberUtilsAdapterImpl(),
		new IncomingCallNotifier(context, featureFlags),
		ToneGenerator::new,

tests/src/com/android/server/telecom/tests/RingerTest.java

+86 −2

Original line number	Diff line number	Diff line
		@@ -99,6 +99,7 @@ import org.mockito.Spy;

		import java.time.Duration;
		import java.util.concurrent.CompletableFuture;
		import java.util.concurrent.ExecutionException;
		import java.util.concurrent.CountDownLatch;
		import java.util.concurrent.ExecutorService;
		import java.util.concurrent.RejectedExecutionException;
		@@ -141,7 +142,6 @@ public class RingerTest extends TelecomTestCase {
		@Mock Ringer.AccessibilityManagerAdapter mockAccessibilityManagerAdapter;
		@Mock private FeatureFlags mFeatureFlags;
		@Mock private AnomalyReporterAdapter mAnomalyReporterAdapter;

		@Spy Ringer.VibrationEffectProxy spyVibrationEffectProxy;

		@Mock InCallTonePlayer mockTonePlayer;
		@@ -154,7 +154,7 @@ public class RingerTest extends TelecomTestCase {

		TestLooperManager mLooperManager;
		boolean mIsHapticPlaybackSupported = true; // Note: initializeRinger() after changes.
		AsyncRingtonePlayer asyncRingtonePlayer = new AsyncRingtonePlayer();
		AsyncRingtonePlayer asyncRingtonePlayer;
		Ringer mRingerUnderTest;
		AudioManager mockAudioManager;
		CompletableFuture<Void> mRingCompletionFuture = new CompletableFuture<>();
		@@ -172,6 +172,7 @@ public class RingerTest extends TelecomTestCase {
		FeatureFlags featureFlags = new FeatureFlagsImpl();
		when(mFeatureFlags.resolveHiddenDependenciesTwo()).thenReturn(
		featureFlags.resolveHiddenDependenciesTwo());
		asyncRingtonePlayer = new AsyncRingtonePlayer(mFeatureFlags);
		doReturn(URI_VIBRATION_EFFECT).when(spyVibrationEffectProxy).get(any(), any());
		when(mockPlayerFactory.createPlayer(any(Call.class), anyInt())).thenReturn(mockTonePlayer);
		mockAudioManager = mock(AudioManager.class);
		@@ -695,6 +696,14 @@ public class RingerTest extends TelecomTestCase {
		@SmallTest
		@Test
		public void testDelayRingerForBtHfpDevices() throws Exception {
		if (mFeatureFlags.resolveHiddenDependenciesTwo()) {
		delayRingerForBtHfpDevicesExecutor();
		} else {
		delayRingerForBtHfpDevicesHandler();
		}
		}

		private void delayRingerForBtHfpDevicesHandler() throws Exception {
		acquireLooper();

		asyncRingtonePlayer.updateBtActiveState(false);
		@@ -722,9 +731,84 @@ public class RingerTest extends TelecomTestCase {
		assertFalse(mRingerUnderTest.isRinging());
		}

		private void delayRingerForBtHfpDevicesExecutor() throws Exception {
		asyncRingtonePlayer.updateBtActiveState(false);
		Ringtone mockRingtone = ensureRingtoneMocked();

		ensureRingerIsAudible();
		assertTrue(mRingerUnderTest.startRinging(mockCall1, true /* isHfpDeviceConnected */));
		assertTrue(mRingerUnderTest.isRinging());

		// For Executor, the PlayRunnable is submitted and runs up to the latch await on its own.
		// At this point, ringtone play should be delayed
		verify(mockRingtone, never()).play(); // Verify play() has NOT been called yet

		// Now, simulate BT HFP audio becoming active
		asyncRingtonePlayer.updateBtActiveState(true);
		// For Executor, the latch is released, and PlayRunnable proceeds on its own.

		// Wait for the asynchronous play operation to complete its BiConsumer callback
		try {
		mRingCompletionFuture.get(1, TimeUnit.SECONDS);
		} catch (Exception e) {
		throw new AssertionError("mRingCompletionFuture did not complete after BT" +
		" active state update in Executor path.", e);
		}

		// Verifications after play should have started
		verify(mockRingtoneFactory, atLeastOnce())
		.getRingtone(any(Call.class), nullable(VolumeShaper.Configuration.class),
		anyBoolean());
		verifyNoMoreInteractions(mockRingtoneFactory);
		verify(mockRingtone).play();

		// Now stop ringing
		mRingerUnderTest.stopRinging();

		// For Executor, the StopRunnable is posted.
		// Wait for the stop() call on the mockRingtone with a timeout.
		verify(mockRingtone, timeout(500).times(1)).stop();

		assertFalse(mRingerUnderTest.isRinging()); // Ringer state should be updated
		}

		@SmallTest
		@Test
		public void testUnblockRingerForStopCommand() throws Exception {
		if (mFeatureFlags.resolveHiddenDependenciesTwo()) {
		unblockRingerForStopCommandExecutor();
		} else {
		unblockRingerForStopCommandHandler();
		}
		}

		private void unblockRingerForStopCommandExecutor() {
		asyncRingtonePlayer.updateBtActiveState(false); // Make play wait on latch
		Ringtone mockRingtone = ensureRingtoneMocked();

		ensureRingerIsAudible();
		assertTrue(mRingerUnderTest.startRinging(mockCall1, true /* isHfpDeviceConnected */));

		mRingerUnderTest.stopRinging();
		try {
		// Wait for the play pathway in AsyncRingtonePlayer to acknowledge the stop
		// by invoking its consumer, which in turn completes this future.
		mRingCompletionFuture.get(1, TimeUnit.SECONDS);
		} catch (Exception e) {
		throw new AssertionError("mRingCompletionFuture did not complete after" +
		" stopRinging. Indicates play operation was not properly unblocked and" +
		" terminated", e);
		}

		// Verifications:
		// 1. The ringtone should never have started playing because it was stopped while waiting.
		verify(mockRingtone, never()).play();

		// 2. The Ringer itself should no longer be in a ringing state.
		assertFalse(mRingerUnderTest.isRinging());
		}

		private void unblockRingerForStopCommandHandler() {
		acquireLooper();

		asyncRingtonePlayer.updateBtActiveState(false);