Bug 223806 – Promote WKWebView getUserMedia SPI to API

• Component: WebRTC
• Status: RESOLVED FIXED

[youenn fablet]

Promote WKWebView getUserMedia SPI to API

[youenn fablet]

Created attachment 424379 [details]
Patch

[Eric Carlson]

Comment on attachment 424379 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424379&action=review

> Source/WebKit/UIProcess/API/Cocoa/WKUIDelegate.h:147
> + @discussion If not implemented, the result is the same as calling the decisionHandler with WKPermissionDecisionPrompt.
> +
> + If you do not implement this method, the web view will behave as if the user selected the Cancel button.

These don't agree with one another.

I wonder if we should mention that the call will be denied if app doesn't have the camera and/or microphone entitlement?

[youenn fablet]

Created attachment 424400 [details]
Patch

[Sam Weinig]

Comment on attachment 424400 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424400&action=review

> Source/WebKit/UIProcess/API/Cocoa/WKUIDelegate.h:58
> +typedef NS_ENUM(NSInteger, WKMediaCaptureType) {
> +    WKMediaCaptureTypeCamera,
> +    WKMediaCaptureTypeMicrophone,
> +    WKMediaCaptureTypeCameraAndMicrophone,
> +} WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));

This seems like something an NS_OPTIONS style bit mask would be better suited for, and therefore you wouldn't need an explicit WKMediaCaptureTypeCameraAndMicrophone value.

[youenn fablet]

