Audio focus: definitive loss of focus removes client from stack

Use annotation for locking instead of javadoc comments.
Better javadoc for focus loss propagation.
When an audio focus user loses focus with AUDIOFOCUS_LOSS,
  remove it from the focus stack to prevent improper
  app behaviors (e.g. starting to play when regaining focus
  after a definitive loss).

Test: 1/ launch media playback app that uses audio focus
         and play music
      2/ launch other media app that requests AUDIOFOCUS_GAIN
         and play media
      3/ "adb shell dumpsys audio" verify first app is not
         in focus stack anymore
Change-Id: I2d4fb8ab8e554a38a650ddce440cc2f315083d08

2 files changed, 39 insertions, 28 deletions

diff --git a/services/core/java/com/android/server/audio/FocusRequester.java b/services/core/java/com/android/server/audio/FocusRequester.java
index c298fe70422d..48f0d5a11e0a 100644
--- a/services/core/java/com/android/server/audio/FocusRequester.java
+++ b/services/core/java/com/android/server/audio/FocusRequester.java
@@ -25,6 +25,7 @@ import android.media.IAudioFocusDispatcher;
 import android.os.IBinder;
 import android.util.Log;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.server.audio.MediaFocusControl.AudioFocusDeathHandler;
 
 import java.io.PrintWriter;
@@ -300,16 +301,19 @@ public class FocusRequester {
     }
 
     /**
-     * Called synchronized on MediaFocusControl.mAudioFocusLock
+     * Handle the loss of focus resulting from a given focus gain.
+     * @param focusGain the focus gain from which the loss of focus is resulting
+     * @param frWinner the new focus owner
+     * @return true if the focus loss is definitive, false otherwise.
      */
