From dc98489cfdafda23e7be9065b3283ae5b8d86ac1 Mon Sep 17 00:00:00 2001
From: Jean-Michel Trivi <jmtrivi@google.com>
Date: Wed, 20 Jun 2018 10:48:05 -0700
Subject: [PATCH] NotificationPlayer: release MediaPlayer on error and
 exception

Add a lock to synchronize changes to mPlayer in the playback
  thread and callback thread.
Refactor the exception handling CreationAndCompletionThread to
  always release the player that was set up to play the sound.
In the error and completion callbacks release the media player.

Bug: 110021815
Test: play notifications with wrong URI, verify players are released.

Change-Id: Ibbd06a64c8211dff24b4cfc5960d017721eca123
---
 .../systemui/media/NotificationPlayer.java    | 53 ++++++++++++-------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/media/NotificationPlayer.java b/packages/SystemUI/src/com/android/systemui/media/NotificationPlayer.java
index f5f06db348abc..89786ee880ad5 100644
--- a/packages/SystemUI/src/com/android/systemui/media/NotificationPlayer.java
+++ b/packages/SystemUI/src/com/android/systemui/media/NotificationPlayer.java
@@ -86,11 +86,12 @@ public class NotificationPlayer implements OnCompletionListener, OnErrorListener
             // synchronized on mCompletionHandlingLock due to the Object.wait() in startSound(cmd)
             mLooper = Looper.myLooper();
             if (DEBUG) Log.d(mTag, "in run: new looper " + mLooper);
+            MediaPlayer player = null;
             synchronized(this) {
                 AudioManager audioManager =
                     (AudioManager) mCmd.context.getSystemService(Context.AUDIO_SERVICE);
                 try {
-                    MediaPlayer player = new MediaPlayer();
+                    player = new MediaPlayer();
                     if (mCmd.attributes == null) {
                         mCmd.attributes = new AudioAttributes.Builder()
                                 .setUsage(AudioAttributes.USAGE_NOTIFICATION)
@@ -137,26 +138,26 @@ public class NotificationPlayer implements OnCompletionListener, OnErrorListener
                         Log.e(mTag, "Exception while sleeping to sync notification playback"
                                 + " with ducking", e);
                     }
-                    try {
-                        player.start();
-                        if (DEBUG) { Log.d(mTag, "player.start"); }
-                    } catch (Exception e) {
+                    player.start();
+                    if (DEBUG) { Log.d(mTag, "player.start"); }
+                } catch (Exception e) {
+                    if (player != null) {
                         player.release();
                         player = null;
-                        // playing the notification didn't work, revert the focus request
-                        abandonAudioFocusAfterError();
                     }
-                    if (mPlayer != null) {
-                        if (DEBUG) { Log.d(mTag, "mPlayer.release"); }
-                        mPlayer.release();
-                    }
-                    mPlayer = player;
-                }
-                catch (Exception e) {
                     Log.w(mTag, "error loading sound for " + mCmd.uri, e);
                     // playing the notification didn't work, revert the focus request
                     abandonAudioFocusAfterError();
                 }
+                final MediaPlayer mp;
+                synchronized (mPlayerLock) {
+                    mp = mPlayer;
+                    mPlayer = player;
+                }
+                if (mp != null) {
+                    if (DEBUG) { Log.d(mTag, "mPlayer.release"); }
+                    mp.release();
+                }
                 this.notify();
             }
             Looper.loop();
@@ -230,14 +231,20 @@ public class NotificationPlayer implements OnCompletionListener, OnErrorListener
                     break;
                 case STOP:
                     if (DEBUG) Log.d(mTag, "STOP");
-                    if (mPlayer != null) {
+                    final MediaPlayer mp;
+                    synchronized (mPlayerLock) {
+                        mp = mPlayer;
+                        mPlayer = null;
+                    }
+                    if (mp != null) {
                         long delay = SystemClock.uptimeMillis() - cmd.requestTime;
                         if (delay > 1000) {
                             Log.w(mTag, "Notification stop delayed by " + delay + "msecs");
                         }
-                        mPlayer.stop();
-                        mPlayer.release();
-                        mPlayer = null;
+                        try {
+                            mp.stop();
+                        } catch (Exception e) { }
+                        mp.release();
                         synchronized(mQueueAudioFocusLock) {
                             if (mAudioManagerWithAudioFocus != null) {
                                 if (DEBUG) { Log.d(mTag, "in STOP: abandonning AudioFocus"); }
@@ -297,6 +304,14 @@ public class NotificationPlayer implements OnCompletionListener, OnErrorListener
                 }
             }
         }
+        synchronized (mPlayerLock) {
+            if (mp == mPlayer) {
+                mPlayer = null;
+            }
+        }
+        if (mp != null) {
+            mp.release();
+        }
     }
 
     public boolean onError(MediaPlayer mp, int what, int extra) {
@@ -311,6 +326,8 @@ public class NotificationPlayer implements OnCompletionListener, OnErrorListener
     @GuardedBy("mCmdQueue")
     private CmdThread mThread;
 
+    private final Object mPlayerLock = new Object();
+    @GuardedBy("mPlayerLock")
     private MediaPlayer mPlayer;