From 66b138c8d8355b65eafad04ae36b4d5986eed764 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Iv=C3=A1n=20Budnik?= <ivanbuper@google.com>
Date: Mon, 20 May 2024 19:08:45 +0100
Subject: [PATCH] Fix RemoteException handling in MediaController

As per go/android-api-guidelines#framework-rethrow-remoteexception, we
should strongly rethrow any RemoteException coming from system_server.

Bug: 205124386
Test: Presubmit.
Flag: EXEMPT bugfix: Low risk change to improve behavior in an already crashing system.
Change-Id: I1f8b3f3e201d7d2f41bb9e1ae5dd3969896b5bd7
---
 .../media/session/MediaController.java        | 153 +++++++++---------
 1 file changed, 72 insertions(+), 81 deletions(-)

diff --git a/media/java/android/media/session/MediaController.java b/media/java/android/media/session/MediaController.java
index a4887567b075..f7c87c3e1383 100644
--- a/media/java/android/media/session/MediaController.java
+++ b/media/java/android/media/session/MediaController.java
@@ -141,10 +141,9 @@ public final class MediaController {
         }
         try {
             return mSessionBinder.sendMediaButton(mContext.getPackageName(), keyEvent);
-        } catch (RemoteException e) {
-            // System is dead. =(
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
-        return false;
     }
 
     /**
@@ -155,9 +154,8 @@ public final class MediaController {
     public @Nullable PlaybackState getPlaybackState() {
         try {
             return mSessionBinder.getPlaybackState();
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling getPlaybackState.", e);
-            return null;
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
     }
 
@@ -169,9 +167,8 @@ public final class MediaController {
     public @Nullable MediaMetadata getMetadata() {
         try {
             return mSessionBinder.getMetadata();
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling getMetadata.", e);
-            return null;
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
     }
 
@@ -185,10 +182,9 @@ public final class MediaController {
         try {
             ParceledListSlice list = mSessionBinder.getQueue();
             return list == null ? null : list.getList();
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling getQueue.", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
-        return null;
     }
 
     /**
@@ -197,10 +193,9 @@ public final class MediaController {
     public @Nullable CharSequence getQueueTitle() {
         try {
             return mSessionBinder.getQueueTitle();
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling getQueueTitle", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
-        return null;
     }
 
     /**
@@ -209,10 +204,9 @@ public final class MediaController {
     public @Nullable Bundle getExtras() {
         try {
             return mSessionBinder.getExtras();
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling getExtras", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
-        return null;
     }
 
     /**
@@ -232,9 +226,8 @@ public final class MediaController {
     public int getRatingType() {
         try {
             return mSessionBinder.getRatingType();
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling getRatingType.", e);
-            return Rating.RATING_NONE;
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
     }
 
@@ -246,10 +239,9 @@ public final class MediaController {
     public long getFlags() {
         try {
             return mSessionBinder.getFlags();
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling getFlags.", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
-        return 0;
     }
 
     /** Returns the current playback info for this session. */
