Bug 217048 - Set default WKWebViewConfiguration._mediaCaptureEnabled value according camera/microphone entitlements
Summary: Set default WKWebViewConfiguration._mediaCaptureEnabled value according camer...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-28 02:48 PDT by youenn fablet
Modified: 2020-10-05 09:37 PDT (History)
8 users (show)

See Also:


Attachments
Patch (6.03 KB, patch)
2020-09-28 03:07 PDT, youenn fablet
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (5.99 KB, patch)
2020-09-28 03:32 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
WIP patch (22.36 KB, patch)
2020-09-28 21:55 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
WIP patch (22.86 KB, patch)
2020-09-29 08:53 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff
WIP patch (21.81 KB, patch)
2020-09-29 10:53 PDT, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2020-09-28 02:48:28 PDT
Set default of WKWebViewConfiguration._mediaCaptureEnabled according camera/microphone entitlements
Comment 1 youenn fablet 2020-09-28 03:07:35 PDT
Created attachment 409877 [details]
Patch
Comment 2 youenn fablet 2020-09-28 03:32:02 PDT
Created attachment 409880 [details]
Patch
Comment 3 Darin Adler 2020-09-28 08:24:30 PDT
Comment on attachment 409880 [details]
Patch

I don’t think this is exactly what we want. If a process does not have the entitlements, you said earlier that trying to do the operation can lead to a crash. We don’t want a WKWebViewConfiguration value that can be set that leads to a crash.

Configuration options should be choices/requests. The default can be true as long as the code responds by not advertising the capability to the web content, and not crashing, when entitlements are not available.
Comment 4 youenn fablet 2020-09-28 08:50:58 PDT
(In reply to Darin Adler from comment #3)
> Comment on attachment 409880 [details]
> Patch
> 
> I don’t think this is exactly what we want. If a process does not have the
> entitlements, you said earlier that trying to do the operation can lead to a
> crash. We don’t want a WKWebViewConfiguration value that can be set that
> leads to a crash.

Eric mentioned we could crash but I am not exactly sure of the exact code path.
The choice can be made independently of the entitlements.
For instance, WebKitTestRunner and TestWebKitAPI are supporting loading pages that call getUserMedia: they do not have entitlements but use mock media devices for instance.

If an app has camera entitlement but not microphone entitlement, I would believe getUserMedia would be supported but we would return OverConstrainedError in case the page wants to start capturing audio. We should not crash in any of these cases.
Comment 5 youenn fablet 2020-09-28 09:41:28 PDT
Discussing with Eric, we could indeed add both entitlement and Plist checks.
It seems like a misconfiguration to add entitlements but not the Plist.
Seems ok to add all checks, especially if PLIst checks can be done synchronously.
Comment 6 Eric Carlson 2020-09-28 21:55:07 PDT
Created attachment 409964 [details]
WIP patch
Comment 7 Eric Carlson 2020-09-29 08:53:51 PDT
Created attachment 410005 [details]
WIP patch
Comment 8 Eric Carlson 2020-09-29 10:53:34 PDT
Created attachment 410017 [details]
WIP patch
Comment 9 Darin Adler 2020-09-29 10:57:56 PDT
I am still not comfortable with the approach here.

Automatically setting *preferences* to match entitlements does not seem like the correct design for this, or any other feature. Obviously entitlements should control *capabilities*, but I see no reason they would alter a preference setting.

This is not about whether preferences should exist, but rather the design for how preferences work. They don’t automatically set themselves; the settings flow in from the app using WebKit, and are consulted along with other considerations to determine the behavior of WebKit.

I do not think it’s helpful to set a preference based on entitlements.

There’s a whole separate debate about whether a given preference should exist, but that’s not my point at all.
Comment 10 youenn fablet 2020-09-29 11:15:12 PDT
OK, let's first fix the potential crash issue for now and finalize the discussion about a preference.
Comment 11 Eric Carlson 2020-09-29 13:44:55 PDT
Closing. 

We will prevent an incorrectly configured app from being terminated in bug 217104.
Comment 12 youenn fablet 2020-09-30 09:56:07 PDT
I think we should compute a default value for WKWebViewConfiguration._mediaCaptureEnabled according Info.plist, at least until we provider proper API.
Comment 13 Darin Adler 2020-09-30 10:36:56 PDT
(In reply to youenn fablet from comment #12)
> I think we should compute a default value for
> WKWebViewConfiguration._mediaCaptureEnabled according Info.plist, at least
> until we provider proper API.

Why? That makes no sense to me.

Who does that help? How does it help them?
Comment 14 youenn fablet 2020-09-30 12:16:14 PDT
(In reply to Darin Adler from comment #13)
> (In reply to youenn fablet from comment #12)
> > I think we should compute a default value for
> > WKWebViewConfiguration._mediaCaptureEnabled according Info.plist, at least
> > until we provider proper API.
> 
> Why? That makes no sense to me.

It is always good to provide good default values, especially if we do not provide APIs to change it.
If an application has no way to have a functional getUserMedia, it seems good to not expose getUserMedia at all.
If an application can have a functional getUserMedia, it seems good to expose it.

> Who does that help? How does it help them?

Web developers can do feature detection more easily with a missing getUserMedia than with a getUserMedia that always reject.
Comment 15 Darin Adler 2020-09-30 13:02:22 PDT
(In reply to youenn fablet from comment #14)
> It is always good to provide good default values, especially if we do not
> provide APIs to change it.

I don’t know if I agree or disagree with this. I don’t know what it means.

> If an application has no way to have a functional getUserMedia, it seems
> good to not expose getUserMedia at all.

I agree.

We don’t need to change WKWebViewConfiguration to make that true. We should not expose getUserMedia if the app doesn’t have sufficient entitlement. That should not be done by changing WKWebViewConfiguration, but by adding code to make that happen.

> If an application can have a functional getUserMedia, it seems good to
> expose it.

I agree.
Comment 16 Darin Adler 2020-09-30 13:03:16 PDT
(In reply to Darin Adler from comment #15)
> (In reply to youenn fablet from comment #14)
> > If an application can have a functional getUserMedia, it seems good to
> > expose it.
> 
> I agree.

Actually, I’m not 100% sure about this. Might be privacy or security reasons to not expose by default for all apps; need to confirm that it’s best to have it on by default for all WebKit clients.
Comment 17 youenn fablet 2020-10-05 09:37:43 PDT
Work continued in https://bugs.webkit.org/show_bug.cgi?id=217319.