[webkit-reviews] review denied: [Bug 169732] Clean up RTCDataChannel : [Attachment 305862] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has denied youenn fablet <youennf at gmail.com>'s
request for review:
Bug 169732: Clean up RTCDataChannel
https://bugs.webkit.org/show_bug.cgi?id=169732

Attachment 305862: Patch

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




--- Comment #17 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 305862
  --> https://bugs.webkit.org/attachment.cgi?id=305862
Patch

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

> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:219
> +void RTCDataChannel::bufferedAmountIsDecreasing(size_t amount)

Could you explain what you updated the function to take a parameter? It now
causes us to call bufferedAmount() at the call site, even if m_stopped is true,
which requires us to deal with m_stopped being true in bufferedAmount().

>
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp:10
8
> +	  
protectedClient->bufferedAmountIsDecreasing(static_cast<size_t>(amount));

Why do we need the cast? I thought buffered_amount() returned a size_t already?

> LayoutTests/webrtc/datachannel/bufferedAmountLowThreshold-expected.txt:4
> +Harness Error (FAIL), message = InvalidStateError (DOM Exception 11): The
object is in an invalid state.

Is this expected?

> LayoutTests/webrtc/datachannel/datachannel-event.html:23
> +    assert_equals(event.type, "RTCDataChannelEvent");

I believe this should be "test", not "RTCDataChannelEvent"

> LayoutTests/webrtc/datachannel/datachannel-event.html:24
> +    assert_equals(event.isTrusted, true, "trusted");

This seems wrong. How can an event created by JS be trusted?