[Audio Focus] Allow a session to specify a request ID

Allow a session to specify a request ID. Normally if
a session abandons focus and then requests it again
we would give it a new ID. This preserves the ID
to avoid making the UI flash.

BUG=988106

Change-Id: Iecf95bad2276908bb1d5910d3aebd17c25731f0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1721039Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684161}

content/browser/media/session/audio_focus_delegate_default.cc

@@ -55,9 +55,7 @@ class AudioFocusDelegateDefault : public AudioFocusDelegate {
 
  private:
   // Finishes an async audio focus request.
-  void FinishAudioFocusRequest(AudioFocusType type);
-  void FinishInitialAudioFocusRequest(AudioFocusType type,
-                                      const base::UnguessableToken& request_id);
+  void FinishAudioFocusRequest(AudioFocusType type, bool success);
 
   // Ensures that |audio_focus_ptr_| is connected.
   void EnsureServiceConnection();
@@ -78,6 +76,9 @@ class AudioFocusDelegateDefault : public AudioFocusDelegate {
 
   // The last requested AudioFocusType by the associated |media_session_|.
   base::Optional<AudioFocusType> audio_focus_type_;
+
+  // ID to uniquely identify the audio focus delegate.
+  base::UnguessableToken const request_id_ = base::UnguessableToken::Create();
 };
 
 }  // anonymous namespace
@@ -105,7 +106,8 @@ AudioFocusDelegateDefault::RequestAudioFocus(AudioFocusType audio_focus_type) {
     request_client_remote_->RequestAudioFocus(
         session_info_.Clone(), audio_focus_type,
         base::BindOnce(&AudioFocusDelegateDefault::FinishAudioFocusRequest,
-                       base::Unretained(this), audio_focus_type));
+                       base::Unretained(this), audio_focus_type,
+                       true /* success */));
   } else {
     EnsureServiceConnection();
 
@@ -114,12 +116,11 @@ AudioFocusDelegateDefault::RequestAudioFocus(AudioFocusType audio_focus_type) {
         media_session_->AddRemote();
 
     audio_focus_ptr_->RequestGroupedAudioFocus(
-        request_client_remote_.BindNewPipeAndPassReceiver(),
+        request_id_, request_client_remote_.BindNewPipeAndPassReceiver(),
         std::move(media_session), session_info_.Clone(), audio_focus_type,
         GetAudioFocusGroupId(media_session_),
-        base::BindOnce(
-            &AudioFocusDelegateDefault::FinishInitialAudioFocusRequest,
-            base::Unretained(this), audio_focus_type));
+        base::BindOnce(&AudioFocusDelegateDefault::FinishAudioFocusRequest,
+                       base::Unretained(this), audio_focus_type));
   }
 
   // Return delayed as we make the async call to request audio focus.
@@ -155,18 +156,13 @@ void AudioFocusDelegateDefault::MediaSessionInfoChanged(
   session_info_ = std::move(session_info);
 }
 
