Revert^2 "Simplify MediaSessionRecord callback invocations"
This reverts commit a42fc0f1a912fbd2292bae395e55c08ff4f66fb7.
Reason for revert: Originally reverted due to a bug in R8 being fixed in aosp/3049902
Change-Id: I1f8c51e79584168aae8b0b0e593b9e97754052cf
diff --git a/services/core/java/com/android/server/media/MediaSessionRecord.java b/services/core/java/com/android/server/media/MediaSessionRecord.java
index 6f8a46b..1387ba9 100644
--- a/services/core/java/com/android/server/media/MediaSessionRecord.java
+++ b/services/core/java/com/android/server/media/MediaSessionRecord.java
@@ -63,7 +63,6 @@
import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
-import android.os.DeadObjectException;
import android.os.Handler;
import android.os.IBinder;
import android.os.Looper;
@@ -85,7 +84,6 @@
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.concurrent.CopyOnWriteArrayList;
@@ -183,6 +181,9 @@
private final boolean mVolumeAdjustmentForRemoteGroupSessions;
private final Object mLock = new Object();
+ // This field is partially guarded by mLock. Writes and non-atomic iterations (for example:
+ // index-based-iterations) must be guarded by mLock. But it is safe to acquire an iterator
+ // without acquiring mLock.
private final CopyOnWriteArrayList<ISessionControllerCallbackHolder>
mControllerCallbackHolders = new CopyOnWriteArrayList<>();
@@ -770,24 +771,9 @@
}
playbackState = mPlaybackState;
}
- Collection<ISessionControllerCallbackHolder> deadCallbackHolders = null;
- for (ISessionControllerCallbackHolder holder : mControllerCallbackHolders) {
- try {
- holder.mCallback.onPlaybackStateChanged(playbackState);
- } catch (DeadObjectException e) {
- if (deadCallbackHolders == null) {
- deadCallbackHolders = new ArrayList<>();
- }
- deadCallbackHolders.add(holder);
- logCallbackException("Removing dead callback in pushPlaybackStateUpdate", holder,
- e);
- } catch (RemoteException e) {
- logCallbackException("unexpected exception in pushPlaybackStateUpdate", holder, e);
- }
- }
- if (deadCallbackHolders != null) {
- removeControllerHoldersSafely(deadCallbackHolders);
- }
+ performOnCallbackHolders(
+ "pushPlaybackStateUpdate",
+ holder -> holder.mCallback.onPlaybackStateChanged(playbackState));
}
private void pushMetadataUpdate() {
@@ -798,23 +784,8 @@
}
metadata = mMetadata;
}
- Collection<ISessionControllerCallbackHolder> deadCallbackHolders = null;
- for (ISessionControllerCallbackHolder holder : mControllerCallbackHolders) {
- try {
- holder.mCallback.onMetadataChanged(metadata);
- } catch (DeadObjectException e) {
- if (deadCallbackHolders == null) {
- deadCallbackHolders = new ArrayList<>();
- }
- deadCallbackHolders.add(holder);
- logCallbackException("Removing dead callback in pushMetadataUpdate", holder, e);
- } catch (RemoteException e) {
- logCallbackException("unexpected exception in pushMetadataUpdate", holder, e);
- }
- }
- if (deadCallbackHolders != null) {
- removeControllerHoldersSafely(deadCallbackHolders);
- }
+ performOnCallbackHolders(
+ "pushMetadataUpdate", holder -> holder.mCallback.onMetadataChanged(metadata));
}
private void pushQueueUpdate() {
@@ -825,31 +796,18 @@
}
toSend = mQueue == null ? null : new ArrayList<>(mQueue);
}
- Collection<ISessionControllerCallbackHolder> deadCallbackHolders = null;
- for (ISessionControllerCallbackHolder holder : mControllerCallbackHolders) {
- ParceledListSlice<QueueItem> parcelableQueue = null;
- if (toSend != null) {
- parcelableQueue = new ParceledListSlice<>(toSend);
- // Limit the size of initial Parcel to prevent binder buffer overflow
- // as onQueueChanged is an async binder call.
- parcelableQueue.setInlineCountLimit(1);
- }
-
- try {
- holder.mCallback.onQueueChanged(parcelableQueue);
- } catch (DeadObjectException e) {
- if (deadCallbackHolders == null) {
- deadCallbackHolders = new ArrayList<>();
- }
- deadCallbackHolders.add(holder);
- logCallbackException("Removing dead callback in pushQueueUpdate", holder, e);
- } catch (RemoteException e) {
- logCallbackException("unexpected exception in pushQueueUpdate", holder, e);
- }
- }
- if (deadCallbackHolders != null) {
- removeControllerHoldersSafely(deadCallbackHolders);
- }
+ performOnCallbackHolders(
+ "pushQueueUpdate",
+ holder -> {
+ ParceledListSlice<QueueItem> parcelableQueue = null;
+ if (toSend != null) {
+ parcelableQueue = new ParceledListSlice<>(toSend);
+ // Limit the size of initial Parcel to prevent binder buffer overflow
+ // as onQueueChanged is an async binder call.
+ parcelableQueue.setInlineCountLimit(1);
+ }
+ holder.mCallback.onQueueChanged(parcelableQueue);
+ });
}
private void pushQueueTitleUpdate() {
@@ -860,23 +818,8 @@
}
queueTitle = mQueueTitle;
}
- Collection<ISessionControllerCallbackHolder> deadCallbackHolders = null;
- for (ISessionControllerCallbackHolder holder : mControllerCallbackHolders) {
- try {
- holder.mCallback.onQueueTitleChanged(queueTitle);
- } catch (DeadObjectException e) {
- if (deadCallbackHolders == null) {
- deadCallbackHolders = new ArrayList<>();
- }
- deadCallbackHolders.add(holder);
- logCallbackException("Removing dead callback in pushQueueTitleUpdate", holder, e);
- } catch (RemoteException e) {
- logCallbackException("unexpected exception in pushQueueTitleUpdate", holder, e);
- }
- }
- if (deadCallbackHolders != null) {
- removeControllerHoldersSafely(deadCallbackHolders);
- }
+ performOnCallbackHolders(
+ "pushQueueTitleUpdate", holder -> holder.mCallback.onQueueTitleChanged(queueTitle));
}
private void pushExtrasUpdate() {
@@ -887,23 +830,8 @@
}
extras = mExtras;
}
- Collection<ISessionControllerCallbackHolder> deadCallbackHolders = null;
- for (ISessionControllerCallbackHolder holder : mControllerCallbackHolders) {
- try {
- holder.mCallback.onExtrasChanged(extras);
- } catch (DeadObjectException e) {
- if (deadCallbackHolders == null) {
- deadCallbackHolders = new ArrayList<>();
- }
- deadCallbackHolders.add(holder);
- logCallbackException("Removing dead callback in pushExtrasUpdate", holder, e);
- } catch (RemoteException e) {
- logCallbackException("unexpected exception in pushExtrasUpdate", holder, e);
- }
- }
- if (deadCallbackHolders != null) {
- removeControllerHoldersSafely(deadCallbackHolders);
- }
+ performOnCallbackHolders(
+ "pushExtrasUpdate", holder -> holder.mCallback.onExtrasChanged(extras));
}
private void pushVolumeUpdate() {
@@ -914,23 +842,8 @@
}
info = getVolumeAttributes();
}
- Collection<ISessionControllerCallbackHolder> deadCallbackHolders = null;
- for (ISessionControllerCallbackHolder holder : mControllerCallbackHolders) {
- try {
- holder.mCallback.onVolumeInfoChanged(info);
- } catch (DeadObjectException e) {
- if (deadCallbackHolders == null) {
- deadCallbackHolders = new ArrayList<>();
- }
- deadCallbackHolders.add(holder);
- logCallbackException("Removing dead callback in pushVolumeUpdate", holder, e);
- } catch (RemoteException e) {
- logCallbackException("unexpected exception in pushVolumeUpdate", holder, e);
- }
- }
- if (deadCallbackHolders != null) {
- removeControllerHoldersSafely(deadCallbackHolders);
- }
+ performOnCallbackHolders(
+ "pushVolumeUpdate", holder -> holder.mCallback.onVolumeInfoChanged(info));
}
private void pushEvent(String event, Bundle data) {
@@ -939,23 +852,7 @@
return;
}
}
- Collection<ISessionControllerCallbackHolder> deadCallbackHolders = null;
- for (ISessionControllerCallbackHolder holder : mControllerCallbackHolders) {
- try {
- holder.mCallback.onEvent(event, data);
- } catch (DeadObjectException e) {
- if (deadCallbackHolders == null) {
- deadCallbackHolders = new ArrayList<>();
- }
- deadCallbackHolders.add(holder);
- logCallbackException("Removing dead callback in pushEvent", holder, e);
- } catch (RemoteException e) {
- logCallbackException("unexpected exception in pushEvent", holder, e);
- }
- }
- if (deadCallbackHolders != null) {
- removeControllerHoldersSafely(deadCallbackHolders);
- }
+ performOnCallbackHolders("pushEvent", holder -> holder.mCallback.onEvent(event, data));
}
private void pushSessionDestroyed() {
@@ -966,20 +863,37 @@
return;
}
}
+ performOnCallbackHolders(
+ "pushSessionDestroyed",
+ holder -> {
+ holder.mCallback.asBinder().unlinkToDeath(holder.mDeathMonitor, 0);
+ holder.mCallback.onSessionDestroyed();
+ });
+ // After notifying clear all listeners
+ synchronized (mLock) {
+ mControllerCallbackHolders.clear();
+ }
+ }
+
+ private interface ControllerCallbackCall {
+
+ void performOn(ISessionControllerCallbackHolder holder) throws RemoteException;
+ }
+
+ private void performOnCallbackHolders(String operationName, ControllerCallbackCall call) {
+ ArrayList<ISessionControllerCallbackHolder> deadCallbackHolders = new ArrayList<>();
for (ISessionControllerCallbackHolder holder : mControllerCallbackHolders) {
try {
- holder.mCallback.asBinder().unlinkToDeath(holder.mDeathMonitor, 0);
- holder.mCallback.onSessionDestroyed();
- } catch (NoSuchElementException e) {
- logCallbackException("error unlinking to binder death", holder, e);
- } catch (DeadObjectException e) {
- logCallbackException("Removing dead callback in pushSessionDestroyed", holder, e);
- } catch (RemoteException e) {
- logCallbackException("unexpected exception in pushSessionDestroyed", holder, e);
+ call.performOn(holder);
+ } catch (RemoteException | NoSuchElementException exception) {
+ deadCallbackHolders.add(holder);
+ logCallbackException(
+ "Exception while executing: " + operationName, holder, exception);
}
}
- // After notifying clear all listeners
- removeControllerHoldersSafely(null);
+ synchronized (mLock) {
+ mControllerCallbackHolders.removeAll(deadCallbackHolders);
+ }
}
private PlaybackState getStateWithUpdatedPosition() {
@@ -1027,17 +941,6 @@
return -1;
}
- private void removeControllerHoldersSafely(
- Collection<ISessionControllerCallbackHolder> holders) {
- synchronized (mLock) {
- if (holders == null) {
- mControllerCallbackHolders.clear();
- } else {
- mControllerCallbackHolders.removeAll(holders);
- }
- }
- }
-
private PlaybackInfo getVolumeAttributes() {
int volumeType;
AudioAttributes attributes;