Enable GlobalMediaControlsForCast and CastMediaRouteProvider by default

The features were enabled via a config change in M87 (CastMRP on CrOS
will soon be enabled in M88). This change enables them by default.

Bug: 1042330, 869214
Change-Id: I111bd1f6d13bf70cf7557004205e7de1dc3e23ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2631191
Reviewed-by: Peter Kasting <[email protected]>
Reviewed-by: John Williams <[email protected]>
Reviewed-by: Mounir Lamouri <[email protected]>
Commit-Queue: Takumi Fujimoto <[email protected]>
Cr-Commit-Position: refs/heads/master@{#849910}

chrome/browser/media/router/media_router_feature.cc

@@ -33,18 +33,13 @@
 namespace media_router {
 
 #if !defined(OS_ANDROID)
-#if !defined(OFFICIAL_BUILD)
-// Enables the media router. Can be useful to disable for local
-// development on Mac because DIAL local discovery opens a local port
-// and triggers a permission prompt. Only toggleable for developer builds.
 const base::Feature kMediaRouter{"MediaRouter",
                                  base::FEATURE_ENABLED_BY_DEFAULT};
-#endif  // !defined(OFFICIAL_BUILD)
 // Controls if browser side DialMediaRouteProvider is enabled.
 const base::Feature kDialMediaRouteProvider{"DialMediaRouteProvider",
                                             base::FEATURE_ENABLED_BY_DEFAULT};
 const base::Feature kCastMediaRouteProvider{"CastMediaRouteProvider",
-                                            base::FEATURE_DISABLED_BY_DEFAULT};
+                                            base::FEATURE_ENABLED_BY_DEFAULT};
 const base::Feature kCastAllowAllIPsFeature{"CastAllowAllIPs",
                                             base::FEATURE_DISABLED_BY_DEFAULT};
 const base::Feature kGlobalMediaControlsCastStartStop{

chrome/browser/media/router/media_router_feature.h

@@ -21,11 +21,18 @@ bool MediaRouterEnabled(content::BrowserContext* context);
 
 #if !defined(OS_ANDROID)
 
+// Enables the media router. Can be disabled in tests unrelated to
+// Media Router where it interferes. Can also be useful to disable for local
+// development on Mac because DIAL local discovery opens a local port
+// and triggers a permission prompt.
+extern const base::Feature kMediaRouter;
+
 // TODO(crbug.com/1028753): Remove default-enabled kDialMediaRouteProvider after
 // tests stop disabling it.
 extern const base::Feature kDialMediaRouteProvider;
 
 extern const base::Feature kCastMediaRouteProvider;
+
 // If enabled, allows Media Router to connect to Cast devices on all IP
 // addresses, not just RFC1918/RFC4193 private addresses. Workaround for
 // https://crbug.com/813974.

chrome/browser/media/router/mojo/media_router_desktop_unittest.cc

@@ -64,11 +64,13 @@ class MediaRouterDesktopTest : public MediaRouterMojoTest {
  protected:
   std::unique_ptr<MediaRouterMojoImpl> CreateMediaRouter() override {
     std::unique_ptr<MockCastMediaSinkService> cast_media_sink_service;
-    // We disable the DIAL MRP because initializing the DIAL MRP requires
-    // initialization of objects it depends on, which is outside the scope of
-    // this unit test. DIAL MRP initialization is covered by Media Router
-    // browser tests.
-    feature_list_.InitAndDisableFeature(kDialMediaRouteProvider);
+    // We disable the DIAL and Cast MRPs because initializing the MRPs requires
+    // initialization of objects they depend on, which is outside the scope of
+    // this unit test. MRP initialization is covered by Media Router browser
+    // tests.
+    feature_list_.InitWithFeatures(
+        {}, /* disabled_features */ {kDialMediaRouteProvider,
+                                     kCastMediaRouteProvider});
     cast_media_sink_service = std::make_unique<MockCastMediaSinkService>();
     cast_media_sink_service_ = cast_media_sink_service.get();
     media_sink_service_ =

chrome/browser/ui/ash/media_notification_provider_impl_unittest.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/ash/media_notification_provider_impl.h"
 
 #include "ash/public/cpp/media_notification_provider_observer.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/unguessable_token.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
@@ -15,6 +16,7 @@
 #include "chrome/test/base/testing_profile_manager.h"
 #include "components/session_manager/core/session_manager.h"
 #include "content/public/test/browser_task_environment.h"
+#include "media/base/media_switches.h"
 #include "services/media_session/public/mojom/audio_focus.mojom.h"
 #include "services/media_session/public/mojom/media_session.mojom.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -51,6 +53,11 @@ class MediaNotificationProviderImplTest : public testing::Test {
 
   void SetUp() override {
     testing::Test::SetUp();
+
+    // Disable a feature unrelated to the unit test. The use of cast features
+    // requires setting up extra dependencies.
+    feature_list_.InitAndDisableFeature(media::kGlobalMediaControlsForCast);
+
     user_manager_->Initialize();
     CHECK(testing_profile_manager_.SetUp());
 
@@ -107,15 +114,16 @@ class MediaNotificationProviderImplTest : public testing::Test {
 
   MediaNotificationProviderImpl* provider() { return provider_.get(); }
 
-  content::BrowserTaskEnvironment browser_environment;
+  content::BrowserTaskEnvironment browser_environment_;
 
  private:
   session_manager::SessionManager session_manager_;
   chromeos::FakeChromeUserManager* user_manager_{
       new chromeos::FakeChromeUserManager()};
   TestingProfileManager testing_profile_manager_{
       TestingBrowserProcess::GetGlobal()};
-  views::LayoutProvider layout_provider;
+  views::LayoutProvider layout_provider_;
+  base::test::ScopedFeatureList feature_list_;
 
   std::unique_ptr<MockMediaNotificationProviderObserver> mock_observer_;
   std::unique_ptr<MediaNotificationProviderImpl> provider_;

chrome/browser/ui/global_media_controls/media_toolbar_button_controller_unittest.cc

@@ -7,7 +7,9 @@
 #include <memory>
 
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/unguessable_token.h"
+#include "chrome/browser/media/router/media_router_feature.h"
 #include "chrome/browser/ui/global_media_controls/media_dialog_delegate.h"
 #include "chrome/browser/ui/global_media_controls/media_notification_service.h"
 #include "chrome/browser/ui/global_media_controls/media_toolbar_button_controller_delegate.h"
@@ -87,13 +89,15 @@ class MediaToolbarButtonControllerTest : public testing::Test {
  public:
   MediaToolbarButtonControllerTest()
       : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME,
-                          base::test::TaskEnvironment::MainThreadType::UI),
-        service_(&profile_, false) {}
+                          base::test::TaskEnvironment::MainThreadType::UI) {}
   ~MediaToolbarButtonControllerTest() override = default;
 
   void SetUp() override {
-    controller_ =
-        std::make_unique<MediaToolbarButtonController>(&delegate_, &service_);
+    // Disable Media Router, which has many dependencies of its own.
+    feature_list_.InitAndDisableFeature(media_router::kMediaRouter);
+    service_ = std::make_unique<MediaNotificationService>(&profile_, false);
+    controller_ = std::make_unique<MediaToolbarButtonController>(
+        &delegate_, service_.get());
   }
 
   void TearDown() override { controller_.reset(); }
@@ -124,13 +128,13 @@ class MediaToolbarButtonControllerTest : public testing::Test {
 
   void SimulateFocusGained(const base::UnguessableToken& id,
                            bool controllable) {
-    service_.OnFocusGained(CreateFocusRequest(id, controllable));
+    service_->OnFocusGained(CreateFocusRequest(id, controllable));
   }
 
   void SimulateFocusLost(const base::UnguessableToken& id) {
     AudioFocusRequestStatePtr focus(AudioFocusRequestState::New());
     focus->request_id = id;
-    service_.OnFocusLost(std::move(focus));
+    service_->OnFocusLost(std::move(focus));
   }
 
   void SimulateNecessaryMetadata(const base::UnguessableToken& id) {
@@ -140,8 +144,8 @@ class MediaToolbarButtonControllerTest : public testing::Test {
     // service, but since the service doesn't run for this test, we'll manually
     // grab the MediaNotificationItem from the MediaNotificationService and
     // set the metadata.
-    auto item_itr = service_.sessions_.find(id.ToString());
-    ASSERT_NE(service_.sessions_.end(), item_itr);
+    auto item_itr = service_->sessions_.find(id.ToString());
+    ASSERT_NE(service_->sessions_.end(), item_itr);
 
     media_session::MediaMetadata metadata;
     metadata.title = base::ASCIIToUTF16("title");
@@ -150,7 +154,7 @@ class MediaToolbarButtonControllerTest : public testing::Test {
   }
 
   void SimulateDialogOpened(MockMediaDialogDelegate* delegate) {
-    delegate->Open(&service_);
+    delegate->Open(service_.get());
   }
 
   MockMediaToolbarButtonControllerDelegate& delegate() { return delegate_; }
@@ -159,8 +163,9 @@ class MediaToolbarButtonControllerTest : public testing::Test {
   content::BrowserTaskEnvironment task_environment_;
   MockMediaToolbarButtonControllerDelegate delegate_;
   TestingProfile profile_;
-  MediaNotificationService service_;
+  std::unique_ptr<MediaNotificationService> service_;
   std::unique_ptr<MediaToolbarButtonController> controller_;
+  base::test::ScopedFeatureList feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaToolbarButtonControllerTest);
 };

chrome/browser/ui/views/frame/test_with_browser_view.h

@@ -8,6 +8,8 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/media/router/media_router_feature.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 
 class BrowserView;
@@ -18,7 +20,11 @@ class BrowserView;
 class TestWithBrowserView : public BrowserWithTestWindowTest {
  public:
   template <typename... Args>
-  TestWithBrowserView(Args... args) : BrowserWithTestWindowTest(args...) {}
+  explicit TestWithBrowserView(Args... args)
+      : BrowserWithTestWindowTest(args...) {
+    // Media Router requires the IO thread, which doesn't exist in this setup.
+    feature_list_.InitAndDisableFeature(media_router::kMediaRouter);
+  }
 
   ~TestWithBrowserView() override;
 
@@ -33,6 +39,7 @@ class TestWithBrowserView : public BrowserWithTestWindowTest {
 
  private:
   BrowserView* browser_view_;  // Not owned.
+  base::test::ScopedFeatureList feature_list_;
 
   DISALLOW_COPY_AND_ASSIGN(TestWithBrowserView);
 };

media/base/media_switches.cc

@@ -381,7 +381,7 @@ const base::Feature kGlobalMediaControlsAutoDismiss{
 // Show Cast sessions in Global Media Controls. It is no-op if
 // kGlobalMediaControls is not enabled.
 const base::Feature kGlobalMediaControlsForCast{
-    "GlobalMediaControlsForCast", base::FEATURE_DISABLED_BY_DEFAULT};
+    "GlobalMediaControlsForCast", base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Allow Global Media Controls in system tray of CrOS.
 const base::Feature kGlobalMediaControlsForChromeOS{