-void AudioFocusDelegateDefault::FinishAudioFocusRequest(AudioFocusType type) {
+void AudioFocusDelegateDefault::FinishAudioFocusRequest(AudioFocusType type,
+                                                        bool success) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(request_client_remote_.is_bound());
 
   audio_focus_type_ = type;
-  media_session_->FinishSystemAudioFocusRequest(type, true /* result */);
-}
-
-void AudioFocusDelegateDefault::FinishInitialAudioFocusRequest(
-    AudioFocusType type,
-    const base::UnguessableToken& request_id) {
-  FinishAudioFocusRequest(type);
+  media_session_->FinishSystemAudioFocusRequest(type, success);
 }
 
 void AudioFocusDelegateDefault::EnsureServiceConnection() {

services/media_session/audio_focus_manager.cc

@@ -63,19 +63,30 @@ void AudioFocusManager::RequestAudioFocus(
     mojom::MediaSessionInfoPtr session_info,
     mojom::AudioFocusType type,
     RequestAudioFocusCallback callback) {
-  RequestGroupedAudioFocus(
-      std::move(receiver), std::move(media_session), std::move(session_info),
-      type, base::UnguessableToken::Create(), std::move(callback));
+  auto request_id = base::UnguessableToken::Create();
+
+  RequestAudioFocusInternal(
+      std::make_unique<AudioFocusRequest>(
+          weak_ptr_factory_.GetWeakPtr(), std::move(receiver),
+          std::move(media_session), std::move(session_info), type, request_id,
+          GetBindingSourceName(), base::UnguessableToken::Create()),
+      type);
+
+  std::move(callback).Run(request_id);
 }
 
 void AudioFocusManager::RequestGroupedAudioFocus(
+    const base::UnguessableToken& request_id,
     mojo::PendingReceiver<mojom::AudioFocusRequestClient> receiver,
     mojo::PendingRemote<mojom::MediaSession> media_session,
     mojom::MediaSessionInfoPtr session_info,
     mojom::AudioFocusType type,
     const base::UnguessableToken& group_id,
     RequestGroupedAudioFocusCallback callback) {
-  base::UnguessableToken request_id = base::UnguessableToken::Create();
+  if (IsFocusEntryPresent(request_id)) {
+    std::move(callback).Run(false /* success */);
+    return;
+  }
 
   RequestAudioFocusInternal(
       std::make_unique<AudioFocusRequest>(
@@ -84,7 +95,7 @@ void AudioFocusManager::RequestGroupedAudioFocus(
           GetBindingSourceName(), group_id),
       type);
 
-  std::move(callback).Run(request_id);
+  std::move(callback).Run(true /* success */);
 }
 
 void AudioFocusManager::GetFocusRequests(GetFocusRequestsCallback callback) {
@@ -314,6 +325,16 @@ std::unique_ptr<AudioFocusRequest> AudioFocusManager::RemoveFocusEntryIfPresent(
   return row;
 }
 
+bool AudioFocusManager::IsFocusEntryPresent(
+    const base::UnguessableToken& id) const {
+  for (auto& row : audio_focus_stack_) {
+    if (row->id() == id)
+      return true;
+  }
+
+  return false;
+}
+
 const std::string& AudioFocusManager::GetBindingSourceName() const {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   return bindings_.dispatch_context()->source_name;

services/media_session/audio_focus_manager.h

@@ -56,12 +56,13 @@ class AudioFocusManager : public mojom::AudioFocusManager,
       mojom::AudioFocusType type,
       RequestAudioFocusCallback callback) override;
   void RequestGroupedAudioFocus(
+      const base::UnguessableToken& request_id,
       mojo::PendingReceiver<mojom::AudioFocusRequestClient> receiver,
       mojo::PendingRemote<mojom::MediaSession> media_session,
       mojom::MediaSessionInfoPtr session_info,
       mojom::AudioFocusType type,
       const base::UnguessableToken& group_id,
-      RequestAudioFocusCallback callback) override;
+      RequestGroupedAudioFocusCallback callback) override;
   void GetFocusRequests(GetFocusRequestsCallback callback) override;
   void AddObserver(
       mojo::PendingRemote<mojom::AudioFocusObserver> observer) override;
@@ -115,6 +116,8 @@ class AudioFocusManager : public mojom::AudioFocusManager,
 
   std::unique_ptr<AudioFocusRequest> RemoveFocusEntryIfPresent(RequestId id);
 
+  bool IsFocusEntryPresent(const base::UnguessableToken& id) const;
+
   // Returns the source name of the binding currently accessing the Audio
   // Focus Manager API over mojo.
   const std::string& GetBindingSourceName() const;

services/media_session/audio_focus_manager_unittest.cc

@@ -105,12 +105,12 @@ class AudioFocusManagerTest
                                                  audio_focus_type);
   }
 