-    void handleExternalFocusGain(int focusGain, final FocusRequester fr) {
-        int focusLoss = focusLossForGainRequest(focusGain);
-        handleFocusLoss(focusLoss, fr);
+    @GuardedBy("MediaFocusControl.mAudioFocusLock")
+    boolean handleFocusLossFromGain(int focusGain, final FocusRequester frWinner) {
+        final int focusLoss = focusLossForGainRequest(focusGain);
+        handleFocusLoss(focusLoss, frWinner);
+        return (focusLoss == AudioManager.AUDIOFOCUS_LOSS);
     }
 
-    /**
-     * Called synchronized on MediaFocusControl.mAudioFocusLock
-     */
+    @GuardedBy("MediaFocusControl.mAudioFocusLock")
     void handleFocusGain(int focusGain) {
         try {
             mFocusLossReceived = AudioManager.AUDIOFOCUS_NONE;
@@ -331,19 +335,15 @@ public class FocusRequester {
         }
     }
 
-    /**
-     * Called synchronized on MediaFocusControl.mAudioFocusLock
-     */
+    @GuardedBy("MediaFocusControl.mAudioFocusLock")
     void handleFocusGainFromRequest(int focusRequestResult) {
         if (focusRequestResult == AudioManager.AUDIOFOCUS_REQUEST_GRANTED) {
             mFocusController.unduckPlayers(this);
         }
     }
 
-    /**
-     * Called synchronized on MediaFocusControl.mAudioFocusLock
-     */
-    void handleFocusLoss(int focusLoss, @Nullable final FocusRequester fr) {
+    @GuardedBy("MediaFocusControl.mAudioFocusLock")
+    void handleFocusLoss(int focusLoss, @Nullable final FocusRequester frWinner) {
         try {
             if (focusLoss != mFocusLossReceived) {
                 mFocusLossReceived = focusLoss;
@@ -371,9 +371,9 @@ public class FocusRequester {
                 boolean handled = false;
                 if (focusLoss == AudioManager.AUDIOFOCUS_LOSS_TRANSIENT_CAN_DUCK
                         && MediaFocusControl.ENFORCE_DUCKING
-                        && fr != null) {
+                        && frWinner != null) {
                     // candidate for enforcement by the framework
-                    if (fr.mCallingUid != this.mCallingUid) {
+                    if (frWinner.mCallingUid != this.mCallingUid) {
                         if ((mGrantFlags
                                 & AudioManager.AUDIOFOCUS_FLAG_PAUSES_ON_DUCKABLE_LOSS) != 0) {
                             // the focus loser declared it would pause instead of duck, let it
@@ -386,7 +386,7 @@ public class FocusRequester {
                             handled = false;
                             Log.v(TAG, "not ducking uid " + this.mCallingUid + " - old SDK");
                         } else {
-                            handled = mFocusController.duckPlayers(fr, this);
+                            handled = mFocusController.duckPlayers(frWinner, this);
                         }
                     } // else: the focus change is within the same app, so let the dispatching
                       //       happen as if the framework was not involved.
diff --git a/services/core/java/com/android/server/audio/MediaFocusControl.java b/services/core/java/com/android/server/audio/MediaFocusControl.java
index c5f563c7a43c..de58b59a2423 100644
--- a/services/core/java/com/android/server/audio/MediaFocusControl.java
+++ b/services/core/java/com/android/server/audio/MediaFocusControl.java
@@ -32,11 +32,15 @@ import android.os.IBinder;
 import android.os.RemoteException;
 import android.util.Log;
 
+import com.android.internal.annotations.GuardedBy;
+
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Map.Entry;
 import java.util.Set;
 import java.util.Stack;
@@ -146,9 +150,7 @@ public class MediaFocusControl implements PlayerFocusEnforcer {
         }
     }
 
-    /**
-     * Called synchronized on mAudioFocusLock
-     */
+    @GuardedBy("mAudioFocusLock")
     private void notifyTopOfAudioFocusStack() {
         // notify the top of the stack it gained focus
         if (!mFocusStack.empty()) {
@@ -160,14 +162,24 @@ public class MediaFocusControl implements PlayerFocusEnforcer {
 
     /**
      * Focus is requested, propagate the associated loss throughout the stack.
+     * Will also remove entries in the stack that have just received a definitive loss of focus.
      * @param focusGain the new focus gain that will later be added at the top of the stack
      */
+    @GuardedBy("mAudioFocusLock")
     private void propagateFocusLossFromGain_syncAf(int focusGain, final FocusRequester fr) {
+        final List<String> clientsToRemove = new LinkedList<String>();
         // going through the audio focus stack to signal new focus, traversing order doesn't
         // matter as all entries respond to the same external focus gain
-        Iterator<FocusRequester> stackIterator = mFocusStack.iterator();
-        while(stackIterator.hasNext()) {
-            stackIterator.next().handleExternalFocusGain(focusGain, fr);
+        for (FocusRequester focusLoser : mFocusStack) {
+            final boolean isDefinitiveLoss =
+                    focusLoser.handleFocusLossFromGain(focusGain, fr);
+            if (isDefinitiveLoss) {
+                clientsToRemove.add(focusLoser.getClientId());
+            }
+        }
+        for (String clientToRemove : clientsToRemove) {
+            removeFocusStackEntry(clientToRemove, false /*signal*/,
+                    true /*notifyFocusFollowers*/);
         }
     }
 
@@ -198,13 +210,12 @@ public class MediaFocusControl implements PlayerFocusEnforcer {
     }
 
     /**
-     * Helper function:
-     * Called synchronized on mAudioFocusLock
      * Remove a focus listener from the focus stack.
      * @param clientToRemove the focus listener
      * @param signal if true and the listener was at the top of the focus stack, i.e. it was holding
      *   focus, notify the next item in the stack it gained focus.
      */
+    @GuardedBy("mAudioFocusLock")
     private void removeFocusStackEntry(String clientToRemove, boolean signal,
             boolean notifyFocusFollowers) {
         // is the current top of the focus stack abandoning focus? (because of request, not death)
@@ -242,10 +253,9 @@ public class MediaFocusControl implements PlayerFocusEnforcer {
     }
 
     /**
-     * Helper function:
-     * Called synchronized on mAudioFocusLock
      * Remove focus listeners from the focus stack for a particular client when it has died.
      */
+    @GuardedBy("mAudioFocusLock")
     private void removeFocusStackEntryOnDeath(IBinder cb) {
         // is the owner of the audio focus part of the client to remove?
         boolean isTopOfStackForClientToRemove = !mFocusStack.isEmpty() &&
@@ -271,10 +281,10 @@ public class MediaFocusControl implements PlayerFocusEnforcer {
 
     /**
      * Helper function for external focus policy:
-     * Called synchronized on mAudioFocusLock
      * Remove focus listeners from the list of potential focus owners for a particular client when
      * it has died.
      */
+    @GuardedBy("mAudioFocusLock")
     private void removeFocusEntryForExtPolicy(IBinder cb) {
         if (mFocusOwnersForFocusPolicy.isEmpty()) {
             return;
@@ -324,6 +334,7 @@ public class MediaFocusControl implements PlayerFocusEnforcer {
      * @return {@link AudioManager#AUDIOFOCUS_REQUEST_GRANTED} or
      *     {@link AudioManager#AUDIOFOCUS_REQUEST_DELAYED}
      */
+    @GuardedBy("mAudioFocusLock")
     private int pushBelowLockedFocusOwners(FocusRequester nfr) {
         int lastLockedFocusOwnerIndex = mFocusStack.size();
         for (int index = mFocusStack.size()-1; index >= 0; index--) {