[webkit-reviews] review denied: [Bug 96023] Expose ability to create WebUserMediaRequest from chromium side : [Attachment 165967] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Justin Lin <justinlin at chromium.org>'s
request for review:
Bug 96023: Expose ability to create WebUserMediaRequest from chromium side
https://bugs.webkit.org/show_bug.cgi?id=96023

Attachment 165967: Patch
https://bugs.webkit.org/attachment.cgi?id=165967&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=165967&action=review


This still strikes me as the wrong approach.  It's tough to write correct
custom bindings in the first place.  It's even tougher to write them in the
wrong layer (i.e., the WebKit layer rather than the bindings themselves).

> Source/WebKit/chromium/ChangeLog:6
> +	   Reviewed by Adam Barth.

This is not accurate.  I have not reviewed this patch.	Please leave the NOBODY
(OOPS!) line in the ChangeLog.	The tools will fill it out with the proper
reviewer when landing your patch.

> Source/WebKit/chromium/src/WebDeviceMediaRequest.cpp:54
> +    WebFrame* webFrame = WebFrame::frameForCurrentContext();

It's very unlikely that we should call frameForCurrentContext.

> Source/WebKit/chromium/src/WebDeviceMediaRequest.cpp:61
> +    ScriptExecutionContext* scriptExecutionContext =
frame->document()->scriptExecutionContext();

This is really the wrong way to get the current script execution context.  The
bindings know how to do this work properly.  Doing this work from the WebKit
layer is pretty awkward.