-  AudioFocusManager::RequestId RequestGroupedAudioFocus(
-      test::MockMediaSession* session,
-      mojom::AudioFocusType audio_focus_type,
-      const base::UnguessableToken& group_id) {
+  bool RequestGroupedAudioFocus(const base::UnguessableToken& request_id,
+                                test::MockMediaSession* session,
+                                mojom::AudioFocusType audio_focus_type,
+                                const base::UnguessableToken& group_id) {
     return session->RequestGroupedAudioFocusFromService(
-        audio_focus_ptr_, audio_focus_type, group_id);
+        request_id, audio_focus_ptr_, audio_focus_type, group_id);
   }
 
   mojom::MediaSessionDebugInfoPtr GetDebugInfo(
@@ -1059,8 +1059,9 @@ TEST_P(AudioFocusManagerTest, AudioFocusGrouping_LayeredFocus) {
 
   base::UnguessableToken group_id = base::UnguessableToken::Create();
 
-  RequestGroupedAudioFocus(&media_session_1, mojom::AudioFocusType::kGain,
-                           group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(base::UnguessableToken::Create(),
+                                       &media_session_1,
+                                       mojom::AudioFocusType::kGain, group_id));
   EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
             GetState(&media_session_1));
 
@@ -1071,8 +1072,9 @@ TEST_P(AudioFocusManagerTest, AudioFocusGrouping_LayeredFocus) {
 
   // When we request audio focus for media_session_3 the group will take audio
   // focus and we suspend the ducking session.
-  RequestGroupedAudioFocus(&media_session_3,
-                           mojom::AudioFocusType::kGainTransient, group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(
+      base::UnguessableToken::Create(), &media_session_3,
+      mojom::AudioFocusType::kGainTransient, group_id));
   EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
             GetState(&media_session_3));
 
@@ -1094,8 +1096,9 @@ TEST_P(AudioFocusManagerTest, AudioFocusGrouping_TransientResume) {
 
   base::UnguessableToken group_id = base::UnguessableToken::Create();
 
-  RequestGroupedAudioFocus(&media_session_1, mojom::AudioFocusType::kGain,
-                           group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(base::UnguessableToken::Create(),
+                                       &media_session_1,
+                                       mojom::AudioFocusType::kGain, group_id));
   EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
             GetState(&media_session_1));
 
@@ -1103,8 +1106,9 @@ TEST_P(AudioFocusManagerTest, AudioFocusGrouping_TransientResume) {
   EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
             GetState(&media_session_2));
 
-  RequestGroupedAudioFocus(&media_session_3, mojom::AudioFocusType::kGain,
-                           group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(base::UnguessableToken::Create(),
+                                       &media_session_3,
+                                       mojom::AudioFocusType::kGain, group_id));
   EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
             GetState(&media_session_3));
 
@@ -1143,13 +1147,15 @@ TEST_P(AudioFocusManagerTest, AudioFocusGrouping_DoNotSuspendSameGroup) {
 
   base::UnguessableToken group_id = base::UnguessableToken::Create();
 
-  RequestGroupedAudioFocus(&media_session_1, mojom::AudioFocusType::kGain,
-                           group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(base::UnguessableToken::Create(),
+                                       &media_session_1,
+                                       mojom::AudioFocusType::kGain, group_id));
   EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
             GetState(&media_session_1));
 
-  RequestGroupedAudioFocus(&media_session_2, mojom::AudioFocusType::kGain,
-                           group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(base::UnguessableToken::Create(),
+                                       &media_session_2,
+                                       mojom::AudioFocusType::kGain, group_id));
   EXPECT_EQ(IsGroupingEnabled()
                 ? mojom::MediaSessionInfo::SessionState::kActive
                 : mojom::MediaSessionInfo::SessionState::kSuspended,
@@ -1164,13 +1170,15 @@ TEST_P(AudioFocusManagerTest, AudioFocusGrouping_DuckSameGroup) {
 
   base::UnguessableToken group_id = base::UnguessableToken::Create();
 
-  RequestGroupedAudioFocus(&media_session_1, mojom::AudioFocusType::kGain,
-                           group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(base::UnguessableToken::Create(),
+                                       &media_session_1,
+                                       mojom::AudioFocusType::kGain, group_id));
   EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
             GetState(&media_session_1));
 
-  RequestGroupedAudioFocus(
-      &media_session_2, mojom::AudioFocusType::kGainTransientMayDuck, group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(
+      base::UnguessableToken::Create(), &media_session_2,
+      mojom::AudioFocusType::kGainTransientMayDuck, group_id));
   EXPECT_EQ(GetStateFromParam(mojom::MediaSessionInfo::SessionState::kDucking),
             GetState(&media_session_1));
 }
