fix: getUserMedia duplicate permissions call (#36787)

* fix: getUserMedia duplicate permissions call

* test: add regression test
This commit is contained in:
Shelley Vohr 2023-01-11 11:55:31 +01:00 committed by GitHub
parent 1d9a4ab02c
commit f31826f4a0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 112 additions and 44 deletions

View file

@ -15,60 +15,60 @@ short-circuit all the permissions checks in MSDC for now to allow us to
unduplicate this code. unduplicate this code.
diff --git a/components/webrtc/media_stream_devices_controller.cc b/components/webrtc/media_stream_devices_controller.cc diff --git a/components/webrtc/media_stream_devices_controller.cc b/components/webrtc/media_stream_devices_controller.cc
index 7dbbcc13901dcd8b7a9ca9c9bdce4f924c1c6a55..5de44c5c20c92a1793060492f925519d6b8befe5 100644 index 7dbbcc13901dcd8b7a9ca9c9bdce4f924c1c6a55..f4def7e347aafe30485fd1e6055c97d471b44776 100644
--- a/components/webrtc/media_stream_devices_controller.cc --- a/components/webrtc/media_stream_devices_controller.cc
+++ b/components/webrtc/media_stream_devices_controller.cc +++ b/components/webrtc/media_stream_devices_controller.cc
@@ -92,10 +92,13 @@ void MediaStreamDevicesController::RequestPermissions( @@ -56,7 +56,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,6 +93,7 @@ void MediaStreamDevicesController::RequestPermissions(
std::vector<blink::PermissionType> permission_types; std::vector<blink::PermissionType> permission_types;
+#if 0 +#if 0
content::PermissionController* permission_controller = content::PermissionController* permission_controller =
web_contents->GetBrowserContext()->GetPermissionController(); web_contents->GetBrowserContext()->GetPermissionController();
+#endif
if (controller->ShouldRequestAudio()) {
+#if 0
content::PermissionResult permission_status =
permission_controller->GetPermissionResultForCurrentDocument(
blink::PermissionType::AUDIO_CAPTURE, rfh);
@@ -110,10 +113,12 @@ void MediaStreamDevicesController::RequestPermissions(
content::PermissionStatusSource::FENCED_FRAME);
return;
}
+#endif
permission_types.push_back(blink::PermissionType::AUDIO_CAPTURE);
}
if (controller->ShouldRequestVideo()) {
+#if 0
content::PermissionResult permission_status =
permission_controller->GetPermissionResultForCurrentDocument(
blink::PermissionType::VIDEO_CAPTURE, rfh);
@@ -128,6 +133,7 @@ void MediaStreamDevicesController::RequestPermissions(
content::PermissionStatusSource::FENCED_FRAME);
return;
}
+#endif
permission_types.push_back(blink::PermissionType::VIDEO_CAPTURE);
@@ -139,6 +145,7 @@ void MediaStreamDevicesController::RequestPermissions(
// pan-tilt-zoom permission and there are suitable PTZ capable devices
// available.
if (request.request_pan_tilt_zoom_permission && has_pan_tilt_zoom_camera) {
+#if 0
permission_status =
permission_controller->GetPermissionResultForCurrentDocument(
blink::PermissionType::CAMERA_PAN_TILT_ZOOM, rfh);
@@ -148,6 +155,7 @@ void MediaStreamDevicesController::RequestPermissions(
controller->RunCallback(/*blocked_by_permissions_policy=*/false);
return;
}
+#endif
@@ -152,19 +154,25 @@ void MediaStreamDevicesController::RequestPermissions(
permission_types.push_back(blink::PermissionType::CAMERA_PAN_TILT_ZOOM); permission_types.push_back(blink::PermissionType::CAMERA_PAN_TILT_ZOOM);
} }
}
+#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(
- permission_types, rfh, request.user_gesture,
- 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(
+ permission_types, rfh, request.user_gesture,
+ base::BindOnce(
+ &MediaStreamDevicesController::PromptAnsweredGroupedRequest,
+ std::move(controller)));
+ }
}
MediaStreamDevicesController::~MediaStreamDevicesController() {
@@ -434,6 +442,7 @@ bool MediaStreamDevicesController::PermissionIsBlockedForReason( @@ -434,6 +442,7 @@ bool MediaStreamDevicesController::PermissionIsBlockedForReason(
return false; return false;
} }
@ -85,3 +85,17 @@ index 7dbbcc13901dcd8b7a9ca9c9bdce4f924c1c6a55..5de44c5c20c92a1793060492f925519d
return false; return false;
} }
diff --git a/components/webrtc/media_stream_devices_controller.h b/components/webrtc/media_stream_devices_controller.h
index b9cf3f6ad047fa16594393134eae5fc5349e7430..5de5e0bf9b455872f69de47d58dda929f00df12c 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();

