[webkit-reviews] review denied: [Bug 104543] MediaStream API: Change the data channel descriptor pattern to a handler pattern : [Attachment 178534] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 10 11:26:32 PST 2012


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Tommy Widenflycht
<tommyw at google.com>'s request for review:
Bug 104543: MediaStream API: Change the data channel descriptor pattern to a
handler pattern
https://bugs.webkit.org/show_bug.cgi?id=104543

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=178534&action=review


> Source/Platform/chromium/public/WebRTCDataChannelHandler.h:39
> +    virtual void setClient(WebRTCDataChannelHandlerClient*) = 0;

Q: do you need to be able to modify the client at runtime?  how about just
passing it to the createDataChannel method?

> Source/Platform/chromium/public/WebRTCDataChannelHandler.h:42
> +    virtual bool reliable() = 0;

nit: this method looks like it is asking a question, so perhaps it should be
named as such?	isReliable?

> Source/Platform/chromium/public/WebRTCDataChannelHandlerClient.h:46
> +    virtual void dataArrived(const WebString&) const = 0;

nit: call this didReceiveData just like WebURLLoaderClient?

> Source/Platform/chromium/public/WebRTCDataChannelHandlerClient.h:47
> +    virtual void dataArrived(const char*, size_t) const = 0;

Q: why are there two versions of this function?  should the first be called
didReceiveStringData and the second didReceiveRawData?	I'm re-using the terms
from WebRTCDataChannelHandler.

> Source/Platform/chromium/public/WebRTCDataChannelHandlerClient.h:48
> +    virtual void error() const = 0;

nit: didReceiveError?


More information about the webkit-reviews mailing list