@@ -271,10 +263,9 @@ public final class MediaController {
     public @Nullable PendingIntent getSessionActivity() {
         try {
             return mSessionBinder.getLaunchPendingIntent();
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling getPendingIntent.", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
-        return null;
     }
 
     /**
@@ -304,8 +295,8 @@ public final class MediaController {
             //       AppOpsManager usages.
             mSessionBinder.setVolumeTo(mContext.getPackageName(), mContext.getOpPackageName(),
                     value, flags);
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling setVolumeTo.", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
     }
 
@@ -329,8 +320,8 @@ public final class MediaController {
             //       AppOpsManager usages.
             mSessionBinder.adjustVolume(mContext.getPackageName(), mContext.getOpPackageName(),
                     direction, flags);
-        } catch (RemoteException e) {
-            Log.wtf(TAG, "Error calling adjustVolumeBy.", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
     }
 
@@ -395,8 +386,8 @@ public final class MediaController {
         }
         try {
             mSessionBinder.sendCommand(mContext.getPackageName(), command, args, cb);
-        } catch (RemoteException e) {
-            Log.d(TAG, "Dead object in sendCommand.", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
     }
 
@@ -409,8 +400,8 @@ public final class MediaController {
         if (mPackageName == null) {
             try {
                 mPackageName = mSessionBinder.getPackageName();
-            } catch (RemoteException e) {
-                Log.d(TAG, "Dead object in getPackageName.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
         return mPackageName;
@@ -430,8 +421,8 @@ public final class MediaController {
         // Get info from the connected session.
         try {
             mSessionInfo = mSessionBinder.getSessionInfo();
-        } catch (RemoteException e) {
-            Log.d(TAG, "Dead object in getSessionInfo.", e);
+        } catch (RemoteException ex) {
+            throw ex.rethrowFromSystemServer();
         }
 
         if (mSessionInfo == null) {
@@ -454,8 +445,8 @@ public final class MediaController {
         if (mTag == null) {
             try {
                 mTag = mSessionBinder.getTag();
-            } catch (RemoteException e) {
-                Log.d(TAG, "Dead object in getTag.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
         return mTag;
@@ -485,8 +476,8 @@ public final class MediaController {
             try {
                 mSessionBinder.registerCallback(mContext.getPackageName(), mCbStub);
                 mCbRegistered = true;
-            } catch (RemoteException e) {
-                Log.e(TAG, "Dead object in registerCallback", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
     }
@@ -504,8 +495,8 @@ public final class MediaController {
         if (mCbRegistered && mCallbacks.size() == 0) {
             try {
                 mSessionBinder.unregisterCallback(mCbStub);
-            } catch (RemoteException e) {
-                Log.e(TAG, "Dead object in removeCallbackLocked");
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
             mCbRegistered = false;
         }
@@ -642,8 +633,8 @@ public final class MediaController {
         public void prepare() {
             try {
                 mSessionBinder.prepare(mContext.getPackageName());
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling prepare.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -666,8 +657,8 @@ public final class MediaController {
             }
             try {
                 mSessionBinder.prepareFromMediaId(mContext.getPackageName(), mediaId, extras);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling prepare(" + mediaId + ").", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -692,8 +683,8 @@ public final class MediaController {
             }
             try {
                 mSessionBinder.prepareFromSearch(mContext.getPackageName(), query, extras);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling prepare(" + query + ").", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -716,8 +707,8 @@ public final class MediaController {
             }
             try {
                 mSessionBinder.prepareFromUri(mContext.getPackageName(), uri, extras);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling prepare(" + uri + ").", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -727,8 +718,8 @@ public final class MediaController {
         public void play() {
             try {
                 mSessionBinder.play(mContext.getPackageName());
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling play.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -746,8 +737,8 @@ public final class MediaController {
             }
             try {
                 mSessionBinder.playFromMediaId(mContext.getPackageName(), mediaId, extras);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling play(" + mediaId + ").", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -768,8 +759,8 @@ public final class MediaController {
             }
             try {
                 mSessionBinder.playFromSearch(mContext.getPackageName(), query, extras);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling play(" + query + ").", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -787,8 +778,8 @@ public final class MediaController {
             }
             try {
                 mSessionBinder.playFromUri(mContext.getPackageName(), uri, extras);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling play(" + uri + ").", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -799,8 +790,8 @@ public final class MediaController {
         public void skipToQueueItem(long id) {
             try {
                 mSessionBinder.skipToQueueItem(mContext.getPackageName(), id);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling skipToItem(" + id + ").", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -811,8 +802,8 @@ public final class MediaController {
         public void pause() {
             try {
                 mSessionBinder.pause(mContext.getPackageName());
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling pause.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -823,8 +814,8 @@ public final class MediaController {
         public void stop() {
             try {
                 mSessionBinder.stop(mContext.getPackageName());
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling stop.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -836,8 +827,8 @@ public final class MediaController {
         public void seekTo(long pos) {
             try {
                 mSessionBinder.seekTo(mContext.getPackageName(), pos);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling seekTo.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -848,8 +839,8 @@ public final class MediaController {
         public void fastForward() {
             try {
                 mSessionBinder.fastForward(mContext.getPackageName());
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling fastForward.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -859,8 +850,8 @@ public final class MediaController {
         public void skipToNext() {
             try {
                 mSessionBinder.next(mContext.getPackageName());
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling next.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -871,8 +862,8 @@ public final class MediaController {
         public void rewind() {
             try {
                 mSessionBinder.rewind(mContext.getPackageName());
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling rewind.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -882,8 +873,8 @@ public final class MediaController {
         public void skipToPrevious() {
             try {
                 mSessionBinder.previous(mContext.getPackageName());
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling previous.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -897,8 +888,8 @@ public final class MediaController {
         public void setRating(Rating rating) {
             try {
                 mSessionBinder.rate(mContext.getPackageName(), rating);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling rate.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -915,8 +906,8 @@ public final class MediaController {
             }
             try {
                 mSessionBinder.setPlaybackSpeed(mContext.getPackageName(), speed);
-            } catch (RemoteException e) {
-                Log.wtf(TAG, "Error calling setPlaybackSpeed.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
 
@@ -950,8 +941,8 @@ public final class MediaController {
             }
             try {
                 mSessionBinder.sendCustomAction(mContext.getPackageName(), action, args);
-            } catch (RemoteException e) {
-                Log.d(TAG, "Dead object in sendCustomAction.", e);
+            } catch (RemoteException ex) {
+                throw ex.rethrowFromSystemServer();
             }
         }
     }
-- 
GitLab