From 51cf4b9f857beaad802f6019792cc84f62337150 Mon Sep 17 00:00:00 2001
From: Elad Alon <[email protected]>
Date: Thu, 25 Sep 2025 03:21:19 -0700
Subject: [PATCH] Avoid propagating GenerateStreams from inactive RFHs

Soft-fail GenerateStreams() calls from RFHs that are no longer active,
as is the case when navigating.

This is the short-term fix. In the long-term, it's also important to
associate the GenerateStreams message with the RFH in the rest of the
pipeline, as the RFH might still asynchronously deactivate at a later
time that is still before the dialog is shown to the user.

Bug: 442860743
Change-Id: Iaf322eb151ee6e916a24dd10468b2f0426216ac1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6966127
Commit-Queue: Elad Alon <[email protected]>
Reviewed-by: Simon Hangl <[email protected]>
Reviewed-by: Guido Urdaneta <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1520479}
---

diff --git a/content/browser/renderer_host/media/media_stream_dispatcher_host.cc b/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
index 7f8d221..6d3615cc 100644
--- a/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
+++ b/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
@@ -52,6 +52,7 @@
 namespace {
 
 using ::blink::mojom::CapturedSurfaceControlResult;
+using ::blink::mojom::MediaStreamRequestResult;
 
 void BindMediaStreamDeviceObserverReceiver(
     GlobalRenderFrameHostId render_frame_host_id,
@@ -359,10 +360,18 @@
         result_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+  RenderFrameHostImpl* const render_frame_host =
+      RenderFrameHostImpl::FromID(render_frame_host_id);
+  if (!render_frame_host || !render_frame_host->IsActive()) {
+    std::move(result_callback)
+        .Run(base::unexpected(MediaStreamRequestResult::INVALID_STATE));
+    return;
+  }
+
   if (request_all_screens) {
     CheckRequestAllScreensAllowed(std::move(get_salt_and_origin_cb),
                                   std::move(result_callback),
-                                  render_frame_host_id);
+                                  render_frame_host);
     return;
   }
 
@@ -376,15 +385,12 @@
         get_salt_and_origin_cb,
     base::OnceCallback<void(GenerateStreamsUIThreadCheckResult)>
         result_callback,
-    GlobalRenderFrameHostId render_frame_host_id) {
+    RenderFrameHost* render_frame_host) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  RenderFrameHostImpl* render_frame_host =
-      RenderFrameHostImpl::FromID(render_frame_host_id);
-  if (!render_frame_host) {
-    CheckStreamsPermissionResultReceived(std::move(get_salt_and_origin_cb),
-                                         std::move(result_callback),
-                                         /*result=*/false);
+  if (!render_frame_host || !render_frame_host->IsActive()) {
+    std::move(result_callback)
+        .Run(base::unexpected(MediaStreamRequestResult::INVALID_STATE));
     return;
   }
 
@@ -401,8 +407,7 @@
     bool result) {
   if (!result) {
     std::move(result_callback)
-        .Run({.request_allowed = false,
-              .salt_and_origin = MediaDeviceSaltAndOrigin::Empty()});
+        .Run(base::unexpected(MediaStreamRequestResult::PERMISSION_DENIED));
     return;
   }
 
@@ -410,8 +415,7 @@
       [](base::OnceCallback<void(GenerateStreamsUIThreadCheckResult)>
              result_callback,
          const MediaDeviceSaltAndOrigin& salt_and_origin) {
-        std::move(result_callback)
-            .Run({.request_allowed = true, .salt_and_origin = salt_and_origin});
+        std::move(result_callback).Run(salt_and_origin);
       },
       std::move(result_callback));
   std::move(get_salt_and_origin_cb).Run(std::move(got_salt_and_origin));
