[Media Router] Ensure dummy notification is hidden when it is redundant.

Bug: 1107164, b/161612881
Change-Id: Idf86fa429de43b5da554d57bd1e8d1cbd186835b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2378772
Commit-Queue: John Williams <jrw@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809019}

chrome/browser/ui/global_media_controls/cast_media_notification_item_unittest.cc

@@ -57,12 +57,16 @@ class MockBitmapFetcher : public BitmapFetcher {
 class MockMediaNotificationController
     : public media_message_center::MediaNotificationController {
  public:
-  MOCK_METHOD1(ShowNotification, void(const std::string&));
-  MOCK_METHOD1(HideNotification, void(const std::string&));
-  MOCK_METHOD1(RemoveItem, void(const std::string&));
-  MOCK_CONST_METHOD0(GetTaskRunner, scoped_refptr<base::SequencedTaskRunner>());
-  MOCK_METHOD2(LogMediaSessionActionButtonPressed,
-               void(const std::string&, MediaSessionAction));
+  MOCK_METHOD(void, ShowNotification, (const std::string&));
+  MOCK_METHOD(void, HideNotification, (const std::string&));
+  MOCK_METHOD(void, RemoveItem, (const std::string&));
+  MOCK_METHOD(scoped_refptr<base::SequencedTaskRunner>,
+              GetTaskRunner,
+              (),
+              (const));
+  MOCK_METHOD(void,
+              LogMediaSessionActionButtonPressed,
+              (const std::string&, MediaSessionAction));
 };
 
 class MockMediaNotificationView

chrome/browser/ui/global_media_controls/cast_media_notification_provider_unittest.cc

@@ -34,15 +34,16 @@ class MockMediaNotificationController
   MockMediaNotificationController() = default;
   ~MockMediaNotificationController() = default;
 
-  MOCK_METHOD1(ShowNotification, void(const std::string& id));
-  MOCK_METHOD1(HideNotification, void(const std::string& id));
-  MOCK_METHOD1(RemoveItem, void(const std::string& id));
+  MOCK_METHOD(void, ShowNotification, (const std::string& id));
+  MOCK_METHOD(void, HideNotification, (const std::string& id));
+  MOCK_METHOD(void, RemoveItem, (const std::string& id));
   scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() const override {
     return nullptr;
   }
-  MOCK_METHOD2(LogMediaSessionActionButtonPressed,
-               void(const std::string& id,
-                    media_session::mojom::MediaSessionAction action));
+  MOCK_METHOD(void,
+              LogMediaSessionActionButtonPressed,
+              (const std::string& id,
+               media_session::mojom::MediaSessionAction action));
 };
 
 class MockMediaNotificationView

chrome/browser/ui/global_media_controls/media_notification_service.cc

@@ -313,6 +313,8 @@ MediaNotificationService::MediaNotificationService(Profile* profile,
             base::BindRepeating(
                 &MediaNotificationService::OnCastNotificationsChanged,
                 base::Unretained(this)));
