Bug 213698

Summary: RTCDataChannel.bufferedAmount should stay the same even if channel is closed
Product: WebKit Reporter: youenn fablet <youennf>
Component: AccessibilityAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson, ews-watchlist, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description youenn fablet 2020-06-28 08:16:46 PDT
RTCDataChannel.bufferedAmount should stay the same even if channel is closed
Comment 1 youenn fablet 2020-06-28 08:24:31 PDT
Created attachment 402996 [details]
Patch
Comment 2 youenn fablet 2020-06-28 09:43:53 PDT
Created attachment 403000 [details]
Patch
Comment 3 Darin Adler 2020-06-28 13:14:31 PDT
Comment on attachment 403000 [details]
Patch

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

> Source/WebCore/Modules/mediastream/RTCDataChannel.cpp:124
> +    m_bufferedAmount += data.utf8().length();

This is an unnecessarily costly operation. It allocates memory for a UTF-8 string just to check its length and then deallocate the memory. If we need to do this, we could make an efficient function that computes this length without allocating and deallocating memory. Another option would be to carry this around already encoded as UTF-8 rather than converting from a String to UTF-8 twice. I think that would mean putting a CString into the message queue? Or maybe the String can be UTF-8 encoded into some other class that the code knows how to handle already?
Comment 4 Darin Adler 2020-06-28 13:15:15 PDT
Comment on attachment 403000 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Test: webrtc/datachannel/bufferedAmount-afterClose.html

Seems unlikely that this test covers all 4 different types of send; if so we should make a more exhaustive test.
Comment 5 Radar WebKit Bug Importer 2020-06-28 13:15:29 PDT
<rdar://problem/64866305>
Comment 6 youenn fablet 2020-06-29 02:22:32 PDT
I filed https://bugs.webkit.org/show_bug.cgi?id=213714 to optimise it.
Will update test
Comment 7 youenn fablet 2020-06-29 02:23:39 PDT
Created attachment 403039 [details]
Patch for landing
Comment 8 youenn fablet 2020-06-29 02:24:42 PDT
Had to update BlobLoader since it is not guaranteed to always call its completion handler (in case load is pending).
Comment 9 EWS 2020-06-29 04:50:08 PDT
Committed r263655: <https://trac.webkit.org/changeset/263655>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403039 [details].