[Audio Focus] Add active changed observer

In audio focus there is a concept of an "active"
media session. This is the top most non-temporary
session. This is slightly different from the current
focused session which may be temporary. We use the
active media session for media controller too.

This adds a method for tracking the current active
media session. This will be used for the media
notification for hiding/showing it. At the moment
it uses focus which may result it being shown
incorrectly.

BUG=897836

Change-Id: I0e2f82ffe23394e04d72622aa0d27cd4e8834d7a
Reviewed-on: https://chromium-review.googlesource.com/c/1330696
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608153}

ash/media/media_notification_controller.cc

@@ -67,13 +67,19 @@ MediaNotificationController::MediaNotificationController(
 
 MediaNotificationController::~MediaNotificationController() = default;
 
-void MediaNotificationController::OnFocusGained(
-    media_session::mojom::MediaSessionInfoPtr session_info,
-    media_session::mojom::AudioFocusType type) {
+void MediaNotificationController::OnActiveSessionChanged(
+    media_session::mojom::AudioFocusRequestStatePtr session) {
+  // Hide the notification if the active session is null.
+  if (session.is_null()) {
+    message_center::MessageCenter::Get()->RemoveNotification(
+        kMediaSessionNotificationId, false);
+    return;
+  }
+
   if (IsMediaSessionNotificationVisible())
     return;
 
-  session_info_ = std::move(session_info);
+  session_info_ = std::move(session->session_info);
 
   std::unique_ptr<message_center::Notification> notification =
       ash::CreateSystemNotification(
@@ -101,15 +107,6 @@ void MediaNotificationController::OnFocusGained(
       std::move(notification));
 }
 
-void MediaNotificationController::OnFocusLost(
-    media_session::mojom::MediaSessionInfoPtr session_info) {
-  if (!IsMediaSessionNotificationVisible())
-    return;
-
-  message_center::MessageCenter::Get()->RemoveNotification(
-      kMediaSessionNotificationId, false);
-}
-
 void MediaNotificationController::MediaSessionInfoChanged(
     media_session::mojom::MediaSessionInfoPtr session_info) {
   session_info_ = std::move(session_info);

ash/media/media_notification_controller.h

@@ -34,9 +34,11 @@ class ASH_EXPORT MediaNotificationController
 
   // media_session::mojom::AudioFocusObserver:
   void OnFocusGained(media_session::mojom::MediaSessionInfoPtr session_info,
-                     media_session::mojom::AudioFocusType type) override;
+                     media_session::mojom::AudioFocusType type) override {}
   void OnFocusLost(
-      media_session::mojom::MediaSessionInfoPtr session_info) override;
+      media_session::mojom::MediaSessionInfoPtr session_info) override {}
+  void OnActiveSessionChanged(
+      media_session::mojom::AudioFocusRequestStatePtr session) override;
 
   // media_session::mojom::MediaSessionObserver:
   void MediaSessionInfoChanged(

ash/media/media_notification_controller_unittest.cc

@@ -17,8 +17,7 @@
 
 namespace ash {
 
-using media_session::mojom::AudioFocusType;
-using media_session::mojom::MediaSessionInfo;
+using media_session::mojom::AudioFocusRequestState;
 
 namespace {
 
@@ -56,43 +55,43 @@ class MediaNotificationControllerTest : public AshTestBase {
   DISALLOW_COPY_AND_ASSIGN(MediaNotificationControllerTest);
 };
 
-TEST_F(MediaNotificationControllerTest, OnFocusGainedLost) {
+TEST_F(MediaNotificationControllerTest, OnActiveSessionChanged) {
   EXPECT_FALSE(IsMediaNotificationShown());
   EXPECT_EQ(0, GetVisibleNotificationCount());
   EXPECT_EQ(0, GetPopupNotificationCount());
 
-  Shell::Get()->media_notification_controller()->OnFocusGained(
-      MediaSessionInfo::New(), AudioFocusType::kGain);
+  Shell::Get()->media_notification_controller()->OnActiveSessionChanged(
+      AudioFocusRequestState::New());
   EXPECT_TRUE(IsMediaNotificationShown());
   EXPECT_EQ(1, GetVisibleNotificationCount());
   EXPECT_EQ(0, GetPopupNotificationCount());
 
-  Shell::Get()->media_notification_controller()->OnFocusGained(
-      MediaSessionInfo::New(), AudioFocusType::kGain);
+  Shell::Get()->media_notification_controller()->OnActiveSessionChanged(
+      AudioFocusRequestState::New());
   EXPECT_TRUE(IsMediaNotificationShown());
   EXPECT_EQ(1, GetVisibleNotificationCount());
   EXPECT_EQ(0, GetPopupNotificationCount());
 
-  Shell::Get()->media_notification_controller()->OnFocusLost(
-      MediaSessionInfo::New());
+  Shell::Get()->media_notification_controller()->OnActiveSessionChanged(
+      nullptr);
   EXPECT_FALSE(IsMediaNotificationShown());
   EXPECT_EQ(0, GetVisibleNotificationCount());
   EXPECT_EQ(0, GetPopupNotificationCount());
 }
 
-TEST_F(MediaNotificationControllerTest, OnFocusLost_Noop) {
+TEST_F(MediaNotificationControllerTest, OnActiveSessionChanged_Noop) {
   EXPECT_FALSE(IsMediaNotificationShown());
 
-  Shell::Get()->media_notification_controller()->OnFocusLost(
-      MediaSessionInfo::New());
+  Shell::Get()->media_notification_controller()->OnActiveSessionChanged(
+      nullptr);
   EXPECT_FALSE(IsMediaNotificationShown());
 }
 
 TEST_F(MediaNotificationControllerTest, NotificationHasCustomViewType) {
   EXPECT_FALSE(IsMediaNotificationShown());
 
-  Shell::Get()->media_notification_controller()->OnFocusGained(
-      MediaSessionInfo::New(), AudioFocusType::kGain);
+  Shell::Get()->media_notification_controller()->OnActiveSessionChanged(
+      AudioFocusRequestState::New());
   message_center::Notification* notification =
       message_center::MessageCenter::Get()->FindVisibleNotificationById(
           kMediaSessionNotificationId);

ash/media/media_notification_view_unittest.cc

@@ -78,8 +78,11 @@ class MediaNotificationViewTest : public AshTestBase {
             base::Unretained(this)));
 
     // Show the notification.
-    Shell::Get()->media_notification_controller()->OnFocusGained(
-        std::move(session_info), media_session::mojom::AudioFocusType::kGain);
+    media_session::mojom::AudioFocusRequestStatePtr session(
+        media_session::mojom::AudioFocusRequestState::New());
+    session->session_info = std::move(session_info);
+    Shell::Get()->media_notification_controller()->OnActiveSessionChanged(
+        std::move(session));
 
     message_center::Notification* notification =
         message_center::MessageCenter::Get()->FindVisibleNotificationById(
@@ -229,7 +232,7 @@ TEST_F(MediaNotificationViewTest, ClickNotification) {
   EXPECT_EQ(0, media_controller()->toggle_suspend_resume_count());
 }
 
-TEST_F(MediaNotificationViewTest, PlayToggle_FromFocusGain) {
+TEST_F(MediaNotificationViewTest, PlayToggle_FromActiveSessionChanged) {
   {
     views::ToggleImageButton* button =
         static_cast<views::ToggleImageButton*>(button_row()->child_at(1));
@@ -237,8 +240,8 @@ TEST_F(MediaNotificationViewTest, PlayToggle_FromFocusGain) {
     EXPECT_FALSE(button->toggled_for_testing());
   }
 
-  Shell::Get()->media_notification_controller()->OnFocusLost(
-      media_session::mojom::MediaSessionInfo::New());
+  Shell::Get()->media_notification_controller()->OnActiveSessionChanged(
+      nullptr);
 
   // Disable the tray and run the loop to make sure that the existing view is
   // destroyed.

content/browser/media/media_internals_audio_focus_helper.h

@@ -29,6 +29,8 @@ class MediaInternalsAudioFocusHelper
                      media_session::mojom::AudioFocusType type) override;
   void OnFocusLost(
       media_session::mojom::MediaSessionInfoPtr media_session) override;
+  void OnActiveSessionChanged(
+      media_session::mojom::AudioFocusRequestStatePtr session) override {}
 
   // Sets whether we should listen to audio focus events.
   void SetEnabled(bool enabled);

services/media_session/audio_focus_manager.cc

@@ -106,6 +106,15 @@ class AudioFocusManager::StackRow : public mojom::AudioFocusRequestClient {
 
   const std::string& source_name() const { return source_name_; }
 
+  mojom::AudioFocusRequestStatePtr ToAudioFocusRequestState() const {
+    auto request = mojom::AudioFocusRequestState::New();
+    request->session_info = session_info_.Clone();
+    request->audio_focus_type = audio_focus_type_;
+    request->request_id = id_;
+    request->source_name = source_name_;
+    return request;
+  }
+
  private:
   void OnConnectionError() {
     // Since we have multiple pathways that can call |OnConnectionError| we
@@ -158,14 +167,8 @@ void AudioFocusManager::RequestAudioFocus(
 void AudioFocusManager::GetFocusRequests(GetFocusRequestsCallback callback) {
   std::vector<mojom::AudioFocusRequestStatePtr> requests;
 
-  for (const auto& row : audio_focus_stack_) {
-    auto request = mojom::AudioFocusRequestState::New();
-    request->session_info = row->info().Clone();
-    request->audio_focus_type = row->audio_focus_type();
-    request->request_id = row->id();
-    request->source_name = row->source_name();
-    requests.push_back(std::move(request));
-  }
+  for (const auto& row : audio_focus_stack_)
+    requests.push_back(row->ToAudioFocusRequestState());
 
   std::move(callback).Run(std::move(requests));
 }
@@ -190,7 +193,7 @@ void AudioFocusManager::AbandonAudioFocusInternal(RequestId id) {
 
   if (audio_focus_stack_.back()->id() != id) {
     RemoveFocusEntryIfPresent(id);
-    active_media_controller_.SetMediaSession(GetActiveGainSession());
+    MaybeUpdateActiveSession();
     return;
   }
 
@@ -203,14 +206,14 @@ void AudioFocusManager::AbandonAudioFocusInternal(RequestId id) {
       observer->OnFocusLost(row->info().Clone());
     });
 
-    active_media_controller_.SetMediaSession(GetActiveGainSession());
+    MaybeUpdateActiveSession();
     return;
   }
 
   if (IsAudioFocusEnforcementEnabled())
     EnforceAudioFocusAbandon(row->audio_focus_type());
 
-  active_media_controller_.SetMediaSession(GetActiveGainSession());
+  MaybeUpdateActiveSession();
 
   // Notify observers that we lost audio focus.
   observers_.ForAllPtrs([&row](mojom::AudioFocusObserver* observer) {
@@ -268,7 +271,7 @@ void AudioFocusManager::RequestAudioFocusInternal(
   row->SetAudioFocusType(type);
   audio_focus_stack_.push_back(std::move(row));
 
-  active_media_controller_.SetMediaSession(GetActiveGainSession());
+  MaybeUpdateActiveSession();
 
   // Notify observers that we were gained audio focus.
   mojom::MediaSessionInfoPtr session_info =
@@ -353,14 +356,29 @@ void AudioFocusManager::EnforceAudioFocusAbandon(mojom::AudioFocusType type) {
   }
 }
 
-mojom::MediaSession* AudioFocusManager::GetActiveGainSession() const {
+void AudioFocusManager::MaybeUpdateActiveSession() {
+  StackRow* active = nullptr;
+
   for (auto& row : base::Reversed(audio_focus_stack_)) {
     if (row->audio_focus_type() != mojom::AudioFocusType::kGain)
       continue;
-    return row->session();
+
+    active = row.get();
+    break;
+  }
+
+  if (!active_media_controller_.SetMediaSession(active ? active->session()
+                                                       : nullptr)) {
+    return;
   }
 
-  return nullptr;
+  mojom::AudioFocusRequestStatePtr state =
+      active ? active->ToAudioFocusRequestState() : nullptr;
+
+  // Notify observers that the active media session changed.
+  observers_.ForAllPtrs([&state](mojom::AudioFocusObserver* observer) {
+    observer->OnActiveSessionChanged(state.Clone());
+  });
 }
 
 AudioFocusManager::AudioFocusManager() {

services/media_session/audio_focus_manager.h

@@ -88,8 +88,7 @@ class AudioFocusManager : public mojom::AudioFocusManager,
   void AbandonAudioFocusInternal(RequestId);
   void EnforceAudioFocusAbandon(mojom::AudioFocusType);
 
-  // Get the top most media session with the gain audio focus type.
-  mojom::MediaSession* GetActiveGainSession() const;
+  void MaybeUpdateActiveSession();
 
   std::unique_ptr<StackRow> RemoveFocusEntryIfPresent(RequestId id);
 

services/media_session/audio_focus_manager_unittest.cc

@@ -985,4 +985,53 @@ TEST_P(AudioFocusManagerTest,
   }
 }
 
+TEST_P(AudioFocusManagerTest, ObserverActiveSessionChanged) {
+  test::MockMediaSession media_session_1;
+  test::MockMediaSession media_session_2;
+
+  {
+    std::unique_ptr<test::TestAudioFocusObserver> observer = CreateObserver();
+
+    RequestAudioFocus(&media_session_1, mojom::AudioFocusType::kGain);
+    EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
+              GetState(&media_session_1));
+
+    EXPECT_EQ(media_session_1.GetRequestIdFromClient(),
+              observer->active_session_->request_id);
+  }
+
+  {
+    std::unique_ptr<test::TestAudioFocusObserver> observer = CreateObserver();
+
+    RequestAudioFocus(&media_session_2, mojom::AudioFocusType::kGainTransient);
+    EXPECT_EQ(mojom::MediaSessionInfo::SessionState::kActive,
+              GetState(&media_session_2));
+
+    EXPECT_NE(
+        test::TestAudioFocusObserver::NotificationType::kActiveSessionChanged,
+        observer->notifications().back());
+    EXPECT_TRUE(observer->active_session_.is_null());
+  }
+
+  {
+    std::unique_ptr<test::TestAudioFocusObserver> observer = CreateObserver();
+    media_session_2.AbandonAudioFocusFromClient();
+
+    EXPECT_NE(
+        test::TestAudioFocusObserver::NotificationType::kActiveSessionChanged,
+        observer->notifications().back());
+    EXPECT_TRUE(observer->active_session_.is_null());
+  }
+
+  {
+    std::unique_ptr<test::TestAudioFocusObserver> observer = CreateObserver();
+    media_session_1.AbandonAudioFocusFromClient();
+
+    EXPECT_EQ(
+        test::TestAudioFocusObserver::NotificationType::kActiveSessionChanged,
+        observer->notifications().back());
+    EXPECT_TRUE(observer->active_session_.is_null());
+  }
+}
+
 }  // namespace media_session

services/media_session/media_controller.cc

@@ -77,20 +77,24 @@ void MediaController::NextTrack() {
     session_->NextTrack();
 }
 
-void MediaController::SetMediaSession(mojom::MediaSession* session) {
+bool MediaController::SetMediaSession(mojom::MediaSession* session) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (session == nullptr) {
+  bool changed = session != session_;
+
+  if (changed) {
     session_binding_.Close();
-  } else if (session_ != session) {
+
+    if (session) {
       // Add |this| as an observer for |session|.
-    session_binding_.Close();
       mojom::MediaSessionObserverPtr observer;
       session_binding_.Bind(mojo::MakeRequest(&observer));
       session->AddObserver(std::move(observer));
     }
+  }
 
   session_ = session;
+  return changed;
 }
 
 void MediaController::BindToInterface(mojom::MediaControllerRequest request) {

services/media_session/media_controller.h

@@ -30,16 +30,19 @@ class MediaController : public mojom::MediaController,
   void Suspend() override;
   void Resume() override;
   void ToggleSuspendResume() override;
-  void AddObserver(mojom::MediaSessionObserverPtr) override;
+  void AddObserver(mojom::MediaSessionObserverPtr observer) override;
   void PreviousTrack() override;
   void NextTrack() override;
 
   // mojom::MediaSessionObserver overrides.
-  void MediaSessionInfoChanged(mojom::MediaSessionInfoPtr) override;
+  void MediaSessionInfoChanged(
+      mojom::MediaSessionInfoPtr session_info) override;
 
-  void SetMediaSession(mojom::MediaSession*);
+  // Sets the media session that the controller should be bound to. If the
+  // session is already bound to the same session then we will return false.
+  bool SetMediaSession(mojom::MediaSession* session);
 
-  void BindToInterface(mojom::MediaControllerRequest);
+  void BindToInterface(mojom::MediaControllerRequest request);
   void FlushForTesting();
 
  private:

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

@@ -42,6 +42,12 @@ void TestAudioFocusObserver::OnFocusLost(
     run_loop_.Quit();
 }
 
+void TestAudioFocusObserver::OnActiveSessionChanged(
+    media_session::mojom::AudioFocusRequestStatePtr session) {
+  active_session_ = std::move(session);
+  notifications_.push_back(NotificationType::kActiveSessionChanged);
+}
+
 void TestAudioFocusObserver::WaitForGainedEvent() {
   if (!focus_gained_session_.is_null())
     return;

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

@@ -22,10 +22,11 @@ class COMPONENT_EXPORT(MEDIA_SESSION_TEST_SUPPORT_CPP) TestAudioFocusObserver
   TestAudioFocusObserver();
   ~TestAudioFocusObserver() override;
 
-  void OnFocusGained(media_session::mojom::MediaSessionInfoPtr,
-                     media_session::mojom::AudioFocusType) override;
-
-  void OnFocusLost(media_session::mojom::MediaSessionInfoPtr) override;
+  void OnFocusGained(media_session::mojom::MediaSessionInfoPtr session,
+                     media_session::mojom::AudioFocusType type) override;
+  void OnFocusLost(media_session::mojom::MediaSessionInfoPtr session) override;
+  void OnActiveSessionChanged(
+      media_session::mojom::AudioFocusRequestStatePtr session) override;
 
   void WaitForGainedEvent();
   void WaitForLostEvent();
@@ -40,11 +41,13 @@ class COMPONENT_EXPORT(MEDIA_SESSION_TEST_SUPPORT_CPP) TestAudioFocusObserver
   // These store the values we received.
   media_session::mojom::MediaSessionInfoPtr focus_gained_session_;
   media_session::mojom::MediaSessionInfoPtr focus_lost_session_;
+  media_session::mojom::AudioFocusRequestStatePtr active_session_;
 
   // These store the order of notifications that were received by the observer.
   enum class NotificationType {
     kFocusGained,
     kFocusLost,
+    kActiveSessionChanged,
   };
   const std::vector<NotificationType>& notifications() const {
     return notifications_;

services/media_session/public/mojom/audio_focus.mojom

@@ -38,13 +38,18 @@ struct AudioFocusRequestState {
 };
 
 // The observer for audio focus events.
-// Next Method ID: 2
 interface AudioFocusObserver {
   // The given |session| gained audio focus with the specified |type|.
-  OnFocusGained@0(MediaSessionInfo session, AudioFocusType type);
+  OnFocusGained(MediaSessionInfo session, AudioFocusType type);
 
   // The given |session| lost audio focus.
-  OnFocusLost@1(MediaSessionInfo session);
+  OnFocusLost(MediaSessionInfo session);
+
+  // The currently active session changed. The active session is the top most
+  // session that is not temporary. This is the session that will be associated
+  // with the MediaController interface. If all the media sessions have ended
+  // then |session| will be null.
+  OnActiveSessionChanged(AudioFocusRequestState? session);
 };
 
 // Controls audio focus for an associated request.