[webkit-reviews] review denied: [Bug 96023] Expose ability to create WebUserMediaRequest from chromium side : [Attachment 162607] 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 162607: Patch
https://bugs.webkit.org/attachment.cgi?id=162607&action=review

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


> Source/WebKit/chromium/public/WebFrame.h:327
> +    virtual WebUserMediaRequest *createWebUserMediaRequest(v8::Isolate*,

WebUserMediaRequest *  ->    WebUserMediaRequest*

Do we need to pass in the isolate here?

Also, we should take v8::Handle as a parameter rather than v8::Local

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1002
> +WebKit::WebUserMediaRequest
*WebFrameImpl::createWebUserMediaRequest(v8::Isolate *isolate,

In WebKit style, the * goes on the other side.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1006
> +    UserMediaController* userMedia = UserMediaController::from(m_frame ?
m_frame->page() : 0);

We should just return early if there's no m_frame.  There's not much point in
continuing.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1008
> +	 return 0;

Four-space indent.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1013
> +	 requestErrorCallback =
V8NavigatorUserMediaErrorCallback::create(errorCallback,
getScriptExecutionContext());

Four-space indent.

> Source/WebKit/chromium/src/WebFrameImpl.cpp:1015
> +    Dictionary requestOptions = Dictionary(options, isolate);

You can just use v8::Isolate::current() here rather than taking the isolate as
a parameter.