Bug 213698 - RTCDataChannel.bufferedAmount should stay the same even if channel is closed
Summary: RTCDataChannel.bufferedAmount should stay the same even if channel is closed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-28 08:16 PDT by youenn fablet
Modified: 2020-06-29 04:50 PDT (History)
11 users (show)

See Also:


Attachments
Patch (5.82 KB, patch)
2020-06-28 08:24 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (8.13 KB, patch)
2020-06-28 09:43 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (13.50 KB, patch)
2020-06-29 02:23 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].