Closed
Bug 1178091
Opened 9 years ago
Closed 8 years ago
Implement RTC-DataChannel-bufferedAmountLow event
Categories
(Core :: WebRTC: Networking, defect, P2)
Core
WebRTC: Networking
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
backlog | webrtc/webaudio+ |
People
(Reporter: shacharz, Assigned: jesup)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
20.62 KB,
patch
|
smaug
:
review+
drno
:
review+
|
Details | Diff | Splinter Review |
4.13 KB,
patch
|
drno
:
review+
|
Details | Diff | Splinter Review |
Comply to spec: https://github.com/w3c/webrtc-pc/pull/233/files Chrome's equivalent issue: https://code.google.com/p/chromium/issues/detail?id=496700
Updated•9 years ago
|
Component: Web Apps → WebRTC
Product: Firefox → Core
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → rjesup
Blocks: webrtc_spec
backlog: --- → webRTC+
Rank: 25
Component: WebRTC → WebRTC: Networking
Priority: -- → P2
Assignee | ||
Comment 1•8 years ago
|
||
tested locally and works with a modified data_test.html; a second patch is coming to add it to the tests in automation. Note that bufferedAmount can't currently include data buffered in the stack; Michael Tuexen has that on his todo list (see comments)
Attachment #8666376 -
Flags: review?(drno)
Attachment #8666376 -
Flags: review?(bugs)
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8666381 -
Flags: review?(drno)
Assignee | ||
Updated•8 years ago
|
Rank: 25 → 23
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4076c821c374
Updated•8 years ago
|
Keywords: dev-doc-needed
Comment 5•8 years ago
|
||
Comment on attachment 8666376 [details] [diff] [review] Implement RTCDataChannel BufferedAmountLowThreshold and bufferedamountlow event Please don't add any onfoo properties to .idl, only to .webidl. And do we need attribute unsigned long bufferedAmountLowThreshold; in the .idl either? .webidl is the thing exposed to scripts these days. (if you end up modifying .idl, update uuid there.) Those fixed, r+ Didn't review netwerk/sctp/datachannel/DataChannel.cpp
Attachment #8666376 -
Flags: review?(bugs) → review+
Comment 6•8 years ago
|
||
Comment on attachment 8666376 [details] [diff] [review] Implement RTCDataChannel BufferedAmountLowThreshold and bufferedamountlow event Review of attachment 8666376 [details] [diff] [review]: ----------------------------------------------------------------- LGTM ::: netwerk/sctp/datachannel/DataChannel.cpp @@ +2640,5 @@ > DataChannel::GetBufferedAmount() > { > + size_t buffered = 0; > + for (auto& buffer : mBufferedData) { > + buffered += buffer->mLength; With real paranoia this can already overflow.
Attachment #8666376 -
Flags: review?(drno) → review+
Comment 7•8 years ago
|
||
Comment on attachment 8666381 [details] [diff] [review] tests for large transfers and onbufferedamountlow event Review of attachment 8666381 [details] [diff] [review]: ----------------------------------------------------------------- LGTM ::: dom/media/tests/mochitest/dataChannel.js @@ +169,5 @@ > ]; > > +var commandsCheckLargeXfer = [ > + function SEND_BIG_BUFFER(test) { > + var buffer = new ArrayBuffer(1024*1024); Would be nice to fill the buffer with some test data rather then just 0's.
Attachment #8666381 -
Flags: review?(drno) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/23cd3f0edf0a https://hg.mozilla.org/integration/mozilla-inbound/rev/40e2d33b759e
Comment 9•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/23cd3f0edf0a https://hg.mozilla.org/mozilla-central/rev/40e2d33b759e
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Comment 10•7 years ago
|
||
This was already listed on Firefox 44 for developers, and bufferedAmountLowThreshold was documented, along with onbufferedamountlow, but now bufferedamountlow is documented too. This is now complete.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•