@@ -448,7 +452,7 @@
 
   for (auto& pending_request : pending_requests_) {
     std::move(pending_request->callback)
-        .Run(blink::mojom::MediaStreamRequestResult::FAILED_DUE_TO_SHUTDOWN,
+        .Run(MediaStreamRequestResult::FAILED_DUE_TO_SHUTDOWN,
              /*label=*/std::string(),
              /*stream_devices_set=*/nullptr,
              /*pan_tilt_zoom_allowed=*/false);
@@ -469,7 +473,7 @@
   const std::optional<bad_message::BadMessageReason> bad_message =
       ValidateControlsForGenerateStreams(controls);
   if (bad_message.has_value()) {
-    ReceivedBadMessage(render_frame_host_id_.child_id, bad_message.value());
+    ReceivedBadMessage(render_frame_host_id_.child_id, *bad_message);
     return;
   }
 
@@ -495,25 +499,21 @@
     GenerateStreamsUIThreadCheckResult ui_check_result) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  if (!ui_check_result.request_allowed) {
-    std::move(callback).Run(
-        blink::mojom::MediaStreamRequestResult::PERMISSION_DENIED,
-        /*label=*/std::string(),
-        /*stream_devices_set=*/nullptr,
-        /*pan_tilt_zoom_allowed=*/false);
+  if (!ui_check_result.has_value()) {
+    std::move(callback).Run(ui_check_result.error(),
+                            /*label=*/std::string(),
+                            /*stream_devices_set=*/nullptr,
+                            /*pan_tilt_zoom_allowed=*/false);
     return;
   }
 
-  MediaDeviceSaltAndOrigin salt_and_origin =
-      std::move(ui_check_result.salt_and_origin);
-  ui_check_result = {.salt_and_origin = MediaDeviceSaltAndOrigin::Empty()};
+  MediaDeviceSaltAndOrigin salt_and_origin = std::move(*ui_check_result);
   if (!MediaStreamManager::IsOriginAllowed(render_frame_host_id_.child_id,
                                            salt_and_origin.origin())) {
-    std::move(callback).Run(
-        blink::mojom::MediaStreamRequestResult::INVALID_SECURITY_ORIGIN,
-        /*label=*/std::string(),
-        /*stream_devices_set=*/nullptr,
-        /*pan_tilt_zoom_allowed=*/false);
+    std::move(callback).Run(MediaStreamRequestResult::INVALID_SECURITY_ORIGIN,
+                            /*label=*/std::string(),
+                            /*stream_devices_set=*/nullptr,
+                            /*pan_tilt_zoom_allowed=*/false);
     return;
   }
 
@@ -778,8 +778,7 @@
     ReceivedBadMessage(render_frame_host_id_.child_id,
                        bad_message::MSDH_GET_OPEN_DEVICE_USE_WITHOUT_FEATURE);
 
-    std::move(callback).Run(
-        blink::mojom::MediaStreamRequestResult::NOT_SUPPORTED, nullptr);
+    std::move(callback).Run(MediaStreamRequestResult::NOT_SUPPORTED, nullptr);
     return;
   }
   // TODO(crbug.com/40058526): Decide whether we need to have another
@@ -806,9 +805,8 @@
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!MediaStreamManager::IsOriginAllowed(render_frame_host_id_.child_id,
                                            salt_and_origin.origin())) {
-    std::move(callback).Run(
-        blink::mojom::MediaStreamRequestResult::INVALID_SECURITY_ORIGIN,
-        nullptr);
+    std::move(callback).Run(MediaStreamRequestResult::INVALID_SECURITY_ORIGIN,
+                            nullptr);
     return;
   }
 
diff --git a/content/browser/renderer_host/media/media_stream_dispatcher_host.h b/content/browser/renderer_host/media/media_stream_dispatcher_host.h
index ed30ead..40cf9ce 100644
--- a/content/browser/renderer_host/media/media_stream_dispatcher_host.h
+++ b/content/browser/renderer_host/media/media_stream_dispatcher_host.h
@@ -13,6 +13,7 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/synchronization/lock.h"
+#include "base/types/expected.h"
 #include "build/build_config.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/media/media_devices_util.h"
@@ -72,10 +73,9 @@
   FRIEND_TEST_ALL_PREFIXES(MediaStreamDispatcherHostMultiCaptureTest,
                            PolicySetMultiCaptureAllowed);
 
-  struct GenerateStreamsUIThreadCheckResult {
-    bool request_allowed = false;
-    MediaDeviceSaltAndOrigin salt_and_origin;
-  };
+  using GenerateStreamsUIThreadCheckResult =
+      ::base::expected<::content::MediaDeviceSaltAndOrigin,
+                       ::blink::mojom::MediaStreamRequestResult>;
 
   struct PendingAccessRequest;
   using RequestsQueue =
@@ -98,7 +98,7 @@
           get_salt_and_origin_cb,
       base::OnceCallback<void(GenerateStreamsUIThreadCheckResult)>

diff --git a/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc b/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
index 3c4c139..1666996f 100644
--- a/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
+++ b/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
@@ -68,17 +68,22 @@
 #endif
 
 using ::blink::mojom::CapturedSurfaceControlResult;
