From 54d129f9eb46a17ce7bf06206daf934de7fb0f8a Mon Sep 17 00:00:00 2001 From: Jean-Michel Trivi Date: Thu, 30 Dec 2021 17:43:47 -0800 Subject: [PATCH] Utilities for managing event listeners from AudioService 2/2 Part 2/2: class to make managing listeners even easier Add class that leverages the CallbackUtil utility methods so listener lists, locks and dispatcher stubs don't need to be members of the class using the new LazyListenerManager class. The utility class is used to refactor the management of the following listeners: Spatializer.OnSpatializerStateChangedListener Spatializer.OnHeadTrackingModeChangedListener AudioManager.OnModeChangedListener AudioManager.OnCommunicationDeviceChangedListener Bug: 206040617 Test: atest AudioModeListenerTest AudioCommunicationDeviceTest \ SpatializerTest Change-Id: Ie20c8ff2dddadc2c778c8a9ba4385f28da80e9a6 --- media/java/android/media/AudioManager.java | 86 ++++++--------------- media/java/android/media/CallbackUtil.java | 86 +++++++++++++++++++++ media/java/android/media/Spatializer.java | 88 +++++++--------------- 3 files changed, 136 insertions(+), 124 deletions(-) diff --git a/media/java/android/media/AudioManager.java b/media/java/android/media/AudioManager.java index 5c4211fdff240..85f84264df9fc 100644 --- a/media/java/android/media/AudioManager.java +++ b/media/java/android/media/AudioManager.java @@ -2994,19 +2994,17 @@ public class AudioManager { void onModeChanged(@AudioMode int mode); } - private final Object mModeListenerLock = new Object(); /** - * List of listeners for audio mode and their associated Executor. - * List is lazy-initialized on first registration + * manages the OnModeChangedListener listeners and the ModeDispatcherStub */ - @GuardedBy("mModeListenerLock") - private @Nullable ArrayList> mModeListeners; + private final CallbackUtil.LazyListenerManager mModeChangedListenerMgr = + new CallbackUtil.LazyListenerManager(); - @GuardedBy("mModeListenerLock") - private ModeDispatcherStub mModeDispatcherStub; - private final class ModeDispatcherStub extends IAudioModeDispatcher.Stub { + final class ModeDispatcherStub extends IAudioModeDispatcher.Stub + implements CallbackUtil.DispatcherStub { + @Override public void register(boolean register) { try { if (register) { @@ -3020,10 +3018,8 @@ public class AudioManager { } @Override - @SuppressLint("GuardedBy") // lock applied inside callListeners method public void dispatchAudioModeChanged(int mode) { - CallbackUtil.callListeners(mModeListeners, mModeListenerLock, - (listener) -> listener.onModeChanged(mode)); + mModeChangedListenerMgr.callListeners((listener) -> listener.onModeChanged(mode)); } } @@ -3036,15 +3032,8 @@ public class AudioManager { public void addOnModeChangedListener( @NonNull @CallbackExecutor Executor executor, @NonNull OnModeChangedListener listener) { - synchronized (mModeListenerLock) { - final Pair>, ModeDispatcherStub> res = - CallbackUtil.addListener("addOnModeChangedListener", - executor, listener, mModeListeners, mModeDispatcherStub, - () -> new ModeDispatcherStub(), - stub -> stub.register(true)); - mModeListeners = res.first; - mModeDispatcherStub = res.second; - } + mModeChangedListenerMgr.addListener(executor, listener, "addOnModeChangedListener", + () -> new ModeDispatcherStub()); } /** @@ -3053,14 +3042,7 @@ public class AudioManager { * @param listener */ public void removeOnModeChangedListener(@NonNull OnModeChangedListener listener) { - synchronized (mModeListenerLock) { - final Pair>, ModeDispatcherStub> res = - CallbackUtil.removeListener("removeOnModeChangedListener", - listener, mModeListeners, mModeDispatcherStub, - stub -> stub.register(false)); - mModeListeners = res.first; - mModeDispatcherStub = res.second; - } + mModeChangedListenerMgr.removeListener(listener, "removeOnModeChangedListener"); } /** @@ -7716,6 +7698,12 @@ public class AudioManager { void onCommunicationDeviceChanged(@Nullable AudioDeviceInfo device); } + /** + * manages the OnCommunicationDeviceChangedListener listeners and the + * CommunicationDeviceDispatcherStub + */ + private final CallbackUtil.LazyListenerManager + mCommDeviceChangedListenerMgr = new CallbackUtil.LazyListenerManager(); /** * Adds a listener for being notified of changes to the communication audio device. * See {@link #setCommunicationDevice(AudioDeviceInfo)}. @@ -7725,16 +7713,9 @@ public class AudioManager { public void addOnCommunicationDeviceChangedListener( @NonNull @CallbackExecutor Executor executor, @NonNull OnCommunicationDeviceChangedListener listener) { - synchronized (mCommDevListenerLock) { - final Pair>, - CommunicationDeviceDispatcherStub> res = - CallbackUtil.addListener("addOnCommunicationDeviceChangedListener", - executor, listener, mCommDevListeners, mCommDevDispatcherStub, - () -> new CommunicationDeviceDispatcherStub(), - stub -> stub.register(true)); - mCommDevListeners = res.first; - mCommDevDispatcherStub = res.second; - } + mCommDeviceChangedListenerMgr.addListener( + executor, listener, "addOnCommunicationDeviceChangedListener", + () -> new CommunicationDeviceDispatcherStub()); } /** @@ -7744,32 +7725,14 @@ public class AudioManager { */ public void removeOnCommunicationDeviceChangedListener( @NonNull OnCommunicationDeviceChangedListener listener) { - synchronized (mCommDevListenerLock) { - final Pair>, - CommunicationDeviceDispatcherStub> res = - CallbackUtil.removeListener("removeOnCommunicationDeviceChangedListener", - listener, mCommDevListeners, mCommDevDispatcherStub, - stub -> stub.register(false)); - mCommDevListeners = res.first; - mCommDevDispatcherStub = res.second; - } + mCommDeviceChangedListenerMgr.removeListener(listener, + "removeOnCommunicationDeviceChangedListener"); } - private final Object mCommDevListenerLock = new Object(); - /** - * List of listeners for preferred device for strategy and their associated Executor. - * List is lazy-initialized on first registration - */ - @GuardedBy("mCommDevListenerLock") - private @Nullable - ArrayList> mCommDevListeners; - - @GuardedBy("mCommDevListenerLock") - private CommunicationDeviceDispatcherStub mCommDevDispatcherStub; - private final class CommunicationDeviceDispatcherStub - extends ICommunicationDeviceDispatcher.Stub { + extends ICommunicationDeviceDispatcher.Stub implements CallbackUtil.DispatcherStub { + @Override public void register(boolean register) { try { if (register) { @@ -7783,10 +7746,9 @@ public class AudioManager { } @Override - @SuppressLint("GuardedBy") // lock applied inside callListeners method public void dispatchCommunicationDeviceChanged(int portId) { AudioDeviceInfo device = getDeviceForPortId(portId, GET_DEVICES_OUTPUTS); - CallbackUtil.callListeners(mCommDevListeners, mCommDevListenerLock, + mCommDeviceChangedListenerMgr.callListeners( (listener) -> listener.onCommunicationDeviceChanged(device)); } } diff --git a/media/java/android/media/CallbackUtil.java b/media/java/android/media/CallbackUtil.java index ac39317ed1d5f..2b5fd25c49a99 100644 --- a/media/java/android/media/CallbackUtil.java +++ b/media/java/android/media/CallbackUtil.java @@ -18,11 +18,14 @@ package android.media; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.SuppressLint; import android.media.permission.ClearCallingIdentityContext; import android.media.permission.SafeCloseable; import android.util.Log; import android.util.Pair; +import com.android.internal.annotations.GuardedBy; + import java.util.ArrayList; import java.util.Objects; import java.util.concurrent.Executor; @@ -221,4 +224,87 @@ import java.util.concurrent.Executor; } } + + /** + * Interface to be implemented by stub implementation for the events received from a server + * to the class managing the listener API. + * For an example see {@link AudioManager#ModeDispatcherStub} which registers with AudioService. + */ + interface DispatcherStub { + /** + * Register/unregister the stub as a listener of the events to be forwarded to the listeners + * managed by LazyListenerManager. + * @param register true for registering, false to unregister + */ + void register(boolean register); + } + + /** + * Class to manage a list of listeners and their callback, and the associated stub which + * receives the events to be forwarded to the listeners. + * The list of listeners and the stub and its registration are lazily initialized and registered + * @param the listener class + */ + static class LazyListenerManager { + private final Object mListenerLock = new Object(); + + @GuardedBy("mListenerLock") + private @Nullable ArrayList> mListeners; + + @GuardedBy("mListenerLock") + private @Nullable DispatcherStub mDispatcherStub; + + LazyListenerManager() { + // nothing to initialize as instances of dispatcher and list of listeners + // are lazily initialized + } + + /** + * Add a new listener / executor pair for the configured listener + * @param executor Executor for the callback + * @param listener the listener to register + * @param methodName the name of the method calling this utility method for easier to read + * exception messages + * @param newStub how to build a new instance of the stub receiving the events when the + * number of listeners goes from 0 to 1, not called until then. + */ + void addListener(@NonNull Executor executor, @NonNull T listener, String methodName, + @NonNull java.util.function.Supplier newStub) { + synchronized (mListenerLock) { + final Pair>, DispatcherStub> res = + CallbackUtil.addListener(methodName, + executor, listener, mListeners, mDispatcherStub, + newStub, + stub -> stub.register(true)); + mListeners = res.first; + mDispatcherStub = res.second; + } + } + + /** + * Remove a previously registered listener + * @param listener the listener to unregister + * @param methodName the name of the method calling this utility method for easier to read + * exception messages + */ + void removeListener(@NonNull T listener, String methodName) { + synchronized (mListenerLock) { + final Pair>, DispatcherStub> res = + CallbackUtil.removeListener(methodName, + listener, mListeners, mDispatcherStub, + stub -> stub.register(false)); + mListeners = res.first; + mDispatcherStub = res.second; + } + } + + /** + * Call the registered listeners with the given callback method + * @param callback the listener method to invoke + */ + @SuppressLint("GuardedBy") // lock applied inside callListeners method + void callListeners(CallbackMethod callback) { + CallbackUtil.callListeners(mListeners, mListenerLock, callback); + } + } } diff --git a/media/java/android/media/Spatializer.java b/media/java/android/media/Spatializer.java index c0793ec58e537..f3f8bbe22637f 100644 --- a/media/java/android/media/Spatializer.java +++ b/media/java/android/media/Spatializer.java @@ -29,7 +29,6 @@ import android.media.permission.ClearCallingIdentityContext; import android.media.permission.SafeCloseable; import android.os.RemoteException; import android.util.Log; -import android.util.Pair; import com.android.internal.annotations.GuardedBy; @@ -376,16 +375,8 @@ public class Spatializer { public void addOnSpatializerStateChangedListener( @NonNull @CallbackExecutor Executor executor, @NonNull OnSpatializerStateChangedListener listener) { - synchronized (mStateListenerLock) { - final Pair>, - SpatializerInfoDispatcherStub> res = - CallbackUtil.addListener("addOnSpatializerStateChangedListener", - executor, listener, mStateListeners, mInfoDispatcherStub, - () -> new SpatializerInfoDispatcherStub(), - stub -> stub.register(true)); - mStateListeners = res.first; - mInfoDispatcherStub = res.second; - } + mStateListenerMgr.addListener(executor, listener, "addOnSpatializerStateChangedListener", + () -> new SpatializerInfoDispatcherStub()); } /** @@ -396,15 +387,7 @@ public class Spatializer { */ public void removeOnSpatializerStateChangedListener( @NonNull OnSpatializerStateChangedListener listener) { - synchronized (mStateListenerLock) { - final Pair>, - SpatializerInfoDispatcherStub> res = - CallbackUtil.removeListener("removeOnSpatializerStateChangedListener", - listener, mStateListeners, mInfoDispatcherStub, - stub -> stub.register(false)); - mStateListeners = res.first; - mInfoDispatcherStub = res.second; - } + mStateListenerMgr.removeListener(listener, "removeOnSpatializerStateChangedListener"); } /** @@ -459,18 +442,16 @@ public class Spatializer { } } - private final Object mStateListenerLock = new Object(); /** - * List of listeners for state listener and their associated Executor. - * List is lazy-initialized on first registration + * manages the OnSpatializerStateChangedListener listeners and the + * SpatializerInfoDispatcherStub */ - @GuardedBy("mStateListenerLock") - private @Nullable ArrayList> mStateListeners; + private final CallbackUtil.LazyListenerManager + mStateListenerMgr = new CallbackUtil.LazyListenerManager(); - @GuardedBy("mStateListenerLock") - private @Nullable SpatializerInfoDispatcherStub mInfoDispatcherStub; - - private final class SpatializerInfoDispatcherStub extends ISpatializerCallback.Stub { + private final class SpatializerInfoDispatcherStub extends ISpatializerCallback.Stub + implements CallbackUtil.DispatcherStub { + @Override public void register(boolean register) { try { if (register) { @@ -486,7 +467,7 @@ public class Spatializer { @Override @SuppressLint("GuardedBy") // lock applied inside callListeners method public void dispatchSpatializerEnabledChanged(boolean enabled) { - CallbackUtil.callListeners(mStateListeners, mStateListenerLock, + mStateListenerMgr.callListeners( (listener) -> listener.onSpatializerEnabledChanged( Spatializer.this, enabled)); } @@ -494,7 +475,7 @@ public class Spatializer { @Override @SuppressLint("GuardedBy") // lock applied inside callListeners method public void dispatchSpatializerAvailableChanged(boolean available) { - CallbackUtil.callListeners(mStateListeners, mStateListenerLock, + mStateListenerMgr.callListeners( (listener) -> listener.onSpatializerAvailableChanged( Spatializer.this, available)); } @@ -612,16 +593,9 @@ public class Spatializer { public void addOnHeadTrackingModeChangedListener( @NonNull @CallbackExecutor Executor executor, @NonNull OnHeadTrackingModeChangedListener listener) { - synchronized (mHeadTrackingListenerLock) { - final Pair>, - SpatializerHeadTrackingDispatcherStub> res = CallbackUtil.addListener( - "addOnHeadTrackingModeChangedListener", executor, listener, - mHeadTrackingListeners, mHeadTrackingDispatcherStub, - () -> new SpatializerHeadTrackingDispatcherStub(), - stub -> stub.register(true)); - mHeadTrackingListeners = res.first; - mHeadTrackingDispatcherStub = res.second; - } + mHeadTrackingListenerMgr.addListener(executor, listener, + "addOnHeadTrackingModeChangedListener", + () -> new SpatializerHeadTrackingDispatcherStub()); } /** @@ -634,15 +608,8 @@ public class Spatializer { @RequiresPermission(android.Manifest.permission.MODIFY_DEFAULT_AUDIO_EFFECTS) public void removeOnHeadTrackingModeChangedListener( @NonNull OnHeadTrackingModeChangedListener listener) { - synchronized (mHeadTrackingListenerLock) { - final Pair>, - SpatializerHeadTrackingDispatcherStub> res = CallbackUtil.removeListener( - "removeOnHeadTrackingModeChangedListener", listener, - mHeadTrackingListeners, mHeadTrackingDispatcherStub, - stub -> stub.register(false)); - mHeadTrackingListeners = res.first; - mHeadTrackingDispatcherStub = res.second; - } + mHeadTrackingListenerMgr.removeListener(listener, + "removeOnHeadTrackingModeChangedListener"); } /** @@ -828,20 +795,17 @@ public class Spatializer { //----------------------------------------------------------------------------- // head tracking callback management and stub - private final Object mHeadTrackingListenerLock = new Object(); /** - * List of listeners for head tracking mode listener and their associated Executor. - * List is lazy-initialized on first registration + * manages the OnHeadTrackingModeChangedListener listeners and the + * SpatializerHeadTrackingDispatcherStub */ - @GuardedBy("mHeadTrackingListenerLock") - private @Nullable ArrayList> - mHeadTrackingListeners; - - @GuardedBy("mHeadTrackingListenerLock") - private @Nullable SpatializerHeadTrackingDispatcherStub mHeadTrackingDispatcherStub; + private final CallbackUtil.LazyListenerManager + mHeadTrackingListenerMgr = new CallbackUtil.LazyListenerManager(); private final class SpatializerHeadTrackingDispatcherStub - extends ISpatializerHeadTrackingModeCallback.Stub { + extends ISpatializerHeadTrackingModeCallback.Stub + implements CallbackUtil.DispatcherStub { + @Override public void register(boolean register) { try { if (register) { @@ -857,14 +821,14 @@ public class Spatializer { @Override @SuppressLint("GuardedBy") // lock applied inside callListeners method public void dispatchSpatializerActualHeadTrackingModeChanged(int mode) { - CallbackUtil.callListeners(mHeadTrackingListeners, mHeadTrackingListenerLock, + mHeadTrackingListenerMgr.callListeners( (listener) -> listener.onHeadTrackingModeChanged(Spatializer.this, mode)); } @Override @SuppressLint("GuardedBy") // lock applied inside callListeners method public void dispatchSpatializerDesiredHeadTrackingModeChanged(int mode) { - CallbackUtil.callListeners(mHeadTrackingListeners, mHeadTrackingListenerLock, + mHeadTrackingListenerMgr.callListeners( (listener) -> listener.onDesiredHeadTrackingModeChanged( Spatializer.this, mode)); }