Bug 213698 – RTCDataChannel.bufferedAmount should stay the same even if channel is closed

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[youenn fablet]

RTCDataChannel.bufferedAmount should stay the same even if channel is closed

[youenn fablet]

Created attachment 402996 [details]
Patch

[youenn fablet]

Created attachment 403000 [details]
Patch

[Darin Adler]

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?

[Darin Adler]

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.

[Radar WebKit Bug Importer]

<rdar://problem/64866305>

[youenn fablet]

I filed https://bugs.webkit.org/show_bug.cgi?id=213714 to optimise it.
Will update test

[youenn fablet]

Created attachment 403039 [details]
Patch for landing

[youenn fablet]

Had to update BlobLoader since it is not guaranteed to always call its completion handler (in case load is pending).

[EWS]

Committed r263655: <https://trac.webkit.org/changeset/263655>

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