Add a feature to control cast mirroring playout delay.

This change adds a feature kCastMirroringPlayoutDelay, with a
corresponding param kCastMirroringPlayoutDelayMs, which controls the
target playout delay during cast mirroring sessions, as long as the
playout delay has not been set by a site-initiated mirroring source.

We are adding this feature to run an experiment to test whether the
current default playout delay may be lowered without harshly degrading
the current mirroring experience.

Previously, we wanted to run this experiment only on AccessCodeCast
users, and thus previous patchsets included a new policy instead of a
new feature.

Bug: b:291616456
Change-Id: Ic61041007719226254df678b914b14ae16ffe3d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4725242
Commit-Queue: Benjamin Zielinski <bzielinski@google.com>
Reviewed-by: Mark Foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1186725}

chrome/browser/media/cast_mirroring_service_host.cc

@@ -20,10 +20,12 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/media/cast_remoting_connector.h"
 #include "chrome/browser/media/router/discovery/access_code/access_code_cast_feature.h"
+#include "chrome/browser/media/router/media_router_feature.h"
 #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
 #include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/tab_sharing/tab_sharing_ui.h"
 #include "components/access_code_cast/common/access_code_cast_metrics.h"
 #include "components/mirroring/browser/single_client_video_capture_host.h"
@@ -147,14 +149,21 @@ content::WebContents* GetContents(
                                        id.main_render_frame_id));
 }
 
+// Gets the profile associated with `web_contents`, if it exists. Else, gets the
+// last used profile if it is loaded.
+Profile* GetProfileOrLastUsedProfile(content::WebContents* web_contents) {
+  if (web_contents) {
+    return Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  }
+  return ProfileManager::GetLastUsedProfileIfLoaded();
+}
+
 // Returns true if this user is allowed to use Access Codes & QR codes to
 // discover cast devices, and AccessCodeCastTabSwitchingUI flag is enabled.
 bool IsAccessCodeCastTabSwitchingUIEnabled(
     const content::WebContentsMediaCaptureId& id) {
-  auto* web_contents = GetContents(id);
-  return web_contents &&
-         media_router::IsAccessCodeCastTabSwitchingUiEnabled(
-             Profile::FromBrowserContext(web_contents->GetBrowserContext()));
+  Profile* profile = GetProfileOrLastUsedProfile(GetContents(id));
+  return media_router::IsAccessCodeCastTabSwitchingUiEnabled(profile);
 }
 
 // Returns the size of the primary display in pixels, or absl::nullopt if it
@@ -200,6 +209,13 @@ void CastMirroringServiceHost::Start(
     return;
   }
 
+  // If the target playout delay has not yet been set (from site-initiated
+  // mirroring request) then try to set it from a feature or commandline.
+  if (!session_params->target_playout_delay) {
+    session_params->target_playout_delay =
+        media_router::GetCastMirroringPlayoutDelay();
+  }
+
   // Although the base::Features get propagated to the mirroring service, the
   // command line flags do not.
   const base::CommandLine& command_line =

chrome/browser/media/cast_mirroring_service_host_browsertest.cc

@@ -6,12 +6,15 @@
 
 #include "base/containers/contains.h"
 #include "base/containers/flat_map.h"
+#include "base/json/json_reader.h"
 #include "base/memory/raw_ptr.h"
 #include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/task/bind_post_task.h"
 #include "base/test/test_timeouts.h"
 #include "build/build_config.h"
 #include "chrome/browser/media/router/discovery/access_code/access_code_cast_feature.h"
+#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/tabs/tab_enums.h"
@@ -20,6 +23,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/browser/ui/tabs/tab_utils.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "components/media_router/common/pref_names.h"
 #include "components/mirroring/mojom/cast_message_channel.mojom.h"
 #include "components/mirroring/mojom/session_observer.mojom.h"
 #include "components/mirroring/mojom/session_parameters.mojom.h"
@@ -148,6 +152,19 @@ class MockVideoCaptureObserver final
   const base::UnguessableToken session_id_ = base::UnguessableToken::Create();
 };
 
+class MockCastMessageChannel : public mojom::CastMessageChannel {
+ public:
+  // mojom::CastMessageChannel mock implementation (outbound messages).
+  MOCK_METHOD(void, OnMessage, (mojom::CastMessagePtr));
+
+  mojo::Receiver<mojom::CastMessageChannel>* GetChannelReceiver() {
+    return &channel_receiver_;
+  }
+
+ private:
+  mojo::Receiver<mojom::CastMessageChannel> channel_receiver_{this};
+};
+
 }  // namespace
 
 class CastMirroringServiceHostBrowserTest