(In reply to Sam Weinig from comment #4)
> Comment on attachment 424400 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=424400&action=review
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKUIDelegate.h:58
> > +typedef NS_ENUM(NSInteger, WKMediaCaptureType) {
> > +    WKMediaCaptureTypeCamera,
> > +    WKMediaCaptureTypeMicrophone,
> > +    WKMediaCaptureTypeCameraAndMicrophone,
> > +} WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> 
> This seems like something an NS_OPTIONS style bit mask would be better
> suited for, and therefore you wouldn't need an explicit
> WKMediaCaptureTypeCameraAndMicrophone value.

We could go both ways. I
went this way for two reasons:
1. With a bit field, there is the theoretical possibility of asking for 'no camera' and 'no microphone'. Patch shows an API test previously dealing with this case.
2. Enum works fine with the usual cases for this API:
a. Deny always: return WKPermissionDecisionDeny
b. Grant always: return WKPermissionDecisionGrant
c. Grant only microphone: return type == WKMediaCaptureTypeMicrophone ? WKPermissionDecisionGrant : WKPermissionDecisionDeny
d. Grant only camera: return type == WKMediaCaptureTypeCamera ? WKPermissionDecisionGrant : WKPermissionDecisionDeny
3. Enum works fine when implementing prompting:
switch (type) {
case WKMediaCaptureTypeCamera:
    return "Allow access to camera(s)";
case WKMediaCaptureTypeMicrophone:
    return "Allow access to microphone(s)";
case WKMediaCaptureTypeCameraAndMicrophone:
    return "Allow access to camera(s) and microphone(s)";
}

[youenn fablet]

Comment on attachment 424400 [details]
Patch

WPE error unrelated

[Eric Carlson]

Comment on attachment 424400 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=424400&action=review

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.h:377
> + @param state state to apply for capture.

s/state state/state State/

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.h:378
> + @param completionHandler A block to invoke with the return value of the function call.

The function has not return value, so maybe "A block to invoke after the camera state has been changed.

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.h:388
> + @param state state to apply for capture.
> + @param completionHandler A block to invoke with the return value of the function call.

Ditto the above.

[Sam Weinig]

(In reply to youenn fablet from comment #5)
> (In reply to Sam Weinig from comment #4)
> > Comment on attachment 424400 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=424400&action=review
> > 
> > > Source/WebKit/UIProcess/API/Cocoa/WKUIDelegate.h:58
> > > +typedef NS_ENUM(NSInteger, WKMediaCaptureType) {
> > > +    WKMediaCaptureTypeCamera,
> > > +    WKMediaCaptureTypeMicrophone,
> > > +    WKMediaCaptureTypeCameraAndMicrophone,
> > > +} WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
> > 
> > This seems like something an NS_OPTIONS style bit mask would be better
> > suited for, and therefore you wouldn't need an explicit
> > WKMediaCaptureTypeCameraAndMicrophone value.
> 
> We could go both ways. I
> went this way for two reasons:
> 1. With a bit field, there is the theoretical possibility of asking for 'no
> camera' and 'no microphone'. Patch shows an API test previously dealing with
> this case.
> 2. Enum works fine with the usual cases for this API:
> a. Deny always: return WKPermissionDecisionDeny
> b. Grant always: return WKPermissionDecisionGrant
> c. Grant only microphone: return type == WKMediaCaptureTypeMicrophone ?
> WKPermissionDecisionGrant : WKPermissionDecisionDeny
> d. Grant only camera: return type == WKMediaCaptureTypeCamera ?
> WKPermissionDecisionGrant : WKPermissionDecisionDeny
> 3. Enum works fine when implementing prompting:
> switch (type) {
> case WKMediaCaptureTypeCamera:
>     return "Allow access to camera(s)";
> case WKMediaCaptureTypeMicrophone:
>     return "Allow access to microphone(s)";
> case WKMediaCaptureTypeCameraAndMicrophone:
>     return "Allow access to camera(s) and microphone(s)";
> }

The biggest problem with this approach is it leads no real room for new types in the future without exponential items to cover all combinations. That seems like a bigger deal than documenting that you must pass in at least one type for it to be meaningful.

[youenn fablet]

> The biggest problem with this approach is it leads no real room for new
> types in the future without exponential items to cover all combinations.
> That seems like a bigger deal than documenting that you must pass in at
> least one type for it to be meaningful.

This delegate is scoped to getUserMedia which by design is dedicated to microphone and/or camera. Hence why the initial proposal was booleans. I changed to an enum as it is as nice or nicer to developers and might still allow some extension, should we see the need in the future.

I am not sure which additional future type we are talking about here.
One known feature in that area is screen sharing.
Though we have no concrete plan yet on how to control screen sharing, we think another delegate might be a better fit.

We plan to have specific delegates for geolocation or device motion orientation as well.

[Sam Weinig]

(In reply to youenn fablet from comment #9)
> > The biggest problem with this approach is it leads no real room for new
> > types in the future without exponential items to cover all combinations.
> > That seems like a bigger deal than documenting that you must pass in at
> > least one type for it to be meaningful.
> 
> This delegate is scoped to getUserMedia which by design is dedicated to
> microphone and/or camera. Hence why the initial proposal was booleans. I
> changed to an enum as it is as nice or nicer to developers and might still
> allow some extension, should we see the need in the future.

getUserMedia is scoped to microphone and/or camera *today*. There may very well be other sensors getUserMedia will deal with in the future. When designing APIs, we should be cognizant that we don't know what the future brings, and design APIs to accommodate.

[youenn fablet]

> getUserMedia is scoped to microphone and/or camera *today*.

Right and getUserMedia model is far from being perfect so I hope we keep it that way.

> There may very well be other sensors getUserMedia will deal with in the future.

It would not be great to ask users a question like: give me "camera AND microphone AND xray AND telescope" or fail.
CameraAndMicrophone combo is an exception, given how this API is biased towards video conferencing.

[Sam Weinig]

Ok, that’s fair I guess.

[youenn fablet]

Created attachment 424530 [details]
Patch for landing

[EWS]

Committed r275162: <https://commits.webkit.org/r275162>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424530 [details].

[Radar WebKit Bug Importer]

<rdar://problem/75957792>

[youenn fablet]

*** Bug 220416 has been marked as a duplicate of this bug. ***

[Kyle Dunn]

Youenn, thanks for getting this resolved! Is there a way for me to determine when this will land in a public iOS build? Or will you update this issue as the time comes?

[Kyle Dunn]

FYI - this appears to have landed in iOS 15. The particulars for this addition are demo'd @ 14:40 here: https://developer.apple.com/videos/play/wwdc2021/10032/

[youenn fablet]

*** Bug 233315 has been marked as a duplicate of this bug. ***