From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 12 Jul 2022 16:51:43 -0700 Subject: short-circuit permissions checks in MediaStreamDevicesController The //components/permissions architecture is complicated and not that widely used in Chromium, and mostly oriented around showing permissions UI and/or remembering per-site permissions, which we're not interested in. Since we do a permissions check prior to invoking the MediaStreamDevicesController, and don't (yet) provide the ability to set granular permissions (e.g. allow video but not audio), just short-circuit all the permissions checks in MSDC for now to allow us to unduplicate this code. diff --git a/components/webrtc/media_stream_devices_controller.cc b/components/webrtc/media_stream_devices_controller.cc index fd5f81970957d970b7d234f45c4948395a461a7a..386599be509389c9bf850c330e76e449dd685318 100644 --- a/components/webrtc/media_stream_devices_controller.cc +++ b/components/webrtc/media_stream_devices_controller.cc @@ -57,7 +57,8 @@ bool PermissionIsRequested(blink::PermissionType permission, void MediaStreamDevicesController::RequestPermissions( const content::MediaStreamRequest& request, MediaStreamDeviceEnumerator* enumerator, - ResultCallback callback) { + ResultCallback callback, + bool previously_approved) { content::RenderFrameHost* rfh = content::RenderFrameHost::FromID( request.render_process_id, request.render_frame_id); // The RFH may have been destroyed by the time the request is processed. @@ -92,7 +93,7 @@ void MediaStreamDevicesController::RequestPermissions( std::move(callback))); std::vector permission_types; - +#if 0 content::PermissionController* permission_controller = web_contents->GetBrowserContext()->GetPermissionController(); @@ -165,19 +166,26 @@ void MediaStreamDevicesController::RequestPermissions( requested_audio_capture_device_ids; permission_request_description.requested_video_capture_device_ids = requested_video_capture_device_ids; - +#endif // It is OK to ignore `request.security_origin` because it will be calculated // from `render_frame_host` and we always ignore `requesting_origin` for // `AUDIO_CAPTURE` and `VIDEO_CAPTURE`. // `render_frame_host->GetMainFrame()->GetLastCommittedOrigin()` will be used // instead. - rfh->GetBrowserContext() - ->GetPermissionController() - ->RequestPermissionsFromCurrentDocument( - rfh, permission_request_description, - base::BindOnce( - &MediaStreamDevicesController::PromptAnsweredGroupedRequest, - std::move(controller))); + if (previously_approved) { + controller->PromptAnsweredGroupedRequest({blink::mojom::PermissionStatus::GRANTED /*audio*/, + blink::mojom::PermissionStatus::GRANTED /*video*/}); + } else { + rfh->GetBrowserContext() + ->GetPermissionController() + ->RequestPermissionsFromCurrentDocument( + rfh, + content::PermissionRequestDescription(permission_types, + request.user_gesture), + base::BindOnce( + &MediaStreamDevicesController::PromptAnsweredGroupedRequest, + std::move(controller))); + } } MediaStreamDevicesController::~MediaStreamDevicesController() { @@ -429,6 +437,7 @@ bool MediaStreamDevicesController::PermissionIsBlockedForReason( return false; } +#if 0 // TODO(raymes): This function wouldn't be needed if // PermissionManager::RequestPermissions returned a denial reason. content::PermissionResult result = @@ -439,6 +448,7 @@ bool MediaStreamDevicesController::PermissionIsBlockedForReason( DCHECK_EQ(blink::mojom::PermissionStatus::DENIED, result.status); return true; } +#endif return false; } diff --git a/components/webrtc/media_stream_devices_controller.h b/components/webrtc/media_stream_devices_controller.h index 4d3bf184243a4c1296f41070e934730a3bd2620e..78610ff9fb93cfa1c9aae898d6f8c83520fc860a 100644 --- a/components/webrtc/media_stream_devices_controller.h +++ b/components/webrtc/media_stream_devices_controller.h @@ -48,7 +48,8 @@ class MediaStreamDevicesController { // synchronously or asynchronously returned via |callback|. static void RequestPermissions(const content::MediaStreamRequest& request, MediaStreamDeviceEnumerator* enumerator, - ResultCallback callback); + ResultCallback callback, + bool previously_approved = false); ~MediaStreamDevicesController();