diff --git a/chrome/browser/ui/global_media_controls/cast_media_notification_item.cc b/chrome/browser/ui/global_media_controls/cast_media_notification_item.cc index 97c4511897245d..a8d4d4999414af 100644 --- a/chrome/browser/ui/global_media_controls/cast_media_notification_item.cc +++ b/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, diff --git a/chrome/browser/ui/global_media_controls/cast_media_notification_item.h b/chrome/browser/ui/global_media_controls/cast_media_notification_item.h index c2290cfe7966ce..6734ada4a699f7 100644 --- a/chrome/browser/ui/global_media_controls/cast_media_notification_item.h +++ b/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 GetWeakPtr() { return weak_ptr_factory_.GetWeakPtr(); @@ -139,7 +139,8 @@ class CastMediaNotificationItem std::unique_ptr 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 actions_; diff --git a/chrome/browser/ui/global_media_controls/cast_media_notification_producer.cc b/chrome/browser/ui/global_media_controls/cast_media_notification_producer.cc index 9432c69285e795..d90b2f3f047a32 100644 --- a/chrome/browser/ui/global_media_controls/cast_media_notification_producer.cc +++ b/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, const media_router::MediaRoute& route) { // TODO(crbug.com/1195382): Display multizone group route. @@ -26,16 +31,18 @@ bool ShouldHideNotification(const raw_ptr 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 -CastMediaNotificationProducer::GetActiveControllableItemIds() { +CastMediaNotificationProducer::GetActiveControllableItemIds() const { std::set 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(); } diff --git a/chrome/browser/ui/global_media_controls/cast_media_notification_producer.h b/chrome/browser/ui/global_media_controls/cast_media_notification_producer.h index fb57b5c4800bbd..687f5ca2899a61 100644 --- a/chrome/browser/ui/global_media_controls/cast_media_notification_producer.h +++ b/chrome/browser/ui/global_media_controls/cast_media_notification_producer.h @@ -48,7 +48,7 @@ class CastMediaNotificationProducer // global_media_controls::MediaItemProducer: base::WeakPtr GetMediaItem( const std::string& id) override; - std::set GetActiveControllableItemIds() override; + std::set GetActiveControllableItemIds() const override; bool HasFrozenItems() override; void OnItemShown(const std::string& id, global_media_controls::MediaItemUI* item_ui) override; diff --git a/chrome/browser/ui/global_media_controls/cast_media_notification_producer_unittest.cc b/chrome/browser/ui/global_media_controls/cast_media_notification_producer_unittest.cc index 817d86bfa8a822..1ac8de49a28076 100644 --- a/chrome/browser/ui/global_media_controls/cast_media_notification_producer_unittest.cc +++ b/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) diff --git a/chrome/browser/ui/global_media_controls/presentation_request_notification_producer.cc b/chrome/browser/ui/global_media_controls/presentation_request_notification_producer.cc index 778d85eb7c80e4..422c255ddd29c0 100644 --- a/chrome/browser/ui/global_media_controls/presentation_request_notification_producer.cc +++ b/chrome/browser/ui/global_media_controls/presentation_request_notification_producer.cc @@ -100,7 +100,7 @@ PresentationRequestNotificationProducer::GetMediaItem(const std::string& id) { } std::set -PresentationRequestNotificationProducer::GetActiveControllableItemIds() { +PresentationRequestNotificationProducer::GetActiveControllableItemIds() const { return (item_ && !should_hide_) ? std::set({item_->id()}) : std::set(); } diff --git a/chrome/browser/ui/global_media_controls/presentation_request_notification_producer.h b/chrome/browser/ui/global_media_controls/presentation_request_notification_producer.h index 6d898f9f4ac07e..af18401d907e98 100644 --- a/chrome/browser/ui/global_media_controls/presentation_request_notification_producer.h +++ b/chrome/browser/ui/global_media_controls/presentation_request_notification_producer.h @@ -65,7 +65,7 @@ class PresentationRequestNotificationProducer final base::WeakPtr GetMediaItem( const std::string& id) override; // Returns the supplemental notification's id if it should be shown. - std::set GetActiveControllableItemIds() override; + std::set GetActiveControllableItemIds() const override; bool HasFrozenItems() override; void OnItemShown(const std::string& id, global_media_controls::MediaItemUI* item_ui) override; diff --git a/chrome/browser/ui/views/global_media_controls/media_toolbar_button_contextual_menu.cc b/chrome/browser/ui/views/global_media_controls/media_toolbar_button_contextual_menu.cc index d934d3d04b028b..f093a6ff6abb39 100644 --- a/chrome/browser/ui/views/global_media_controls/media_toolbar_button_contextual_menu.cc +++ b/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::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( diff --git a/chrome/browser/ui/views/global_media_controls/media_toolbar_button_contextual_menu.h b/chrome/browser/ui/views/global_media_controls/media_toolbar_button_contextual_menu.h index 14e773071851e9..384643ebe799c6 100644 --- a/chrome/browser/ui/views/global_media_controls/media_toolbar_button_contextual_menu.h +++ b/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_; + const raw_ptr item_manager_; }; #endif // CHROME_BROWSER_UI_VIEWS_GLOBAL_MEDIA_CONTROLS_MEDIA_TOOLBAR_BUTTON_CONTEXTUAL_MENU_H_ diff --git a/components/global_media_controls/public/media_item_producer.h b/components/global_media_controls/public/media_item_producer.h index b97b2f7f5d684f..6c21532cc13522 100644 --- a/components/global_media_controls/public/media_item_producer.h +++ b/components/global_media_controls/public/media_item_producer.h @@ -25,7 +25,7 @@ class MediaItemProducer { virtual base::WeakPtr GetMediaItem(const std::string& id) = 0; - virtual std::set GetActiveControllableItemIds() = 0; + virtual std::set 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. diff --git a/components/global_media_controls/public/media_session_item_producer.cc b/components/global_media_controls/public/media_session_item_producer.cc index 017bd45b251378..bb142c7cac3e0c 100644 --- a/components/global_media_controls/public/media_session_item_producer.cc +++ b/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 MediaSessionItemProducer::GetActiveControllableItemIds() { +std::set MediaSessionItemProducer::GetActiveControllableItemIds() + const { return active_controllable_session_ids_; } diff --git a/components/global_media_controls/public/media_session_item_producer.h b/components/global_media_controls/public/media_session_item_producer.h index 840583cad8a514..c47b07197a4f32 100644 --- a/components/global_media_controls/public/media_session_item_producer.h +++ b/components/global_media_controls/public/media_session_item_producer.h @@ -56,7 +56,7 @@ class COMPONENT_EXPORT(GLOBAL_MEDIA_CONTROLS) MediaSessionItemProducer // MediaItemProducer: base::WeakPtr GetMediaItem( const std::string& id) override; - std::set GetActiveControllableItemIds() override; + std::set GetActiveControllableItemIds() const override; bool HasFrozenItems() override; void OnItemShown(const std::string& id, MediaItemUI* item_ui) override; bool IsItemActivelyPlaying(const std::string& id) override; diff --git a/components/global_media_controls/public/test/mock_media_item_producer.cc b/components/global_media_controls/public/test/mock_media_item_producer.cc index 276ccd5a60fecc..920853ba47061e 100644 --- a/components/global_media_controls/public/test/mock_media_item_producer.cc +++ b/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 MockMediaItemProducer::GetActiveControllableItemIds() { +std::set MockMediaItemProducer::GetActiveControllableItemIds() + const { std::set active_items; for (auto const& item_pair : items_) { if (item_pair.second.active) diff --git a/components/global_media_controls/public/test/mock_media_item_producer.h b/components/global_media_controls/public/test/mock_media_item_producer.h index cd61f354ac7282..da5de5097917d8 100644 --- a/components/global_media_controls/public/test/mock_media_item_producer.h +++ b/components/global_media_controls/public/test/mock_media_item_producer.h @@ -25,7 +25,7 @@ class MockMediaItemProducer : public MediaItemProducer { base::WeakPtr GetMediaItem( const std::string& id) override; - std::set GetActiveControllableItemIds() override; + std::set GetActiveControllableItemIds() const override; bool HasFrozenItems() override; MOCK_METHOD(void, OnItemShown, (const std::string&, MediaItemUI*)); MOCK_METHOD(void, OnDialogDisplayed, ()); diff --git a/components/media_router/common/media_route.h b/components/media_router/common/media_route.h index 3f3f0ac378af4f..4a9a16691b02da 100644 --- a/components/media_router/common/media_route.h +++ b/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'