+using ::blink::mojom::MediaStreamRequestResult;
 using ::blink::mojom::MediaStreamType;
 using ::testing::_;
+using ::testing::Bool;
 using ::testing::InSequence;
 using ::testing::InvokeWithoutArgs;
 using ::testing::Return;
 
+using GenerateStreamsUIThreadCheckResult =
+    ::base::expected<::content::MediaDeviceSaltAndOrigin,
+                     ::blink::mojom::MediaStreamRequestResult>;
+
 namespace content {
 
 namespace {
 
-const GlobalRenderFrameHostId kRenderFrameHostId{5, 6};
 constexpr int kRequesterId = 7;
 constexpr int kPageRequestId = 8;
 constexpr const char* kRegularVideoDeviceId1 = "stub_device_1";
@@ -144,8 +149,7 @@
                void(int request_id,
                     const blink::mojom::StreamDevicesSet& stream_devices_set));
   MOCK_METHOD2(OnStreamGenerationFailure,
-               void(int request_id,
-                    blink::mojom::MediaStreamRequestResult result));
+               void(int request_id, MediaStreamRequestResult result));
   MOCK_METHOD0(OnDeviceStopSuccess, void());
   MOCK_METHOD0(OnDeviceOpenSuccess, void());
 
@@ -221,11 +225,11 @@
  private:
   // These handler methods do minimal things and delegate to the mock methods.
   void OnStreamsGenerated(int request_id,
-                          blink::mojom::MediaStreamRequestResult result,
+                          MediaStreamRequestResult result,
                           const std::string& label,
                           blink::mojom::StreamDevicesSetPtr stream_devices_set,
                           bool pan_tilt_zoom_allowed) {
-    if (result != blink::mojom::MediaStreamRequestResult::OK) {
+    if (result != MediaStreamRequestResult::OK) {
       DCHECK(!stream_devices_set);
       OnStreamGenerationFailed(request_id, result);
       return;
@@ -242,7 +246,7 @@
   }
 
   void OnStreamGenerationFailed(int request_id,
-                                blink::mojom::MediaStreamRequestResult result) {
+                                MediaStreamRequestResult result) {
     OnStreamGenerationFailure(request_id, result);
     if (!quit_closures_.empty()) {
       task_runner_->PostTask(FROM_HERE, std::move(quit_closures_.front()));
@@ -289,7 +293,7 @@
       : FakeMediaStreamUIProxy(/*tests_use_fake_render_frame_hosts=*/true) {}
   void OnStarted(
       base::OnceClosure stop,
-      content::MediaStreamUI::SourceCallback source,
+      MediaStreamUI::SourceCallback source,
       MediaStreamUIProxy::WindowIdCallback window_id_callback,
       const std::string& label,
       std::vector<DesktopMediaID> screen_share_ids,
@@ -301,12 +305,27 @@
   MOCK_METHOD1(MockOnStarted, void(base::OnceClosure& stop));
 };
 
-class MediaStreamDispatcherHostTest : public testing::Test {
+class MockContentBrowserClient : public ContentBrowserClient {
+ public:
+  MOCK_METHOD(bool,
+              IsMultiCaptureAllowed,
+              (RenderFrameHost * render_frame_host),
+              (override));
+};
+
+class MediaStreamDispatcherHostTest : public RenderViewHostTestHarness {
  public:
   MediaStreamDispatcherHostTest()
-      : task_environment_(BrowserTaskEnvironment::IO_MAINLOOP),
+      : RenderViewHostTestHarness(
+#if BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_WIN)
+            base::test::TaskEnvironment::MainThreadType::UI),
+#else
+            base::test::TaskEnvironment::MainThreadType::IO),
+#endif
         salt_and_origin_(CreateRandomMediaDeviceIDSalt(),
                          url::Origin::Create(GURL("https://test.com"))) {
+    SetBrowserClientForTesting(&content_browser_client_);
+
     scoped_feature_list_
         .InitFromCommandLine(/*enable_features=*/
                              "UserMediaCaptureOnFocus,GetAllScreensMedia",
@@ -328,8 +347,16 @@
         audio_system_.get(), std::move(mock_video_capture_provider));
     salt_and_origin_.set_has_focus(true);
     salt_and_origin_.set_is_background(false);
+  }
+
+  ~MediaStreamDispatcherHostTest() override {}
+
+  void SetUp() override {
+    RenderViewHostTestHarness::SetUp();
+    RenderFrameHostTester::For(main_rfh())->InitializeRenderFrameIfNeeded();
+
     host_ = std::make_unique<MockMediaStreamDispatcherHost>(
-        kRenderFrameHostId, media_stream_manager_.get());
+        main_rfh()->GetGlobalId(), media_stream_manager_.get());
     host_->set_get_salt_and_origin_cb_for_testing(
         base::BindRepeating(&MediaStreamDispatcherHostTest::GetSaltAndOrigin,
                             base::Unretained(this)));
@@ -343,17 +370,8 @@
     ash::CrasAudioClient::InitializeFake();
     ash::CrasAudioHandler::InitializeForTesting();
 #endif
-  }
 
-  ~MediaStreamDispatcherHostTest() override {
-    audio_manager_->Shutdown();
-#if BUILDFLAG(IS_CHROMEOS)
-    ash::CrasAudioHandler::Shutdown();
-    ash::CrasAudioClient::Shutdown();
-#endif
-  }
 
-  void SetUp() override {
     stub_video_device_ids_.emplace_back(kRegularVideoDeviceId1);
     stub_video_device_ids_.emplace_back(kDepthVideoDeviceId);
     ON_CALL(*mock_video_capture_provider_, GetDeviceInfosAsync(_))
@@ -385,8 +403,18 @@
   }
 
   void TearDown() override {
+    mock_video_capture_provider_ = nullptr;
+
     host_->CancelAllRequests();
     host_.reset();
+
+    audio_manager_->Shutdown();
+#if BUILDFLAG(IS_CHROMEOS)
+    ash::CrasAudioHandler::Shutdown();
+    ash::CrasAudioClient::Shutdown();
+#endif
+
+    RenderViewHostTestHarness::TearDown();
   }
 
   void GetSaltAndOrigin(GlobalRenderFrameHostId,
@@ -438,7 +466,7 @@
   void GenerateStreamAndWaitForFailure(
       int page_request_id,
       const blink::StreamControls& controls,
-      blink::mojom::MediaStreamRequestResult expected_result) {
+      MediaStreamRequestResult expected_result) {
     base::RunLoop run_loop;
     EXPECT_CALL(*host_,
                 OnStreamGenerationFailure(page_request_id, expected_result));
@@ -564,11 +592,11 @@
   std::unique_ptr<MediaStreamManager> media_stream_manager_;
   std::unique_ptr<media::AudioManager> audio_manager_;
   std::unique_ptr<media::AudioSystem> audio_system_;
-  BrowserTaskEnvironment task_environment_;
   MediaDeviceSaltAndOrigin salt_and_origin_;
   media::AudioDeviceDescriptions audio_device_descriptions_;
   std::vector<std::string> stub_video_device_ids_;
   raw_ptr<MockVideoCaptureProvider> mock_video_capture_provider_;
+  MockContentBrowserClient content_browser_client_;
 };
 
 TEST_F(MediaStreamDispatcherHostTest, GenerateStreamWithVideoOnly) {
@@ -612,7 +640,7 @@
   EXPECT_CALL(
       *this,
       MockOnBadMessage(
-          kRenderFrameHostId.child_id,
+          main_rfh()->GetGlobalId().child_id,
           bad_message::
               MSDH_SUPPRESS_LOCAL_AUDIO_PLAYBACK_BUT_AUDIO_NOT_REQUESTED))
       .Times(1);
@@ -630,7 +658,7 @@
 
   EXPECT_CALL(*this,
               MockOnBadMessage(
-                  kRenderFrameHostId.child_id,
+                  main_rfh()->GetGlobalId().child_id,
                   bad_message::MSDH_HOTWORD_ENABLED_BUT_AUDIO_NOT_REQUESTED))
       .Times(1);
   host_->OnGenerateStreams(kPageRequestId, controls);
@@ -647,7 +675,7 @@
 
   EXPECT_CALL(*this,
               MockOnBadMessage(
-                  kRenderFrameHostId.child_id,
+                  main_rfh()->GetGlobalId().child_id,
                   bad_message::MSDH_DISABLE_LOCAL_ECHO_BUT_AUDIO_NOT_REQUESTED))
       .Times(1);
   host_->OnGenerateStreams(kPageRequestId, controls);
@@ -663,7 +691,7 @@
 
   EXPECT_CALL(*this,
               MockOnBadMessage(
-                  kRenderFrameHostId.child_id,
+                  main_rfh()->GetGlobalId().child_id,
                   bad_message::MSDH_RESTRICT_OWN_AUDIO_IS_SET_WHEN_UNSUPPORTED))
       .Times(media::IsRestrictOwnAudioSupported() ? 0 : 1);
   host_->OnGenerateStreams(kPageRequestId, controls);
@@ -677,7 +705,7 @@
 
   GenerateStreamAndWaitForFailure(
       kPageRequestId, controls,
-      blink::mojom::MediaStreamRequestResult::FAILED_DUE_TO_SHUTDOWN);
+      MediaStreamRequestResult::FAILED_DUE_TO_SHUTDOWN);
 }
 
 TEST_F(MediaStreamDispatcherHostTest, GenerateStreamWithAudioAndVideo) {
@@ -809,10 +837,12 @@
       video_device(/*stream_index=*/0u).value().session_id();
 
   // Generate second stream from another render frame.
+  std::unique_ptr<WebContents> other_tab = CreateTestWebContents();
+  RenderFrameHost* const other_rfh = other_tab->GetPrimaryMainFrame();
+  RenderFrameHostTester::For(other_rfh)->InitializeRenderFrameIfNeeded();
+
   host_ = std::make_unique<MockMediaStreamDispatcherHost>(
-      GlobalRenderFrameHostId{kRenderFrameHostId.child_id,
-                              kRenderFrameHostId.frame_routing_id + 1},
-      media_stream_manager_.get());
+      other_rfh->GetGlobalId(), media_stream_manager_.get());
   host_->set_get_salt_and_origin_cb_for_testing(
       base::BindRepeating(&MediaStreamDispatcherHostTest::GetSaltAndOrigin,
                           base::Unretained(this)));
@@ -844,11 +874,9 @@
                           base::Unretained(this)));
 
   base::RunLoop run_loop;
-  EXPECT_CALL(
-      *host_,
-      OnStreamGenerationFailure(
-          kPageRequestId,
-          blink::mojom::MediaStreamRequestResult::FAILED_DUE_TO_SHUTDOWN));
+  EXPECT_CALL(*host_, OnStreamGenerationFailure(
+                          kPageRequestId,
+                          MediaStreamRequestResult::FAILED_DUE_TO_SHUTDOWN));
   host_->OnGenerateStreams(kPageRequestId, controls, run_loop.QuitClosure());
   run_loop.RunUntilIdle();
 }
@@ -1004,9 +1032,8 @@
   blink::StreamControls controls(true, true);
   controls.video.device_ids = {"invalid source id"};
 
-  GenerateStreamAndWaitForFailure(
-      kPageRequestId, controls,
-      blink::mojom::MediaStreamRequestResult::NO_HARDWARE);
+  GenerateStreamAndWaitForFailure(kPageRequestId, controls,
+                                  MediaStreamRequestResult::NO_HARDWARE);
 }
 
 // Test that generating a stream with an invalid audio source id fail.