@@ -176,13 +193,74 @@ class CastMirroringServiceHostBrowserTest
     mojo::PendingRemote<mojom::SessionObserver> observer;
     observer_receiver_.Bind(observer.InitWithNewPipeAndPassReceiver());
     mojo::PendingRemote<mojom::CastMessageChannel> outbound_channel;
-    outbound_channel_receiver_.Bind(
+    outbound_channel_receiver_ = std::make_unique<MockCastMessageChannel>();
+    outbound_channel_receiver_->GetChannelReceiver()->Bind(
+        outbound_channel.InitWithNewPipeAndPassReceiver());
+    auto session_params = mojom::SessionParameters::New();
+    session_params->source_id = "SourceID";
+    host_->Start(std::move(session_params), std::move(observer),
+                 std::move(outbound_channel),
+                 inbound_channel_.BindNewPipeAndPassReceiver(), "Sink Name");
+  }
+
+  // Starts a tab mirroring session, and sets a target playout delay.
+  // `media_source_delay` simulates the mirroring delay that is set from a media
+  // source when starting a mirroring session, i.e. set by a site-initiated
+  // mirroring source. `feature_delay` is the value that media_router_feature's
+  // `GetCastMirroringPlayoutDelay` is expected to return.
+  void StartTabMirroringWithTargetPlayoutDelay(
+      base::TimeDelta media_source_delay,
+      base::TimeDelta feature_delay) {
+    int expected_delay_ms = !media_source_delay.is_zero()
+                                ? media_source_delay.InMilliseconds()
+                                : feature_delay.InMilliseconds();
+
+    content::WebContents* web_contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    ASSERT_TRUE(web_contents);
+    host_ = std::make_unique<CastMirroringServiceHost>(
+        BuildMediaIdForTabMirroring(web_contents));
+    mojo::PendingRemote<mojom::SessionObserver> observer;
+    observer_receiver_.Bind(observer.InitWithNewPipeAndPassReceiver());
+    mojo::PendingRemote<mojom::CastMessageChannel> outbound_channel;
+    outbound_channel_receiver_ = std::make_unique<MockCastMessageChannel>();
+    outbound_channel_receiver_->GetChannelReceiver()->Bind(
         outbound_channel.InitWithNewPipeAndPassReceiver());
     auto session_params = mojom::SessionParameters::New();
     session_params->source_id = "SourceID";
+    if (!media_source_delay.is_zero()) {
+      session_params->target_playout_delay = media_source_delay;
+    }
+
+    base::RunLoop run_loop;
+    EXPECT_CALL(*outbound_channel_receiver_, OnMessage(_))
+        .WillOnce(
+            testing::Invoke([expected_delay_ms, &run_loop](
+                                mirroring::mojom::CastMessagePtr message) {
+              const absl::optional<base::Value> root_or_error =
+                  base::JSONReader::Read(message->json_format_data);
+              ASSERT_TRUE(root_or_error);
+              const base::Value::Dict& root = root_or_error->GetDict();
+              const std::string* type = root.FindString("type");
+              ASSERT_TRUE(type);
+              if (*type == "OFFER") {
+                const base::Value::Dict* offer = root.FindDict("offer");
+                EXPECT_TRUE(offer);
+                const base::Value::List* streams =
+                    offer->FindList("supportedStreams");
+                for (auto& stream : *streams) {
+                  const base::Value::Dict& stream_dict = stream.GetDict();
+                  const int stream_target_delay =
+                      stream_dict.FindInt("targetDelay").value();
+                  EXPECT_EQ(stream_target_delay, expected_delay_ms);
+                }
+              }
+              run_loop.Quit();
+            }));
     host_->Start(std::move(session_params), std::move(observer),
                  std::move(outbound_channel),
                  inbound_channel_.BindNewPipeAndPassReceiver(), "Sink Name");
+    run_loop.Run();
   }
 
   void EnableAccessCodeCast() {
@@ -305,12 +383,12 @@ class CastMirroringServiceHostBrowserTest
   }
 
   mojo::Receiver<mojom::SessionObserver> observer_receiver_{this};
-  mojo::Receiver<mojom::CastMessageChannel> outbound_channel_receiver_{this};
   mojo::Receiver<mojom::AudioStreamCreatorClient> audio_client_receiver_{this};
   mojo::Remote<mojom::CastMessageChannel> inbound_channel_;
 
   std::unique_ptr<CastMirroringServiceHost> host_;
   std::unique_ptr<MockVideoCaptureObserver> video_frame_receiver_;