@@ -1181,13 +1189,15 @@ TEST_P(AudioFocusManagerTest, AudioFocusGrouping_TransientSameGroup) {
 
   base::UnguessableToken group_id = base::UnguessableToken::Create();
 
-  RequestGroupedAudioFocus(&media_session_1, mojom::AudioFocusType::kGain,
-                           group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(base::UnguessableToken::Create(),
+                                       &media_session_1,
+                                       mojom::AudioFocusType::kGain, group_id));
   EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
             GetState(&media_session_1));
 
-  RequestGroupedAudioFocus(&media_session_2,
-                           mojom::AudioFocusType::kGainTransient, group_id);
+  ASSERT_TRUE(RequestGroupedAudioFocus(
+      base::UnguessableToken::Create(), &media_session_2,
+      mojom::AudioFocusType::kGainTransient, group_id));
   EXPECT_EQ(IsGroupingEnabled()
                 ? mojom::MediaSessionInfo::SessionState::kActive
                 : mojom::MediaSessionInfo::SessionState::kSuspended,
@@ -1675,4 +1685,30 @@ TEST_P(AudioFocusManagerTest, TransientPauseShouldDelayLastActionOnly) {
             GetState(&media_session_1));
 }
 
+TEST_P(AudioFocusManagerTest, RequestIdValidation) {
+  test::MockMediaSession media_session_1;
+  test::MockMediaSession media_session_2;
+
+  base::UnguessableToken request_id = base::UnguessableToken::Create();
+
+  EXPECT_TRUE(RequestGroupedAudioFocus(request_id, &media_session_1,
+                                       mojom::AudioFocusType::kGain,
+                                       base::UnguessableToken::Create()));
+  EXPECT_EQ(request_id, GetAudioFocusedSession());
+
+  // The audio focus request should fail since we have already used that id.
+  EXPECT_FALSE(RequestGroupedAudioFocus(request_id, &media_session_2,
+                                        mojom::AudioFocusType::kGain,
+                                        base::UnguessableToken::Create()));
+
+  media_session_1.AbandonAudioFocusFromClient();
+  EXPECT_EQ(base::UnguessableToken::Null(), GetAudioFocusedSession());
+
+  // If we abandon focus then we should be able to use the id now.
+  EXPECT_TRUE(RequestGroupedAudioFocus(request_id, &media_session_2,
+                                       mojom::AudioFocusType::kGain,
+                                       base::UnguessableToken::Create()));
+  EXPECT_EQ(request_id, GetAudioFocusedSession());
+}
+
 }  // namespace media_session

services/media_session/public/cpp/test/mock_media_session.cc

@@ -323,37 +323,42 @@ base::UnguessableToken MockMediaSession::RequestAudioFocusFromService(
   return request_id_;
 }
 