+    presentation_request_notification_provider_ =
+        std::make_unique<PresentationRequestNotificationProvider>(this);
   }
   if (media_router::GlobalMediaControlsCastStartStopEnabled()) {
     presentation_request_notification_provider_ =
@@ -424,8 +426,7 @@ void MediaNotificationService::OnFocusLost(
       &MediaNotificationService::OnItemUnfrozen, base::Unretained(this), id));
   active_controllable_session_ids_.erase(id);
   frozen_session_ids_.insert(id);
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(&id);
 }
 
 void MediaNotificationService::ShowNotification(const std::string& id) {
@@ -437,9 +438,7 @@ void MediaNotificationService::ShowNotification(const std::string& id) {
   }
 
   active_controllable_session_ids_.insert(id);
-
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(&id);
 
   if (!dialog_delegate_)
     return;
@@ -465,8 +464,7 @@ void MediaNotificationService::HideNotification(const std::string& id) {
     dragged_out_session_ids_.erase(id);
   }
 
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(&id);
 
   if (!dialog_delegate_)
     return;
@@ -483,16 +481,20 @@ void MediaNotificationService::RemoveItem(const std::string& id) {
   active_controllable_session_ids_.erase(id);
   frozen_session_ids_.erase(id);
   inactive_session_ids_.erase(id);
+  supplemental_notifications_.erase(id);
 
   if (base::Contains(dragged_out_session_ids_, id)) {
     overlay_media_notifications_manager_.CloseOverlayNotification(id);
     dragged_out_session_ids_.erase(id);
   }
 
+  // Copy |id| to avoid a dangling reference after the session is deleted.  This
+  // happens when |id| refers to a string owned by the session being removed.
+  const auto id_copy{id};
+
   sessions_.erase(id);
 
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(&id_copy);
 }
 
 void MediaNotificationService::LogMediaSessionActionButtonPressed(
@@ -594,9 +596,7 @@ void MediaNotificationService::OnContainerDraggedOut(const std::string& id,
       id, std::move(overlay_notification));
   active_controllable_session_ids_.erase(id);
   dragged_out_session_ids_.insert(id);
-
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(&id);
 }
 
 void MediaNotificationService::OnAudioSinkChosen(const std::string& id,
@@ -614,6 +614,13 @@ void MediaNotificationService::Shutdown() {
   presentation_request_notification_provider_.reset();
 }
 
+void MediaNotificationService::AddSupplementalNotification(
+    const std::string& id,
+    content::WebContents* web_contents) {
+  supplemental_notifications_.emplace(id, web_contents);
+  ShowNotification(id);
+}
+
 void MediaNotificationService::OnOverlayNotificationClosed(
     const std::string& id) {
   // If the session has been destroyed, no action is needed.
@@ -636,8 +643,7 @@ void MediaNotificationService::OnOverlayNotificationClosed(
     active_controllable_session_ids_.insert(id);
   dragged_out_session_ids_.erase(id);
 
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(&id);
 
   // If there's a dialog currently open, then we should show the item in the
   // dialog.
@@ -654,8 +660,7 @@ void MediaNotificationService::OnOverlayNotificationClosed(
 }
 
 void MediaNotificationService::OnCastNotificationsChanged() {
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(nullptr);
 }
 
 void MediaNotificationService::SetDialogDelegate(
@@ -729,8 +734,7 @@ void MediaNotificationService::OnSessionBecameActive(const std::string& id) {
   else
     active_controllable_session_ids_.insert(id);
 
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(&id);
 
   // If there's a dialog currently open, then we should show the item in the
   // dialog.
@@ -810,8 +814,7 @@ void MediaNotificationService::OnItemUnfrozen(const std::string& id) {
   if (!base::Contains(dragged_out_session_ids_, id))
     active_controllable_session_ids_.insert(id);
 
-  for (auto& observer : observers_)
-    observer.OnNotificationListChanged();
+  OnNotificationChanged(&id);
 }
 
 void MediaNotificationService::OnReceivedAudioFocusRequests(
@@ -851,3 +854,34 @@ MediaNotificationService::GetNonSessionNotificationItem(const std::string& id) {
 
   return nullptr;
 }
+
+void MediaNotificationService::OnNotificationChanged(
+    const std::string* changed_notification_id) {
+  for (auto& observer : observers_)
+    observer.OnNotificationListChanged();
+
+  // Avoid re-examining the supplemental notifications as a side-effect or
+  // showing or hiding a supplemental notification.
+  if (!changed_notification_id ||
+      base::Contains(supplemental_notifications_, *changed_notification_id))
+    return;
+
+  // Show or hide supplemental notifications as necessary.
+  for (const auto& pair : supplemental_notifications_) {
+    auto* web_contents = pair.second;
+    const bool should_hide = std::any_of(
+        sessions_.begin(), sessions_.end(),
+        [web_contents, this](const auto& pair) {
+          return pair.second.web_contents() == web_contents &&
+                 base::Contains(active_controllable_session_ids_, pair.first);
+        });
+
+    // If there is an active session associated with the same web contents as
+    // this supplemental notification, hide it; if not, show it.
+    if (should_hide) {
+      HideNotification(pair.first);
+    } else {
+      ShowNotification(pair.first);
+    }
+  }
+}

chrome/browser/ui/global_media_controls/media_notification_service.h

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
@@ -90,6 +91,11 @@ class MediaNotificationService
   // KeyedService implementation.
   void Shutdown() override;
 
+  // Adds a "suppplemental" notification, which should only be shown if there is
+  // no other notification associated with the same web contents.
+  void AddSupplementalNotification(const std::string& id,
+                                   content::WebContents* web_contents);
+
   // Called by the |overlay_media_notifications_manager_| when an overlay
   // notification is closed.
   void OnOverlayNotificationClosed(const std::string& id);
@@ -129,6 +135,9 @@ class MediaNotificationService
       const std::string& id,
       base::RepeatingCallback<void(bool)> callback);
 
+  void OnPresentationRequestCreated(
+      std::unique_ptr<media_router::StartPresentationContext> context);
+
   void OnStartPresentationContextCreated(
       std::unique_ptr<media_router::StartPresentationContext> context);
 
@@ -286,6 +295,12 @@ class MediaNotificationService
   base::WeakPtr<media_message_center::MediaNotificationItem>
   GetNotificationItem(const std::string& id);
 
+  // Called after changing anything about a notification to notify any observers
+  // and update the visibility of supplemental notifications.  If the change is
+  // associated with a particular notification ID, that ID should be passed as
+  // the argument, otherwise the argument should be nullptr.
+  void OnNotificationChanged(const std::string* changed_notification_id);
+
   // Looks up a Session object by its ID.  Returns null if not found.
   Session* GetSession(const std::string& id);
 
@@ -320,6 +335,14 @@ class MediaNotificationService
   // again (e.g. by playing the session).
   std::unordered_set<std::string> inactive_session_ids_;
 
+  // A mapping of supplemental notification IDs to their associated web
+  // contents.  See MediaNotificationController::AddSupplementalNotification for
+  // a description of supplemental notifications.
+  //
+  // This map should usually have at most one item.
+  base::flat_map<std::string, content::WebContents*>
+      supplemental_notifications_;
+
   // Stores a Session for each media session keyed by its |request_id| in string
   // format.
   std::map<std::string, Session> sessions_;

chrome/browser/ui/global_media_controls/media_notification_service_unittest.cc

@@ -347,6 +347,8 @@ class MediaNotificationServiceTest : public testing::Test {
 
   MockMediaNotificationServiceObserver& observer() { return observer_; }
 
+  MediaNotificationService* service() { return service_.get(); }
+
  private:
   content::BrowserTaskEnvironment task_environment_;
   MockMediaNotificationServiceObserver observer_;
@@ -408,6 +410,8 @@ TEST_F(MediaNotificationServiceTest, ShowControllableOnGainAndHideOnLoss) {
   EXPECT_TRUE(HasFrozenNotifications());
   testing::Mock::VerifyAndClearExpectations(&observer());
 
+  service()->ShowNotification(id.ToString());
+
   // Once the freeze timer fires, we should hide the media session.
   EXPECT_CALL(observer(), OnNotificationListChanged()).Times(AtLeast(1));
   EXPECT_CALL(dialog_delegate, HideMediaSession(id.ToString()));

components/media_message_center/media_notification_view_impl_unittest.cc

@@ -68,15 +68,16 @@ class MockMediaNotificationController : public MediaNotificationController {
   ~MockMediaNotificationController() = default;
 
   // MediaNotificationController implementation.
-  MOCK_METHOD1(ShowNotification, void(const std::string& id));
-  MOCK_METHOD1(HideNotification, void(const std::string& id));
-  MOCK_METHOD1(RemoveItem, void(const std::string& id));
+  MOCK_METHOD(void, ShowNotification, (const std::string& id));
+  MOCK_METHOD(void, HideNotification, (const std::string& id));
+  MOCK_METHOD(void, RemoveItem, (const std::string& id));
   scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() const override {
     return nullptr;
   }
-  MOCK_METHOD2(LogMediaSessionActionButtonPressed,
-               void(const std::string& id,
-                    media_session::mojom::MediaSessionAction action));
+  MOCK_METHOD(void,
+              LogMediaSessionActionButtonPressed,
+              (const std::string& id,
+               media_session::mojom::MediaSessionAction action));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockMediaNotificationController);