[Backport] Security bug 829317 (1/2)

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4020095:
Make browser start recording indicator

Prior to this CL, the recording indicator was turned on by a call sent
by the renderer process.
This CL makes the browser process turn on the indicator directly, which protects against a potential malicious renderer that intentionally does
not make the call.
The difference in behavior is that, originally, the indicator was turned on after the tracks were started and now it is turned on before they start. The same approach is already in use for pepper media capture.
In both cases, the indicator was turned off by the browser when the sources backing the tracks stopped.

Bug: 829317
Change-Id: I1af1f481cbc0e8eb0fb96919313b667ac1fa0a0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4020095
Reviewed-by: Tomas Gunnarsson <tommi@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1070795}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/461077
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

5 files changed, 42 insertions, 14 deletions

diff --git a/chromium/content/browser/renderer_host/media/media_stream_dispatcher_host.cc b/chromium/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
index 73363670165..5ba244698fe 100644
--- a/chromium/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
+++ b/chromium/content/browser/renderer_host/media/media_stream_dispatcher_host.cc
@@ -30,6 +30,7 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/mojom/mediastream/media_stream.mojom.h"
 #include "url/origin.h"
 
@@ -597,6 +598,8 @@ void MediaStreamDispatcherHost::SetCapturingLinkSecured(
 void MediaStreamDispatcherHost::OnStreamStarted(const std::string& label) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  DCHECK(!base::FeatureList::IsEnabled(
+      blink::features::kStartMediaStreamCaptureIndicatorInBrowser));
   media_stream_manager_->OnStreamStarted(label);
 }
 
diff --git a/chromium/content/browser/renderer_host/media/media_stream_manager.cc b/chromium/content/browser/renderer_host/media/media_stream_manager.cc
index aaaa33780fe..c759c4bd2b9 100644
--- a/chromium/content/browser/renderer_host/media/media_stream_manager.cc
+++ b/chromium/content/browser/renderer_host/media/media_stream_manager.cc
@@ -934,8 +934,8 @@ class MediaStreamManager::DeviceRequest {
   }
 
   void RunMojoCallbacks() {
-    if (generate_stream_cb) {
-      std::move(generate_stream_cb)
+    if (generate_streams_cb_) {
+      std::move(generate_streams_cb_)
           .Run(MediaStreamRequestResult::FAILED_DUE_TO_SHUTDOWN,
                /*label=*/std::string(),
                /*stream_devices_set=*/nullptr,
@@ -961,6 +961,18 @@ class MediaStreamManager::DeviceRequest {
 
   void SetLabel(const std::string& label) { label_ = label; }
 
+  void SetGenerateStreamsCallback(GenerateStreamsCallback callback) {
+    generate_streams_cb_ = std::move(callback);
+  }
+
+  GenerateStreamsCallback&& MoveGenerateStreamsCallback() {
+    return std::move(generate_streams_cb_);
+  }
+
+  bool HasGenerateStreamsCallback() const {
+    return !generate_streams_cb_.is_null();
+  }
+
   // The render process id that requested this stream to be generated and that
   // will receive a handle to the MediaStream. This may be different from
   // MediaStreamRequest::render_process_id which in the tab capture case
@@ -1001,7 +1013,6 @@ class MediaStreamManager::DeviceRequest {
   // Currently it is only used by |DEVICE_ACCESS| type.
   MediaAccessRequestCallback media_access_request_cb;
 
-  GenerateStreamsCallback generate_stream_cb;
 
   // This callback is used by transferred MediaStreamTracks to access and clone
   // an existing open MediaStreamDevice (identified by its session_id). If the
@@ -1087,6 +1098,7 @@ class MediaStreamManager::DeviceRequest {
   int target_process_id_;
   int target_frame_id_;
   std::string label_;
+  GenerateStreamsCallback generate_streams_cb_;
 };
 
 // static
@@ -1291,7 +1303,7 @@ void MediaStreamManager::GenerateStreams(
     MediaDeviceSaltAndOrigin salt_and_origin,
     bool user_gesture,
     StreamSelectionInfoPtr audio_stream_selection_info_ptr,
-    GenerateStreamsCallback generate_stream_cb,
+    GenerateStreamsCallback generate_streams_cb,
     DeviceStoppedCallback device_stopped_cb,
     DeviceChangedCallback device_changed_cb,
     DeviceRequestStateChangeCallback device_request_state_change_cb,
@@ -1306,7 +1318,7 @@ void MediaStreamManager::GenerateStreams(
       std::move(device_stopped_cb), std::move(device_changed_cb),
       std::move(device_request_state_change_cb),
       std::move(device_capture_handle_change_cb));
-  request->generate_stream_cb = std::move(generate_stream_cb);
+  request->SetGenerateStreamsCallback(std::move(generate_streams_cb));
   DeviceRequest* const request_ptr = request.get();
   const std::string label = AddRequest(std::move(request));
 
@@ -2483,7 +2495,7 @@ void MediaStreamManager::FinalizeGenerateStreams(const std::string& label,
                                                  DeviceRequest* request) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(request);
-  DCHECK(request->generate_stream_cb);
+  DCHECK(request->HasGenerateStreamsCallback());
   SendLogMessage(
       base::StringPrintf("FinalizeGenerateStreams({label=%s}, {requester_id="
                          "%d}, {request_type=%s})",
@@ -2500,7 +2512,7 @@ void MediaStreamManager::FinalizeGenerateStreams(const std::string& label,
   if (request->IsGetDisplayMediaSet()) {
     PanTiltZoomPermissionChecked(
         label, MediaStreamDevice(),
-        base::BindOnce(std::move(request->generate_stream_cb),
+        base::BindOnce(request->MoveGenerateStreamsCallback(),
                        MediaStreamRequestResult::OK, label,
                        std::move(stream_devices_set)),
         /*pan_tilt_zoom_allowed=*/false);
@@ -2524,7 +2536,7 @@ void MediaStreamManager::FinalizeGenerateStreams(const std::string& label,
           &MediaStreamManager::PanTiltZoomPermissionChecked,
           base::Unretained(this), label,
           request->stream_devices_set.stream_devices[0]->video_device,
-          base::BindOnce(std::move(request->generate_stream_cb),
+          base::BindOnce(request->MoveGenerateStreamsCallback(),
                          MediaStreamRequestResult::OK, label,
                          std::move(stream_devices_set))));
 }
@@ -2649,11 +2661,10 @@ void MediaStreamManager::FinalizeRequestFailed(
 
   switch (request->request_type()) {
     case blink::MEDIA_GENERATE_STREAM: {
-      DCHECK(request->generate_stream_cb);
-      std::move(request->generate_stream_cb)
-          .Run(result, /*label=*/std::string(),
-               /*stream_devices_set=*/nullptr,
-               /*pan_tilt_zoom_allowed=*/false);
+      DCHECK(request->HasGenerateStreamsCallback());
+      request->MoveGenerateStreamsCallback().Run(
+          result, std::string(), /*stream_devices_set=*/nullptr,
+          /*pan_tilt_zoom_allowed=*/false);
       break;
     }
     case blink::MEDIA_GET_OPEN_DEVICE: {
@@ -2945,6 +2956,10 @@ void MediaStreamManager::HandleRequestDone(const std::string& label,
       OnStreamStarted(label);
       break;
     case blink::MEDIA_GENERATE_STREAM: {
+      if (base::FeatureList::IsEnabled(
+              blink::features::kStartMediaStreamCaptureIndicatorInBrowser)) {
+        OnStreamStarted(label);
+      }
       FinalizeGenerateStreams(label, request);
       break;
     }
diff --git a/chromium/third_party/blink/common/features.cc b/chromium/third_party/blink/common/features.cc
index 247d2720789..5507ca29f21 100644
--- a/chromium/third_party/blink/common/features.cc
+++ b/chromium/third_party/blink/common/features.cc
@@ -1643,5 +1643,8 @@ BASE_FEATURE(kPostMessageDifferentPartitionSameOriginBlocked,
              "PostMessageDifferentPartitionSameOriginBlocked",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
+BASE_FEATURE(kStartMediaStreamCaptureIndicatorInBrowser,
+             "StartMediaStreamCaptureIndicatorInBrowser",
+             base::FEATURE_ENABLED_BY_DEFAULT);
 }  // namespace features
 }  // namespace blink
diff --git a/chromium/third_party/blink/public/common/features.h b/chromium/third_party/blink/public/common/features.h
index a3f93cf27df..26e4742c0c1 100644
--- a/chromium/third_party/blink/public/common/features.h
+++ b/chromium/third_party/blink/public/common/features.h
@@ -885,6 +885,9 @@ BLINK_COMMON_EXPORT extern const base::FeatureParam<int>
 BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(
     kPostMessageDifferentPartitionSameOriginBlocked);
 
+// Make the browser decide when to turn on the capture indicator (red button)
+BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(
+    kStartMediaStreamCaptureIndicatorInBrowser);
 }  // namespace features
 }  // namespace blink
 
diff --git a/chromium/third_party/blink/renderer/modules/mediastream/user_media_processor.cc b/chromium/third_party/blink/renderer/modules/mediastream/user_media_processor.cc
index 377b3b4fd93..2b12ff46941 100644
--- a/chromium/third_party/blink/renderer/modules/mediastream/user_media_processor.cc
+++ b/chromium/third_party/blink/renderer/modules/mediastream/user_media_processor.cc
@@ -22,6 +22,7 @@
 #include "media/capture/video_capture_types.h"
 #include "media/webrtc/constants.h"
 #include "third_party/blink/public/common/browser_interface_broker_proxy.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/mediastream/media_stream_controls.h"
 #include "third_party/blink/public/common/mediastream/media_stream_request.h"
 #include "third_party/blink/public/mojom/mediastream/media_stream.mojom-blink.h"
@@ -1646,7 +1647,10 @@ void UserMediaProcessor::OnCreateNativeTracksCompleted(
   if (result == MediaStreamRequestResult::OK) {
     GetUserMediaRequestSucceeded(request_info->descriptors(),
                                  request_info->request());
-    GetMediaStreamDispatcherHost()->OnStreamStarted(label);
+    if (!base::FeatureList::IsEnabled(
+            blink::features::kStartMediaStreamCaptureIndicatorInBrowser)) {
+      GetMediaStreamDispatcherHost()->OnStreamStarted(label);
+    }
   } else {
     GetUserMediaRequestFailed(result, constraint_name);