@@ -1014,9 +1041,8 @@
   blink::StreamControls controls(true, true);
   controls.audio.device_ids = {"invalid source id"};
 
-  GenerateStreamAndWaitForFailure(
-      kPageRequestId, controls,
-      blink::mojom::MediaStreamRequestResult::NO_HARDWARE);
+  GenerateStreamAndWaitForFailure(kPageRequestId, controls,
+                                  MediaStreamRequestResult::NO_HARDWARE);
 }
 
 TEST_F(MediaStreamDispatcherHostTest, GenerateStreamsNoAvailableVideoDevice) {
@@ -1024,9 +1050,8 @@
   blink::StreamControls controls(true, true);
 
   SetupFakeUI(false);
-  GenerateStreamAndWaitForFailure(
-      kPageRequestId, controls,
-      blink::mojom::MediaStreamRequestResult::NO_HARDWARE);
+  GenerateStreamAndWaitForFailure(kPageRequestId, controls,
+                                  MediaStreamRequestResult::NO_HARDWARE);
 }
 
 // Test that if a OnStopStreamDevice message is received for a device that has
@@ -1148,7 +1173,8 @@
                              run_loop.QuitClosure());
   }
 
-  media_stream_manager_->CancelAllRequests(kRenderFrameHostId, kRequesterId);
+  media_stream_manager_->CancelAllRequests(main_rfh()->GetGlobalId(),
+                                           kRequesterId);
   run_loop.RunUntilIdle();
 }
 
@@ -1167,7 +1193,8 @@
... (truncated)