Correct HeadsetMediaButton behavior for external calls.
There was an issue where onExternalCallChanged callbacks into
HaedsetMediaButton would not actually change the active state the
MediaSession. It turns out we were just calling onCallRemoved when a call
became external. onCallRemoved would just ignore external calls and not
change the MediaSession active status. Refactored the core logic of the
onCallAdded/onCallRemoved callbacks into separate methods and just call
these directly from onExternalCallChanged; this ensure we skip the
external call checks which were in onCallAdded/onCallRemoved.
Refactored HeadsetMediaButton code to enable abstracting out the
MediaSession for testing purposes and added a new unit test suite to
test the behavior of adding/removing/external calls.
Test: Wrote HeadsetMediaButtonTest unit test suite to verify the behavior
of media session activation/deactivation when calls are added, removed
and move between an external/non-external state.
Test: atest com.android.server.telecom.tests.HeadsetMediaButtonTest
Test: Performed manual call testing using a wired headset USB adapter;
verified that the headset media buttons can be used to mute/unmute, answer
and hangup a call.
Bug: 221492698
Change-Id: I77f6e3737b3246424e831686150cd37562c39107
diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java
index f98513e..9d6dd07 100755
--- a/src/com/android/server/telecom/CallsManager.java
+++ b/src/com/android/server/telecom/CallsManager.java
@@ -3300,7 +3300,7 @@
*
* @return {@code True} if there are any non-external calls, {@code false} otherwise.
*/
- boolean hasAnyCalls() {
+ public boolean hasAnyCalls() {
if (mCalls.isEmpty()) {
return false;
}
diff --git a/src/com/android/server/telecom/HeadsetMediaButton.java b/src/com/android/server/telecom/HeadsetMediaButton.java
index ad95c34..b1471c2 100644
--- a/src/com/android/server/telecom/HeadsetMediaButton.java
+++ b/src/com/android/server/telecom/HeadsetMediaButton.java
@@ -46,6 +46,47 @@
private static final int MSG_MEDIA_SESSION_INITIALIZE = 0;
private static final int MSG_MEDIA_SESSION_SET_ACTIVE = 1;
+ /**
+ * Wrapper class that abstracts an instance of {@link MediaSession} to the
+ * {@link MediaSessionAdapter} interface this class uses. This is done because
+ * {@link MediaSession} is a final class and cannot be mocked for testing purposes.
+ */
+ public class MediaSessionWrapper implements MediaSessionAdapter {
+ private final MediaSession mMediaSession;
+
+ public MediaSessionWrapper(MediaSession mediaSession) {
+ mMediaSession = mediaSession;
+ }
+
+ /**
+ * Sets the underlying {@link MediaSession} active status.
+ * @param active
+ */
+ @Override
+ public void setActive(boolean active) {
+ mMediaSession.setActive(active);
+ }
+
+ /**
+ * Gets the underlying {@link MediaSession} active status.
+ * @return {@code true} if active, {@code false} otherwise.
+ */
+ @Override
+ public boolean isActive() {
+ return mMediaSession.isActive();
+ }
+ }
+
+ /**
+ * Interface which defines the basic functionality of a {@link MediaSession} which is important
+ * for the {@link HeadsetMediaButton} to operator; this is for testing purposes so we can mock
+ * out that functionality.
+ */
+ public interface MediaSessionAdapter {
+ void setActive(boolean active);
+ boolean isActive();
+ }
+
private final MediaSession.Callback mSessionCallback = new MediaSession.Callback() {
@Override
public boolean onMediaButtonEvent(Intent intent) {
@@ -81,7 +122,7 @@
session.setFlags(MediaSession.FLAG_EXCLUSIVE_GLOBAL_PRIORITY
| MediaSession.FLAG_HANDLES_MEDIA_BUTTONS);
session.setPlaybackToLocal(AUDIO_ATTRIBUTES);
- mSession = session;
+ mSession = new MediaSessionWrapper(session);
break;
}
case MSG_MEDIA_SESSION_SET_ACTIVE: {
@@ -102,9 +143,37 @@
private final Context mContext;
private final CallsManager mCallsManager;
private final TelecomSystem.SyncRoot mLock;
- private MediaSession mSession;
+ private MediaSessionAdapter mSession;
private KeyEvent mLastHookEvent;
+ /**
+ * Constructor used for testing purposes to initialize a {@link HeadsetMediaButton} with a
+ * specified {@link MediaSessionAdapter}. Will not trigger MSG_MEDIA_SESSION_INITIALIZE and
+ * cause an actual {@link MediaSession} instance to be created.
+ * @param context the context
+ * @param callsManager the mock calls manager
+ * @param lock the lock
+ * @param adapter the adapter
+ */
+ @VisibleForTesting
+ public HeadsetMediaButton(
+ Context context,
+ CallsManager callsManager,
+ TelecomSystem.SyncRoot lock,
+ MediaSessionAdapter adapter) {
+ mContext = context;
+ mCallsManager = callsManager;
+ mLock = lock;
+ mSession = adapter;
+ }
+
+ /**
+ * Production code constructor; this version triggers MSG_MEDIA_SESSION_INITIALIZE which will
+ * create an actual instance of {@link MediaSession}.
+ * @param context the context
+ * @param callsManager the calls manager
+ * @param lock the telecom lock
+ */
public HeadsetMediaButton(
Context context,
CallsManager callsManager,
@@ -155,6 +224,13 @@
if (call.isExternalCall()) {
return;
}
+ handleCallAddition();
+ }
+
+ /**
+ * Triggers session activation due to call addition.
+ */
+ private void handleCallAddition() {
mMediaSessionHandler.obtainMessage(MSG_MEDIA_SESSION_SET_ACTIVE, 1, 0).sendToTarget();
}
@@ -164,6 +240,13 @@
if (call.isExternalCall()) {
return;
}
+ handleCallRemoval();
+ }
+
+ /**
+ * Triggers session deactivation due to call removal.
+ */
+ private void handleCallRemoval() {
if (!mCallsManager.hasAnyCalls()) {
mMediaSessionHandler.obtainMessage(MSG_MEDIA_SESSION_SET_ACTIVE, 0, 0).sendToTarget();
}
@@ -172,10 +255,20 @@
/** ${inheritDoc} */
@Override
public void onExternalCallChanged(Call call, boolean isExternalCall) {
+ // Note: We don't use the onCallAdded/onCallRemoved methods here since they do checks to see
+ // if the call is external or not and would skip the session activation/deactivation.
if (isExternalCall) {
- onCallRemoved(call);
+ handleCallRemoval();
} else {
- onCallAdded(call);
+ handleCallAddition();
}
}
+
+ @VisibleForTesting
+ /**
+ * @return the handler this class instance uses for operation; used for unit testing.
+ */
+ public Handler getHandler() {
+ return mMediaSessionHandler;
+ }
}
diff --git a/tests/src/com/android/server/telecom/tests/HeadsetMediaButtonTest.java b/tests/src/com/android/server/telecom/tests/HeadsetMediaButtonTest.java
new file mode 100644
index 0000000..6d15e60
--- /dev/null
+++ b/tests/src/com/android/server/telecom/tests/HeadsetMediaButtonTest.java
@@ -0,0 +1,120 @@
+/*
+ * 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 com.android.server.telecom.Call;
+import com.android.server.telecom.CallsManager;
+import com.android.server.telecom.HeadsetMediaButton;
+import com.android.server.telecom.TelecomSystem;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@RunWith(JUnit4.class)
+public class HeadsetMediaButtonTest extends TelecomTestCase {
+ private static final int TEST_TIMEOUT_MILLIS = 1000;
+
+ private HeadsetMediaButton mHeadsetMediaButton;
+
+ @Mock private CallsManager mMockCallsManager;
+ @Mock private HeadsetMediaButton.MediaSessionAdapter mMediaSessionAdapter;
+ private TelecomSystem.SyncRoot mLock = new TelecomSystem.SyncRoot() {};
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+ mHeadsetMediaButton = new HeadsetMediaButton(mContext, mMockCallsManager, mLock,
+ mMediaSessionAdapter);
+ }
+
+ @Override
+ @After
+ public void tearDown() throws Exception {
+ mHeadsetMediaButton = null;
+ super.tearDown();
+ }
+
+ /**
+ * Nominal case; just add a call and remove it.
+ */
+ @Test
+ public void testAddCall() {
+ Call regularCall = getRegularCall();
+
+ when(mMockCallsManager.hasAnyCalls()).thenReturn(true);
+ mHeadsetMediaButton.onCallAdded(regularCall);
+ waitForHandlerAction(mHeadsetMediaButton.getHandler(), TEST_TIMEOUT_MILLIS);
+ verify(mMediaSessionAdapter).setActive(eq(true));
+ // ... and thus we see how the original code isn't amenable to tests.
+ when(mMediaSessionAdapter.isActive()).thenReturn(true);
+
+ when(mMockCallsManager.hasAnyCalls()).thenReturn(false);
+ mHeadsetMediaButton.onCallRemoved(regularCall);
+ waitForHandlerAction(mHeadsetMediaButton.getHandler(), TEST_TIMEOUT_MILLIS);
+ verify(mMediaSessionAdapter).setActive(eq(false));
+ }
+
+ /**
+ * Test a case where a regular call becomes an external call, and back again.
+ */
+ @Test
+ public void testRegularCallThatBecomesExternal() {
+ Call regularCall = getRegularCall();
+
+ // Start with a regular old call.
+ when(mMockCallsManager.hasAnyCalls()).thenReturn(true);
+ mHeadsetMediaButton.onCallAdded(regularCall);
+ waitForHandlerAction(mHeadsetMediaButton.getHandler(), TEST_TIMEOUT_MILLIS);
+ verify(mMediaSessionAdapter).setActive(eq(true));
+ when(mMediaSessionAdapter.isActive()).thenReturn(true);
+
+ // Change so it is external.
+ when(regularCall.isExternalCall()).thenReturn(true);
+ when(mMockCallsManager.hasAnyCalls()).thenReturn(false);
+ mHeadsetMediaButton.onExternalCallChanged(regularCall, true);
+ // Expect to set session inactive.
+ waitForHandlerAction(mHeadsetMediaButton.getHandler(), TEST_TIMEOUT_MILLIS);
+ verify(mMediaSessionAdapter).setActive(eq(false));
+
+ // For good measure lets make it non-external again.
+ when(regularCall.isExternalCall()).thenReturn(false);
+ when(mMockCallsManager.hasAnyCalls()).thenReturn(true);
+ mHeadsetMediaButton.onExternalCallChanged(regularCall, false);
+ // Expect to set session active.
+ waitForHandlerAction(mHeadsetMediaButton.getHandler(), TEST_TIMEOUT_MILLIS);
+ verify(mMediaSessionAdapter).setActive(eq(true));
+ }
+
+ /**
+ * @return a mock call instance of a regular non-external call.
+ */
+ private Call getRegularCall() {
+ Call regularCall = Mockito.mock(Call.class);
+ when(regularCall.isExternalCall()).thenReturn(false);
+ return regularCall;
+ }
+}