View file

@ -119,7 +119,8 @@ void MediaAccessAllowed(const content::MediaStreamRequest& request,
blink::mojom::MediaStreamType::DEVICE_AUDIO_CAPTURE) { blink::mojom::MediaStreamType::DEVICE_AUDIO_CAPTURE) {
webrtc::MediaStreamDevicesController::RequestPermissions( webrtc::MediaStreamDevicesController::RequestPermissions(
request, MediaCaptureDevicesDispatcher::GetInstance(), request, MediaCaptureDevicesDispatcher::GetInstance(),
base::BindOnce(&OnMediaStreamRequestResponse, std::move(callback))); base::BindOnce(&OnMediaStreamRequestResponse, std::move(callback)),
allowed);
} else if (request.video_type == } else if (request.video_type ==
blink::mojom::MediaStreamType::DISPLAY_VIDEO_CAPTURE || blink::mojom::MediaStreamType::DISPLAY_VIDEO_CAPTURE ||
request.video_type == blink::mojom::MediaStreamType:: request.video_type == blink::mojom::MediaStreamType::

View file

@ -1053,6 +1053,22 @@ describe('session module', () => {
describe('ses.setPermissionRequestHandler(handler)', () => { describe('ses.setPermissionRequestHandler(handler)', () => {
afterEach(closeAllWindows); afterEach(closeAllWindows);
// These tests are done on an http server because navigator.userAgentData
// requires a secure context.
let server: http.Server;
let serverUrl: string;
before(async () => {
server = http.createServer((req, res) => {
res.setHeader('Content-Type', 'text/html');
res.end('');
});
await new Promise<void>(resolve => server.listen(0, '127.0.0.1', resolve));
serverUrl = `http://localhost:${(server.address() as any).port}`;
});
after(() => {
server.close();
});
it('cancels any pending requests when cleared', async () => { it('cancels any pending requests when cleared', async () => {
const w = new BrowserWindow({ const w = new BrowserWindow({
show: false, show: false,
@ -1085,6 +1101,43 @@ describe('session module', () => {
const [, name] = await result; const [, name] = await result;
expect(name).to.deep.equal('SecurityError'); expect(name).to.deep.equal('SecurityError');
}); });
it('successfully resolves when calling legacy getUserMedia', async () => {
const ses = session.fromPartition('' + Math.random());
ses.setPermissionRequestHandler(
(_webContents, _permission, callback) => {
callback(true);
}
);
const w = new BrowserWindow({ show: false, webPreferences: { session: ses } });
await w.loadURL(serverUrl);
const { ok, message } = await w.webContents.executeJavaScript(`
new Promise((resolve, reject) => navigator.getUserMedia({
video: true,
audio: true,
}, x => resolve({ok: x instanceof MediaStream}), e => reject({ok: false, message: e.message})))
`);
expect(ok).to.be.true(message);
});
it('successfully rejects when calling legacy getUserMedia', async () => {
const ses = session.fromPartition('' + Math.random());
ses.setPermissionRequestHandler(
(_webContents, _permission, callback) => {
callback(false);
}
);
const w = new BrowserWindow({ show: false, webPreferences: { session: ses } });
await w.loadURL(serverUrl);
await expect(w.webContents.executeJavaScript(`
new Promise((resolve, reject) => navigator.getUserMedia({
video: true,
audio: true,
}, x => resolve({ok: x instanceof MediaStream}), e => reject({ok: false, message: e.message})))
`)).to.eventually.be.rejectedWith('Permission denied');
});
}); });
describe('ses.setPermissionCheckHandler(handler)', () => { describe('ses.setPermissionCheckHandler(handler)', () => {