[Webkit-unassigned] [Bug 43239] Support for HTML Media Capture API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=43239

Sam Weinig <sam at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #294874|review?                     |review-
              Flags|                            |

--- Comment #27 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 294874
  --> https://bugs.webkit.org/attachment.cgi?id=294874
Proposed Patch V9

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

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.h:118
> +#if PLATFORM(IOS)
> +        WKUserMediaCaptureDeviceAuthorizerIOS *captureDeviceAuthorizer();
> +        RetainPtr<WKUserMediaCaptureDeviceAuthorizerIOS> m_captureDeviceAuthorizer;
> +#endif

This class really shouldn't have any data members.  Can this be merged into WebPageProxy.

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.h:119
> +        WebCore::MediaProducer::MediaStateFlags m_mediaState { WebCore::MediaProducer::IsNotPlaying };

Ditto.

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:375
> +    void (^cameraAuthorizationBlock)(void) = ^ {
> +        if (requiresVideo) {
> +            [captureDeviceAuthorizer() requestCameraAuthorization:^(BOOL authorized) {
> +                if (!authorized)
> +                    request.deny(UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied);
> +                else
> +                    uiDelegateAuthorizationBlock();
> +            }];
> +        } else
> +            uiDelegateAuthorizationBlock();
> +    };
> +
> +    if (requiresAudio) {
> +        [captureDeviceAuthorizer() requestMicrophoneAuthorization:^(BOOL authorized) {
> +            if (!authorized)
> +                request.deny(UserMediaPermissionRequestProxy::UserMediaAccessDenialReason::PermissionDenied);
> +            else
> +                cameraAuthorizationBlock();
> +        }];
> +    } else
> +        cameraAuthorizationBlock();

Why does this need to be iOS specific?

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:395
> +    [(id <WKUIDelegatePrivate>)delegate _webView:webView checkUserMediaPermissionForURL:requestFrameURL mainFrameURL:mainFrameURL frameIdentifier:frame.frameID() decisionHandler:^(NSString *salt, BOOL authorized) {

It's a bit odd that you pass SecurityOrigin's to this level, but then don't use them, but rather get URLs.

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:402
> +void UIDelegate::UIClient::isPlayingAudioDidChange(WebKit::WebPageProxy& page)

It is really weird that isPlayingAudioDidChange is what triggers _webViewDidBeginCaptureSession/_webViewDidEndCaptureSession.  This should be re-worked to be more direct.

> Source/WebKit2/UIProcess/ios/WKUserMediaCaptureDeviceAuthorizerIOS.h:28
> + at interface WKUserMediaCaptureDeviceAuthorizerIOS : NSObject

As this class is not AP/SPII, it should not have the WK prefix.  I am generally confused about why this class is necessary, as it seems to store no state.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161117/62f12131/attachment.html>