-base::UnguessableToken MockMediaSession::RequestGroupedAudioFocusFromService(
+bool MockMediaSession::RequestGroupedAudioFocusFromService(
+    const base::UnguessableToken& request_id,
     mojom::AudioFocusManagerPtr& service,
     mojom::AudioFocusType audio_focus_type,
     const base::UnguessableToken& group_id) {
   if (afr_client_.is_bound()) {
     RequestAudioFocusFromClient(audio_focus_type);
-  } else {
-    DCHECK(request_id_.is_empty());
+    SetState(mojom::MediaSessionInfo::SessionState::kActive);
+    return true;
+  }
 
-    // Build a new audio focus request.
-    mojo::PendingRemote<mojom::MediaSession> media_session;
-    receivers_.Add(this, media_session.InitWithNewPipeAndPassReceiver());
+  DCHECK(request_id_.is_empty());
 
-    service->RequestGroupedAudioFocus(
-        afr_client_.BindNewPipeAndPassReceiver(), std::move(media_session),
-        GetMediaSessionInfoSync(), audio_focus_type, group_id,
-        base::BindOnce(
-            [](base::UnguessableToken* id,
-               const base::UnguessableToken& received_id) {
-              *id = received_id;
-            },
-            &request_id_));
+  // Build a new audio focus request.
+  mojo::PendingRemote<mojom::MediaSession> media_session;
+  receivers_.Add(this, media_session.InitWithNewPipeAndPassReceiver());
+  bool success;
 
-    service.FlushForTesting();
-    afr_client_.FlushForTesting();
-  }
+  service->RequestGroupedAudioFocus(
+      request_id, afr_client_.BindNewPipeAndPassReceiver(),
+      std::move(media_session), GetMediaSessionInfoSync(), audio_focus_type,
+      group_id,
+      base::BindOnce([](bool* success, bool result) { *success = result; },
+                     &success));
 
-  DCHECK(!request_id_.is_empty());
-  SetState(mojom::MediaSessionInfo::SessionState::kActive);
+  service.FlushForTesting();
+  afr_client_.FlushForTesting();
 
-  return request_id_;
+  if (success) {
+    request_id_ = request_id;
+    SetState(mojom::MediaSessionInfo::SessionState::kActive);
+  } else {
+    afr_client_.reset();
+  }
+
+  return success;
 }
 
 mojom::MediaSessionInfo::SessionState MockMediaSession::GetState() const {

services/media_session/public/cpp/test/mock_media_session.h

@@ -150,7 +150,8 @@ class COMPONENT_EXPORT(MEDIA_SESSION_TEST_SUPPORT_CPP) MockMediaSession
       mojom::AudioFocusManagerPtr& service,
       mojom::AudioFocusType audio_foucs_type);
 
-  base::UnguessableToken RequestGroupedAudioFocusFromService(
+  bool RequestGroupedAudioFocusFromService(
+      const base::UnguessableToken& request_id,
       mojom::AudioFocusManagerPtr& service,
       mojom::AudioFocusType audio_focus_type,
       const base::UnguessableToken& group_id);

services/media_session/public/mojom/audio_focus.mojom

@@ -101,16 +101,20 @@ interface AudioFocusManager {
                       AudioFocusType type) =>
                           (mojo_base.mojom.UnguessableToken request_id);
 
-  // Requests audio focus as above but with a |group_id| that is used for
-  // grouping sessions together. This is when a group of media sessions
-  // will share audio focus.
+  // Requests audio focus with |type| for the |media_session| with
+  // |session_info|. Media sessions should provide a |request| that will
+  // provide an AudioFocusRequestClient that can be used to control this
+  // request. |request_id| should be generated by the client and should be
+  // unique. If it is not valid then the request will fail. |group_id| is
+  // used for grouping sessions together. This is when a group of sessions
+  // will share audio focus. Returns whether the request succeeded.
   [MinVersion=4] RequestGroupedAudioFocus@4(
+      mojo_base.mojom.UnguessableToken request_id,
       pending_receiver<AudioFocusRequestClient> client,
       pending_remote<MediaSession> media_session,
       MediaSessionInfo session_info,
       AudioFocusType type,
-      mojo_base.mojom.UnguessableToken group_id) =>
-          (mojo_base.mojom.UnguessableToken request_id);
+      mojo_base.mojom.UnguessableToken group_id) => (bool success);
 
   // Gets all the information about all |MediaSessions| that have requested
   // audio focus and their current requested type.