Merge "Revert "Simplify MediaSessionRecord callback invocations"" into main am: 84ebcbd07c

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/3048543

Change-Id: I9a0dc661e39dd4d65f7b0d1e33a190b68b9d0232
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

1 files changed, 151 insertions, 54 deletions

diff --git a/services/core/java/com/android/server/media/MediaSessionRecord.java b/services/core/java/com/android/server/media/MediaSessionRecord.java
index 1387ba915147..6f8a46be089f 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.net.Uri;
 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 com.android.server.uri.UriGrantsManagerInternal;
 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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
     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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
             }
             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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
             }
             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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
             }
             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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
             }
             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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
             }
             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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
             }
             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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
                 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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
                 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 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR
         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;