+  std::unique_ptr<MockCastMessageChannel> outbound_channel_receiver_;
 };
 
 IN_PROC_BROWSER_TEST_F(CastMirroringServiceHostBrowserTest, CaptureTabVideo) {
@@ -388,6 +466,16 @@ IN_PROC_BROWSER_TEST_F(CastMirroringServiceHostBrowserTest, PauseSession) {
   StopMirroring();
 }
 
+IN_PROC_BROWSER_TEST_F(CastMirroringServiceHostBrowserTest,
+                       TabMirrorWithPresetPlayoutDelay) {
+  StartTabMirroringWithTargetPlayoutDelay(base::Milliseconds(200),
+                                          base::Milliseconds(0));
+  GetVideoCaptureHost();
+  StartVideoCapturing();
+  RequestRefreshFrame();
+  StopMirroring();
+}
+
 class CastMirroringServiceHostBrowserTestTabSwitcher
     : public CastMirroringServiceHostBrowserTest,
       public ::testing::WithParamInterface<bool> {
@@ -447,4 +535,70 @@ IN_PROC_BROWSER_TEST_P(CastMirroringServiceHostBrowserTestTabSwitcher,
   StopMirroring();
 }
 
+// Tests for which a target delay is set in the feature
+// kCastMirroringPlayoutDelayMs.
+class CastMirroringServiceHostBrowserTestTargetDelay
+    : public CastMirroringServiceHostBrowserTest {
+ public:
+  // A value to set for playout delay which is different than the default value
+  // used by casting code.
+  const int kFeaturePlayoutDelay500ms = 500;
+
+  CastMirroringServiceHostBrowserTestTargetDelay() {
+    feature_list_.Reset();
+    base::FieldTrialParams feature_params;
+    feature_params[media_router::kCastMirroringPlayoutDelayMs.name] =
+        base::NumberToString(kFeaturePlayoutDelay500ms);
+    feature_list_.InitAndEnableFeatureWithParameters(
+        media_router::kCastMirroringPlayoutDelay, feature_params);
+  }
+
+  CastMirroringServiceHostBrowserTestTargetDelay(
+      const CastMirroringServiceHostBrowserTestTargetDelay&) = delete;
+  CastMirroringServiceHostBrowserTestTargetDelay& operator=(
+      const CastMirroringServiceHostBrowserTestTargetDelay&) = delete;
+
+  ~CastMirroringServiceHostBrowserTestTargetDelay() override = default;
+
+  void VerifyEnabledFeatureParam() {
+    ASSERT_TRUE(
+        base::FeatureList::IsEnabled(media_router::kCastMirroringPlayoutDelay));
+
+    ASSERT_EQ(media_router::kCastMirroringPlayoutDelayMs.Get(),
+              kFeaturePlayoutDelay500ms);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Test that the target playout delay set in the kCastMirroringPlayoutDelay
+// feature param is used when starting the session, if there is no preset delay
+// from a media source.
+IN_PROC_BROWSER_TEST_F(CastMirroringServiceHostBrowserTestTargetDelay,
+                       TabMirrorWithPlayoutDelay) {
+  VerifyEnabledFeatureParam();
+  StartTabMirroringWithTargetPlayoutDelay(
+      /* media_source_delay = */ base::Milliseconds(0),
+      /*feature_delay = */ base::Milliseconds(kFeaturePlayoutDelay500ms));
+  GetVideoCaptureHost();
+  StartVideoCapturing();
+  RequestRefreshFrame();
+  StopMirroring();
+}
+
+// Test that a playout delay set by a media source overrides a delay set in the
+// kCastMirroringPlayoutDelay feature param.
+IN_PROC_BROWSER_TEST_F(CastMirroringServiceHostBrowserTestTargetDelay,
+                       TabMirrorWithPresetPlayoutDelay) {
+  VerifyEnabledFeatureParam();
+  StartTabMirroringWithTargetPlayoutDelay(
+      /* media_source_delay = */ base::Milliseconds(200),
+      /* feature_delay = */ base::Milliseconds(kFeaturePlayoutDelay500ms));
+  GetVideoCaptureHost();
+  StartVideoCapturing();
+  RequestRefreshFrame();
+  StopMirroring();
+}
+
 }  // namespace mirroring

chrome/browser/media/router/media_router_feature.cc

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/base64.h"
+#include "base/command_line.h"
 #include "base/containers/flat_map.h"
 #include "base/feature_list.h"
 #include "base/no_destructor.h"
@@ -21,6 +22,7 @@
 #include "components/prefs/pref_service.h"
 #include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/content_features.h"
 #include "crypto/random.h"
 #include "media/base/media_switches.h"
@@ -64,6 +66,11 @@ BASE_FEATURE(kFallbackToAudioTabMirroring,
 BASE_FEATURE(kCastDialogStopButton,
              "CastDialogStopButton",
              base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kCastMirroringPlayoutDelay,
+             "CastMirroringPlayoutDelay",
+             base::FEATURE_DISABLED_BY_DEFAULT);
+const base::FeatureParam<int> kCastMirroringPlayoutDelayMs{
+    &kCastMirroringPlayoutDelay, "cast_mirroring_playout_delay_ms", -1};
 #if BUILDFLAG(IS_CHROMEOS)
 BASE_FEATURE(kGlobalMediaControlsCastStartStop,
              "GlobalMediaControlsCastStartStop",
@@ -88,6 +95,13 @@ base::flat_map<content::BrowserContext*, bool>& GetStoredPrefValues() {
 
   return *stored_pref_values;
 }
+
+#if !BUILDFLAG(IS_ANDROID)
+// TODO(mfoltz): Add full implementation for validating playout delay value.
+bool IsValidMirroringPlayoutDelayMs(int delay_ms) {
+  return delay_ms <= 1000 && delay_ms >= 1;
+}
+#endif  // !BUILDFLAG(IS_ANDROID)
 }  // namespace
 
 void ClearMediaRouterStoredPrefsForTesting() {
@@ -186,6 +200,32 @@ bool GlobalMediaControlsCastStartStopEnabled(content::BrowserContext* context) {
          MediaRouterEnabled(context);
 }
 
+absl::optional<base::TimeDelta> GetCastMirroringPlayoutDelay() {
+  absl::optional<base::TimeDelta> target_playout_delay;
+
+  // First see if there is a command line switch for mirroring playout delay.
+  // Otherwise, check the relevant feature.
+  const base::CommandLine* cl = base::CommandLine::ForCurrentProcess();
+  if (cl->HasSwitch(switches::kCastMirroringTargetPlayoutDelay)) {
+    int switch_playout_delay = 0;
+    if (base::StringToInt(
+            cl->GetSwitchValueASCII(switches::kCastMirroringTargetPlayoutDelay),
+            &switch_playout_delay) &&
+        IsValidMirroringPlayoutDelayMs(switch_playout_delay)) {
+      target_playout_delay = base::Milliseconds(switch_playout_delay);
+    }
+  }
+
+  if (!target_playout_delay.has_value() &&
+      base::FeatureList::IsEnabled(kCastMirroringPlayoutDelay) &&
+      IsValidMirroringPlayoutDelayMs(kCastMirroringPlayoutDelayMs.Get())) {
+    target_playout_delay =
+        base::Milliseconds(kCastMirroringPlayoutDelayMs.Get());
+  }
+
+  return target_playout_delay;
+}
+
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 }  // namespace media_router

chrome/browser/media/router/media_router_feature.h

@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_FEATURE_H_
 
 #include "base/feature_list.h"
+#include "base/metrics/field_trial_params.h"
+#include "base/time/time.h"
 #include "build/build_config.h"
 
 class PrefRegistrySimple;
@@ -69,6 +71,11 @@ BASE_DECLARE_FEATURE(kFallbackToAudioTabMirroring);
 // dialog instead of the entire sink button being a stop button.
 BASE_DECLARE_FEATURE(kCastDialogStopButton);
 
+// If enabled, mirroring sessions use the playout delay specified by
+// `kCastMirroringPlayoutDelayMs`.
+BASE_DECLARE_FEATURE(kCastMirroringPlayoutDelay);
+extern const base::FeatureParam<int> kCastMirroringPlayoutDelayMs;
+
 // Registers |kMediaRouterCastAllowAllIPs| with local state pref |registry|.
 void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
 
@@ -91,6 +98,10 @@ bool DialMediaRouteProviderEnabled();
 // Returns true if global media controls are used to start and stop casting and
 // Media Router is enabled for |context|.
 bool GlobalMediaControlsCastStartStopEnabled(content::BrowserContext* context);
+
+// Returns the optional value to use for mirroring playout delay from the
+// relevant command line flag or feature, if any are set.
+absl::optional<base::TimeDelta> GetCastMirroringPlayoutDelay();
 #endif  // !BUILDFLAG(IS_ANDROID)
 
 }  // namespace media_router