[Webkit-unassigned] [Bug 199151] [SOUP] Use libsoup WebSockets API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=199151

--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to youenn fablet from comment #4)
> Nice to see that.
> We have similar test issues with the NSURLSession code path that we will
> need to handle one way or the other.

So, the NSURLSession impl is not used yet in production, and not tested by the bots either?

> I run some of these tests with Chrome and Firefox and some of them fail with
> either one or both, which indicates that these tests might be WebKit
> implementation specific.

I'll prioritize the ones that we knoe are bugs or missing features.

> (In reply to Carlos Garcia Campos from comment #2)
> > As I said most of the tests are failing because of missing console messages,
> > so they are actually working and I won't mention them here.
> 
> Right, I am not sure we will be able to get back the exact same missing
> console messages given that part of the messages are generated by processing
> done by libsoup/nsurlsession now.

Indeed, if we keep using console messages for those events, we will need platform specific test results. I don't know if it's possible to ignore those messages (if we end up adding them back) in layout tests to not show them in the results.

> > 
> > http/tests/websocket/tests/hybi/bad-handshake-crash.html
> > 
> > This needs investigation, "WebSocket is open" message is unexpectedly shown
> > in the output.
> 
> I think we also have an issue there with NSURLSession.
> 
> > http/tests/websocket/tests/hybi/client-close.html
> > 
> > I've debugged this and I don't understand why it fails, the diff is:
> > -PASS closeEvent.reason is "close_frame[:2]='\\x88\\x80'"
> > +FAIL closeEvent.reason should be close_frame[:2]='\x88\x80'. Was
> > close_frame[:2]='\x88\x82'.
> 
> Same issue here with NSURLSession.

Interesting, I debugged it for a while, but couldn't understand what's going on there.

> > http/tests/websocket/tests/hybi/close-code-and-reason.html
> > 
> > This one expects error codes that the spec says shouldn't be used like 1006.
> > But close reason is also failing, so it needs some more investigation.
> > 
> > http/tests/websocket/tests/hybi/close.html
> > 
> > It seems onclose is not called in a couple of cases.
> > 
> > http/tests/websocket/tests/hybi/deflate-frame-parameter.html
> > http/tests/websocket/tests/hybi/extensions.html
> > 
> > x-webkit-deflate-frame extension not supported
> 
> The idea would be to move from x-webkit-deflate-frame to the standard
> version permessage-deflate.

Since it's in a spec now I think we could implement it directly in libsoup, otherwise I'll add new API to libsoup to implement extensions. I guess this doesn't cause any regression in functionality, only that frames are not compressed.

> > 
> > http/tests/websocket/tests/hybi/handshake-fail-by-no-cr.html
> > 
> > Libsoup doesn't fail when status line finishes with \n since that's what the
> > http spec suggests.
> > 
> > http/tests/websocket/tests/hybi/handshake-ok-with-http-version-beyond-1_1.
> > html
> > 
> > There are a couple of cases where libsoup doesn't fail either.
> > 
> > http/tests/websocket/tests/hybi/inspector/*
> > 
> > It seems we are not notifying the inspector in some cases about websockets
> > with the new code path.
> 
> I am not sure we will be ever able to get back the same level of inspector
> control with the new code path.

I'll take a look.

> > http/tests/websocket/tests/hybi/null-character.html
> > 
> > libsoup API always expect a null terminated string for text frames. I
> > haven't found anything in the spec about it, it just says that text frames
> > should contains valid utf8, and null character si perfectly valid, so maybe
> > we need to add new API in libsoup to allow passing char* + length.
> > 
> > http/tests/websocket/tests/hybi/send-object-tostring-check.html
> > 
> > It expects to be closed cleanly, but apparently it isn't.
> > 
> > http/tests/websocket/tests/hybi/simple-wss.html
> > 
> > We don't have a way to accept the invalid certificate only for tests.
> 
> The plan is to reuse the existing NSURLSession code path for these.
> I would hope you could do the same for libsoup.

I'll check it too. We should be able, since the handshake happens in the session like a normal load.

> > http/tests/websocket/tests/hybi/inspector/binary.html
> > http/tests/websocket/tests/hybi/send-blob-onmessage-origin.html
> > http/tests/websocket/tests/hybi/send-blob.html
> > 
> > Blobs are not implemented in new code path.
> 
> Yep, this will be needed to implement.
> This will require to queue additional outgoing messages until the blob is
> read/put in memory and sent.

I'll take a look too.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190625/975b6cef/attachment-0001.html>