[webkit-reviews] review denied: [Bug 99080] MediaStream API: Implement RTCDataChannel : [Attachment 168244] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Tommy Widenflycht
<tommyw at google.com>'s request for review:
Bug 99080: MediaStream API: Implement RTCDataChannel
https://bugs.webkit.org/show_bug.cgi?id=99080

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

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


This patch could use another iteration, but my comments are mostly just naming
nits.

> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:42
> +PassRefPtr<RTCDataChannel> RTCDataChannel::create(ScriptExecutionContext*
context, RTCPeerConnectionHandler* handler, String label, bool reliable,
ExceptionCode& ec)

String -> const String&

> Source/WebCore/Modules/mediastream/RTCDataChannelEvent.idl:30
> +	  readonly attribute RTCDataChannel channel;

Bad indent

> Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:413
> +PassRefPtr<RTCDataChannel> RTCPeerConnection::createDataChannel(String
label, const Dictionary& dataChannelDict, ExceptionCode& ec)

dataChannelDict -> options?

> Source/WebCore/platform/mediastream/RTCDataChannelDescriptor.h:38
> +    class Owner {

Typically we'd call this a client rather than an owner.

Also, we'd usually declare it as a top-level class "RTCDataChannelClient"

It's confusing for a RefCounted object to have an "owner", which is why we tend
not to use that term.