[Zenith] Control other Cast sessions' visibility from the context menu

CastNotificationProducer::GetActiveControllableItems() now returns
active Cast notification items that represent Cast sessions (1) started
from the current computer, or (2) started from other devices and users
select to show other Cast sessions.

Also, MediaToolbarButtonContextMenu now notifies MediaItemManager when
the context menu is closed.

Bug: 1287332
Change-Id: I19ca903b3f7e5a8938722652472ce4b470324678
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3547062
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: Muyao Xu <muyaoxu@google.com>
Auto-Submit: Muyao Xu <muyaoxu@google.com>
Cr-Commit-Position: refs/heads/main@{#985576}

chrome/browser/ui/global_media_controls/cast_media_notification_item.cc

@@ -155,7 +155,7 @@ CastMediaNotificationItem::CastMediaNotificationItem(
       profile_(profile),
       session_controller_(std::move(session_controller)),
       media_route_id_(route.media_route_id()),
-      is_local_presentation_(route.is_local_presentation()),
+      route_is_local_(route.is_local()),
       image_downloader_(
           profile,
           base::BindRepeating(&CastMediaNotificationItem::ImageChanged,

chrome/browser/ui/global_media_controls/cast_media_notification_item.h

@@ -78,7 +78,7 @@ class CastMediaNotificationItem
   }
   Profile* profile() { return profile_; }
   bool is_active() const { return is_active_; }
-  bool is_local_presentation() const { return is_local_presentation_; }
+  bool route_is_local() const { return route_is_local_; }
 
   base::WeakPtr<CastMediaNotificationItem> GetWeakPtr() {
     return weak_ptr_factory_.GetWeakPtr();
@@ -139,7 +139,8 @@ class CastMediaNotificationItem
 
   std::unique_ptr<CastMediaSessionController> session_controller_;
   const media_router::MediaRoute::Id media_route_id_;
-  const bool is_local_presentation_;
+  // True if the route is started from the |profile_| on the current device.
+  const bool route_is_local_;
   ImageDownloader image_downloader_;
   media_session::MediaMetadata metadata_;
   std::vector<media_session::mojom::MediaSessionAction> actions_;

chrome/browser/ui/global_media_controls/cast_media_notification_producer.cc

@@ -14,10 +14,15 @@
 #include "components/media_message_center/media_notification_util.h"
 #include "components/media_router/browser/media_router.h"
 #include "components/media_router/browser/media_router_factory.h"
+#include "components/media_router/common/pref_names.h"
 #include "components/media_router/common/providers/cast/cast_media_source.h"
+#include "components/prefs/pref_service.h"
 
 namespace {
 
+// Returns false if a notification item shouldn't be created for |route|.
+// If a route should be hidden, it's not possible to create an item
+// for this route until the next time |OnModuleUpdated()| is called.
 bool ShouldHideNotification(const raw_ptr<Profile> profile,
                             const media_router::MediaRoute& route) {
   // TODO(crbug.com/1195382): Display multizone group route.
@@ -26,16 +31,18 @@ bool ShouldHideNotification(const raw_ptr<Profile> profile,
   }
 
   if (media_router::GlobalMediaControlsCastStartStopEnabled(profile)) {
-    // Hide a route if it's not for display or it's a mirroring route.
+    // Hide a route if it's a mirroring route.
     if (route.media_source().IsTabMirroringSource() ||
         route.media_source().IsDesktopMirroringSource() ||
         route.media_source().IsLocalFileSource())
       return true;
   } else if (route.controller_type() !=
              media_router::RouteControllerType::kGeneric) {
+    // Hide a route if it doesn't have a generic controller (play, pause etc.).
     return true;
   }
 
+  // Skip the multizone member check if it's a DIAL route.
   if (!route.media_source().IsCastPresentationUrl()) {
     return false;
   }
@@ -83,11 +90,30 @@ CastMediaNotificationProducer::GetMediaItem(const std::string& id) {
 }
 
 std::set<std::string>
-CastMediaNotificationProducer::GetActiveControllableItemIds() {
+CastMediaNotificationProducer::GetActiveControllableItemIds() const {
   std::set<std::string> ids;
   for (const auto& item : items_) {
-    if (item.second.is_active())
-      ids.insert(item.first);
+    if (!item.second.is_active())
+      continue;
+
+// kMediaRouterShowCastSessionsStartedByOtherDevices is not registered on
+// Android nor ChromeOS.
+// // TODO(crbug.com/1308053): Enable it on ChromeOS once Cast+GMC ships.
+#if !BUILDFLAG(IS_CHROMEOS)
+    // The non-local Cast session filter should not be put in
+    // |ShouldHideNotification()| because it's used to determine if an item
+    // should be created. It's possible that users later change the pref to
+    // show all Cast sessions.
+    if (media_router::GlobalMediaControlsCastStartStopEnabled(profile_) &&
+        !this->profile_->GetPrefs()->GetBoolean(
+            media_router::prefs::
+                kMediaRouterShowCastSessionsStartedByOtherDevices) &&
+        !item.second.route_is_local()) {
+      continue;
+    }
+#endif
+
+    ids.insert(item.first);
   }
   return ids;
 }
@@ -170,17 +196,15 @@ void CastMediaNotificationProducer::OnRoutesUpdated(
 }
 
 size_t CastMediaNotificationProducer::GetActiveItemCount() const {
-  return std::count_if(items_.begin(), items_.end(), [](const auto& item) {
-    return item.second.is_active();
-  });
+  return GetActiveControllableItemIds().size();
 }
 
 bool CastMediaNotificationProducer::HasActiveItems() const {
-  return GetActiveItemCount() != 0;
+  return !GetActiveControllableItemIds().empty();
 }
 
 bool CastMediaNotificationProducer::HasLocalMediaRoute() const {
   return std::find_if(items_.begin(), items_.end(), [](const auto& item) {
-           return item.second.is_local_presentation();
+           return item.second.route_is_local();
          }) != items_.end();
 }

chrome/browser/ui/global_media_controls/cast_media_notification_producer.h

@@ -48,7 +48,7 @@ class CastMediaNotificationProducer
   // global_media_controls::MediaItemProducer:
   base::WeakPtr<media_message_center::MediaNotificationItem> GetMediaItem(
       const std::string& id) override;
-  std::set<std::string> GetActiveControllableItemIds() override;
+  std::set<std::string> GetActiveControllableItemIds() const override;
   bool HasFrozenItems() override;
   void OnItemShown(const std::string& id,
                    global_media_controls::MediaItemUI* item_ui) override;

chrome/browser/ui/global_media_controls/cast_media_notification_producer_unittest.cc

@@ -13,6 +13,8 @@
 #include "components/media_message_center/mock_media_notification_view.h"
 #include "components/media_router/browser/test/mock_media_router.h"
 #include "components/media_router/common/media_route.h"
+#include "components/media_router/common/pref_names.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -51,6 +53,8 @@ class CastMediaNotificationProducerTest : public testing::Test {
 
   void TearDown() override { notification_producer_.reset(); }
 
+  TestingProfile* profile() { return &profile_; }
+
  protected:
   content::BrowserTaskEnvironment task_environment_;
   TestingProfile profile_;
@@ -141,4 +145,21 @@ TEST_F(CastMediaNotificationProducerTest, RoutesWithoutNotifications) {
       {mirroring_route, multizone_member_route, connecting_route});
   EXPECT_EQ(0u, notification_producer_->GetActiveItemCount());
 }
+
+TEST_F(CastMediaNotificationProducerTest, NonLocalRoutesWithoutNotifications) {
+  MediaRoute non_local_route = CreateRoute("non-local-route");
+  non_local_route.set_local(false);
+  sync_preferences::TestingPrefServiceSyncable* pref_service =
+      profile()->GetTestingPrefService();
+
+  notification_producer_->OnRoutesUpdated({non_local_route});
+  EXPECT_EQ(1u, notification_producer_->GetActiveItemCount());
+
+  // There is no need to call |OnRouteUpdated()| here because this is a
+  // client-side change.
+  pref_service->SetBoolean(
+      media_router::prefs::kMediaRouterShowCastSessionsStartedByOtherDevices,
+      false);
+  EXPECT_EQ(0u, notification_producer_->GetActiveItemCount());
+}
 #endif  // BUILDFLAG(IS_CHROMEOS)

chrome/browser/ui/global_media_controls/presentation_request_notification_producer.cc

@@ -100,7 +100,7 @@ PresentationRequestNotificationProducer::GetMediaItem(const std::string& id) {
 }
 
 std::set<std::string>
-PresentationRequestNotificationProducer::GetActiveControllableItemIds() {
+PresentationRequestNotificationProducer::GetActiveControllableItemIds() const {
   return (item_ && !should_hide_) ? std::set<std::string>({item_->id()})
                                   : std::set<std::string>();
 }

chrome/browser/ui/global_media_controls/presentation_request_notification_producer.h

@@ -65,7 +65,7 @@ class PresentationRequestNotificationProducer final
   base::WeakPtr<media_message_center::MediaNotificationItem> GetMediaItem(
       const std::string& id) override;
   // Returns the supplemental notification's id if it should be shown.
-  std::set<std::string> GetActiveControllableItemIds() override;
+  std::set<std::string> GetActiveControllableItemIds() const override;
   bool HasFrozenItems() override;
   void OnItemShown(const std::string& id,
                    global_media_controls::MediaItemUI* item_ui) override;

chrome/browser/ui/views/global_media_controls/media_toolbar_button_contextual_menu.cc

@@ -9,13 +9,24 @@
 #include "chrome/browser/media/router/media_router_feature.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/global_media_controls/media_notification_service.h"
+#include "chrome/browser/ui/global_media_controls/media_notification_service_factory.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/global_media_controls/public/media_item_manager.h"
 #include "components/media_router/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 
+namespace {
+global_media_controls::MediaItemManager* GetItemManagerFromBrowser(
+    Browser* browser) {
+  return MediaNotificationServiceFactory::GetForProfile(browser->profile())
+      ->media_item_manager();
+}
+}  // namespace
+
 std::unique_ptr<MediaToolbarButtonContextualMenu>
 MediaToolbarButtonContextualMenu::Create(Browser* browser) {
   if (media_router::GlobalMediaControlsCastStartStopEnabled(
@@ -27,7 +38,7 @@ MediaToolbarButtonContextualMenu::Create(Browser* browser) {
 
 MediaToolbarButtonContextualMenu::MediaToolbarButtonContextualMenu(
     Browser* browser)
-    : browser_(browser) {}
+    : browser_(browser), item_manager_(GetItemManagerFromBrowser(browser_)) {}
 
 MediaToolbarButtonContextualMenu::~MediaToolbarButtonContextualMenu() = default;
 
@@ -79,6 +90,12 @@ void MediaToolbarButtonContextualMenu::ExecuteCommand(int command_id,
   }
 }
 
+void MediaToolbarButtonContextualMenu::MenuClosed(ui::SimpleMenuModel* source) {
+  if (item_manager_) {
+    item_manager_->OnItemsChanged();
+  }
+}
+
 void MediaToolbarButtonContextualMenu::ToggleShowOtherSessions() {
   PrefService* pref_service = browser_->profile()->GetPrefs();
   pref_service->SetBoolean(

chrome/browser/ui/views/global_media_controls/media_toolbar_button_contextual_menu.h

@@ -10,6 +10,9 @@
 #include "ui/base/models/simple_menu_model.h"
 
 class Browser;
+namespace global_media_controls {
+class MediaItemManager;
+}
 
 // The contextual menu of the media toolbar button has two items, both of which
 // are related to Cast. So this class should be instantiated only when
@@ -33,6 +36,7 @@ class MediaToolbarButtonContextualMenu : public ui::SimpleMenuModel::Delegate {
   // ui::SimpleMenuModel::Delegate:
   bool IsCommandIdChecked(int command_id) const override;
   void ExecuteCommand(int command_id, int event_flags) override;
+  void MenuClosed(ui::SimpleMenuModel* source) override;
 
   void ToggleShowOtherSessions();
 
@@ -42,5 +46,6 @@ class MediaToolbarButtonContextualMenu : public ui::SimpleMenuModel::Delegate {
 #endif  // BUILDFLAG(GOOGLE_CHROME_BRANDING)
 
   const raw_ptr<Browser> browser_;
+  const raw_ptr<global_media_controls::MediaItemManager> item_manager_;
 };
 #endif  // CHROME_BROWSER_UI_VIEWS_GLOBAL_MEDIA_CONTROLS_MEDIA_TOOLBAR_BUTTON_CONTEXTUAL_MENU_H_

components/global_media_controls/public/media_item_producer.h

@@ -25,7 +25,7 @@ class MediaItemProducer {
   virtual base::WeakPtr<media_message_center::MediaNotificationItem>
   GetMediaItem(const std::string& id) = 0;
 
-  virtual std::set<std::string> GetActiveControllableItemIds() = 0;
+  virtual std::set<std::string> GetActiveControllableItemIds() const = 0;
 
   // Returns true if the item producer has any "frozen" items, which are items
   // that were recently active with a chance to become active again.

components/global_media_controls/public/media_session_item_producer.cc

@@ -241,7 +241,8 @@ MediaSessionItemProducer::GetMediaItem(const std::string& id) {
   return it == sessions_.end() ? nullptr : it->second.item()->GetWeakPtr();
 }
 
-std::set<std::string> MediaSessionItemProducer::GetActiveControllableItemIds() {
+std::set<std::string> MediaSessionItemProducer::GetActiveControllableItemIds()
+    const {
   return active_controllable_session_ids_;
 }
 

components/global_media_controls/public/media_session_item_producer.h

@@ -56,7 +56,7 @@ class COMPONENT_EXPORT(GLOBAL_MEDIA_CONTROLS) MediaSessionItemProducer
   // MediaItemProducer:
   base::WeakPtr<media_message_center::MediaNotificationItem> GetMediaItem(
       const std::string& id) override;
-  std::set<std::string> GetActiveControllableItemIds() override;
+  std::set<std::string> GetActiveControllableItemIds() const override;
   bool HasFrozenItems() override;
   void OnItemShown(const std::string& id, MediaItemUI* item_ui) override;
   bool IsItemActivelyPlaying(const std::string& id) override;

components/global_media_controls/public/test/mock_media_item_producer.cc

@@ -51,7 +51,8 @@ MockMediaItemProducer::GetMediaItem(const std::string& id) {
   return iter->second.item.GetWeakPtr();
 }
 
-std::set<std::string> MockMediaItemProducer::GetActiveControllableItemIds() {
+std::set<std::string> MockMediaItemProducer::GetActiveControllableItemIds()
+    const {
   std::set<std::string> active_items;
   for (auto const& item_pair : items_) {
     if (item_pair.second.active)

components/global_media_controls/public/test/mock_media_item_producer.h

@@ -25,7 +25,7 @@ class MockMediaItemProducer : public MediaItemProducer {
 
   base::WeakPtr<media_message_center::MediaNotificationItem> GetMediaItem(
       const std::string& id) override;
-  std::set<std::string> GetActiveControllableItemIds() override;
+  std::set<std::string> GetActiveControllableItemIds() const override;
   bool HasFrozenItems() override;
   MOCK_METHOD(void, OnItemShown, (const std::string&, MediaItemUI*));
   MOCK_METHOD(void, OnDialogDisplayed, ());

components/media_router/common/media_route.h

@@ -143,6 +143,7 @@ class MediaRoute {
 
   // |true| if the presentation associated with this route is a local
   // presentation.
+  // TODO(crbug.com/1309770): Remove |is_local_presentation_|.
   bool is_local_presentation_ = false;
 
   // |true| if the route is created by the MRP but is waiting for receivers'