[webkit-reviews] review granted: [Bug 174096] [MediaStream] Protect request and web view during gUM client callback : [Attachment 314487] Proposed patch.

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 174096: [MediaStream] Protect request and web view during gUM client
callback
https://bugs.webkit.org/show_bug.cgi?id=174096

Attachment 314487: Proposed patch.

https://bugs.webkit.org/attachment.cgi?id=314487&action=review




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 314487
  --> https://bugs.webkit.org/attachment.cgi?id=314487
Proposed patch.

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

> Source/WebKit2/ChangeLog:9
> +	   Retain the message and WebView during asynchronous calls so they
won't be 

s/message/request/?
It is not totally clear to me why there is a need to retain both request and
WebView. Can you detail why?

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:430
> +    [delegate _webView:protectedWebView.get()
requestUserMediaAuthorizationForDevices:devices url:requestFrameURL
mainFrameURL:mainFrameURL decisionHandler:decisionHandler.get()];

I am not very familiar with ObjC and BlockPtr.
I guess there is no destruction issue but just in case, I guess BlockPtr is
different from unique_ptr for instance.
why are we doing decisionHandler.get() here?

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:449
> +    auto requestCameraAuthorization = BlockPtr<void()>::fromCallable([this,
&frame, protectedRequest = makeRef(request), webView =
RetainPtr<WKWebView>(m_uiDelegate.m_webView)]() {

requestCameraAuthorization is used in cases where this could be called
synchronously.
Maybe we could make the inside of requestCameraAuthorization a helper method so
that we would capture the request and the web view only in the case we need to
call the helper method asynchronously?