Revert "Simplify MediaSessionRecord callback invocations"
This reverts commit b1b6f7f3495eb6efbe0c70dc3be83659e076036c.
Reason for revert: b/335809645
Change-Id: I6b0aa4d1a3b9f87598a8ba13b073b4c10cc696c3
diff --git a/services/core/java/com/android/server/media/MediaSessionRecord.java b/services/core/java/com/android/server/media/MediaSessionRecord.java
index 1387ba9..6f8a46b 100644
--- a/services/core/java/com/android/server/media/MediaSessionRecord.java
+++ b/services/core/java/com/android/server/media/MediaSessionRecord.java
@@ -63,6 +63,7 @@
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;
@@ -84,6 +85,7 @@
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;
@@ -181,9 +183,6 @@
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<>();
@@ -771,9 +770,24 @@
}
playbackState = mPlaybackState;
}
- performOnCallbackHolders(
- "pushPlaybackStateUpdate",
- holder -> holder.mCallback.onPlaybackStateChanged(playbackState));
+ 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);
+ }
}
private void pushMetadataUpdate() {
@@ -784,8 +798,23 @@
}
metadata = mMetadata;
}
- performOnCallbackHolders(
- "pushMetadataUpdate", holder -> holder.mCallback.onMetadataChanged(metadata));
+ 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);
+ }
}
private void pushQueueUpdate() {
@@ -796,18 +825,31 @@
}
toSend = mQueue == null ? null : new ArrayList<>(mQueue);
}
- 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);
- });
+ 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);
+ }
}
private void pushQueueTitleUpdate() {
@@ -818,8 +860,23 @@
}
queueTitle = mQueueTitle;
}
- performOnCallbackHolders(
- "pushQueueTitleUpdate", holder -> holder.mCallback.onQueueTitleChanged(queueTitle));
+ 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);
+ }
}
private void pushExtrasUpdate() {
@@ -830,8 +887,23 @@
}
extras = mExtras;
}
- performOnCallbackHolders(
- "pushExtrasUpdate", holder -> holder.mCallback.onExtrasChanged(extras));
+ 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);
+ }
}
private void pushVolumeUpdate() {
@@ -842,8 +914,23 @@
}
info = getVolumeAttributes();
}
- performOnCallbackHolders(
- "pushVolumeUpdate", holder -> holder.mCallback.onVolumeInfoChanged(info));
+ 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);
+ }
}
private void pushEvent(String event, Bundle data) {
@@ -852,7 +939,23 @@
return;
}
}
- performOnCallbackHolders("pushEvent", holder -> holder.mCallback.onEvent(event, data));
+ 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);
+ }
}
private void pushSessionDestroyed() {
@@ -863,37 +966,20 @@
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 {
- call.performOn(holder);
- } catch (RemoteException | NoSuchElementException exception) {
- deadCallbackHolders.add(holder);
- logCallbackException(
- "Exception while executing: " + operationName, holder, exception);
+ 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);
}
}
- synchronized (mLock) {
- mControllerCallbackHolders.removeAll(deadCallbackHolders);
- }
+ // After notifying clear all listeners
+ removeControllerHoldersSafely(null);
}
private PlaybackState getStateWithUpdatedPosition() {
@@ -941,6 +1027,17 @@
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;