RESOLVED FIXED Bug 215563
NetworkSocketChannel should delay sending back error message as done by NetworkSocketStream
https://bugs.webkit.org/show_bug.cgi?id=215563
Summary NetworkSocketChannel should delay sending back error message as done by Netwo...
youenn fablet
Reported 2020-08-17 03:17:58 PDT
Delay sending back to WebProcess WebSocket error message as done in NetworkSocketStream
Attachments
Patch (3.92 KB, patch)
2020-08-17 03:20 PDT, youenn fablet
no flags
youenn fablet
Comment 1 2020-08-17 03:20:51 PDT
Alex Christensen
Comment 2 2020-08-17 09:03:05 PDT
Comment on attachment 406709 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406709&action=review > Source/WebKit/NetworkProcess/NetworkSocketChannel.h:94 > + String m_errorMessage; It would be nice if we could make a version of Timer::startOneShot that takes a lambda so these could be in a lambda capture instead of members of this object. > Source/WebKit/NetworkProcess/NetworkSocketChannel.h:95 > + Optional<std::pair<unsigned short, String>> m_closeInfo; Could we make this uint16_t?
youenn fablet
Comment 3 2020-08-18 08:13:24 PDT
(In reply to Alex Christensen from comment #2) > Comment on attachment 406709 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406709&action=review > > > Source/WebKit/NetworkProcess/NetworkSocketChannel.h:94 > > + String m_errorMessage; > > It would be nice if we could make a version of Timer::startOneShot that > takes a lambda so these could be in a lambda capture instead of members of > this object. Agreed, but here we have two call sites, one for the error message and one for code and reason. > > Source/WebKit/NetworkProcess/NetworkSocketChannel.h:95 > > + Optional<std::pair<unsigned short, String>> m_closeInfo; > > Could we make this uint16_t? We probably could since it is an unsigned short for WebIDL as well. But this would not be very consistent with other parts of the WebSocket implementation.
EWS
Comment 4 2020-08-18 08:16:06 PDT
Committed r265814: <https://trac.webkit.org/changeset/265814> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406709 [details].
Radar WebKit Bug Importer
Comment 5 2020-08-18 08:17:17 PDT
Note You need to log in before you can comment on or make changes to this bug.