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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 30 08:51:21 PDT 2017

Chris Dumez <cdumez at apple.com> has denied youenn fablet <youennf at gmail.com>'s
request for review:
Bug 169732: Clean up RTCDataChannel

Attachment 305862: Patch


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

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().

> +	  

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?

More information about the webkit-reviews mailing list