RESOLVED FIXED200162
[SOUP] Switch to use libsoup WebSockets API 
https://bugs.webkit.org/show_bug.cgi?id=200162
Summary [SOUP] Switch to use libsoup WebSockets API
Carlos Garcia Campos
Reported 2019-07-26 04:08:28 PDT
Switch to use libsoup WebSockets API
Attachments
Patch (442.33 KB, patch)
2019-07-26 05:45 PDT, Carlos Garcia Campos 		mcatanzaro: review+
DetailsFormatted DiffDiff
Patch for landing (389.29 KB, patch)
2019-08-01 02:56 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Patch for landing (389.29 KB, patch)
2019-08-01 03:01 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Patch for landing (390.23 KB, patch)
2019-08-01 03:20 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Patch for landing (390.77 KB, patch)
2019-08-01 03:35 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Patch for landing (390.80 KB, patch)
2019-08-01 03:41 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Patch for landing (390.83 KB, patch)
2019-08-01 03:51 PDT, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2019-07-26 05:45:07 PDT
Created attachment 374961 [details] Patch
Michael Catanzaro
Comment 2 2019-07-26 08:55:03 PDT
Comment on attachment 374961 [details] Patch Code changes look good of course. Would be nice to see this pass EWS before landing. Looks like we'll need Mac-specific test results. I guess we could ask Apple to provide those results if it's hard to get them off of EWS.
Michael Catanzaro
Comment 3 2019-07-26 08:55:44 PDT
GTK EWS is just failing to apply the patch. :)
Carlos Garcia Campos
Comment 4 2019-07-26 09:00:41 PDT
(In reply to Michael Catanzaro from comment #2) > Comment on attachment 374961 [details] > Patch > > Code changes look good of course. Would be nice to see this pass EWS before > landing. Looks like we'll need Mac-specific test results. I guess we could > ask Apple to provide those results if it's hard to get them off of EWS. I'll submit a patch for landing once dependencies land. This doesn't affect Apple, it's only enabled for libsoup based ports.
Carlos Garcia Campos
Comment 5 2019-07-26 09:01:03 PDT
(In reply to Michael Catanzaro from comment #3) > GTK EWS is just failing to apply the patch. :) Of course, please see the dependencies.
Michael Catanzaro
Comment 6 2019-07-26 09:06:06 PDT
Comment on attachment 374961 [details] Patch Well I see new cross-platform tests that do not have cross-platform results and are not skipped: http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-unsolicited-negotiation-response.html http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-invalid-parameter.html: Added. http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-parameter.html: Added. http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-set-bfinal.html: Added. http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-split-frames.html: Added. http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-unsolicited-negotiation-response.html: Added. http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-window-bits.html: Added.
Carlos Garcia Campos
Comment 7 2019-07-29 01:27:14 PDT
(In reply to Michael Catanzaro from comment #6) > Comment on attachment 374961 [details] > Patch > > Well I see new cross-platform tests that do not have cross-platform results > and are not skipped: > > http/tests/websocket/tests/hybi/imported/blink/permessage-deflate- > unsolicited-negotiation-response.html > http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-invalid- > parameter.html: Added. > http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-parameter. > html: Added. > http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-set-bfinal. > html: Added. > http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-split- > frames.html: Added. > http/tests/websocket/tests/hybi/imported/blink/permessage-deflate- > unsolicited-negotiation-response.html: Added. > http/tests/websocket/tests/hybi/imported/blink/permessage-deflate-window- > bits.html: Added. Ah! you are right, I forgot to skip those.
Alex Christensen
Comment 8 2019-07-30 10:22:59 PDT
I like everything about this.
Michael Catanzaro
Comment 9 2019-07-30 11:01:05 PDT
Comment on attachment 374961 [details] Patch Me too, so much so that I forgot I had not already given r+.... Just one thing: the end goal should be to remove SocketStreamHandleImpl altogether for platforms with platform-native sockets, not leave it with RELEASE_ASSERTs. This means that we might need to eventually introduce a new build flag, something like #if USE(PLATFORM_WEBSOCKETS) that will be equivalent to #if HAVE(NSURLSESSION_WEBSOCKET) || USE(SOUP). But that can be future.
Michael Catanzaro
Comment 10 2019-07-30 11:01:41 PDT
Do try to pass EWS before landing, of course.
Carlos Garcia Campos
Comment 11 2019-08-01 02:56:23 PDT
Created attachment 375297 [details] Patch for landing
Carlos Garcia Campos
Comment 12 2019-08-01 03:01:43 PDT
Created attachment 375298 [details] Patch for landing
Carlos Garcia Campos
Comment 13 2019-08-01 03:20:14 PDT
Created attachment 375299 [details] Patch for landing
Carlos Garcia Campos
Comment 14 2019-08-01 03:35:32 PDT
Created attachment 375300 [details] Patch for landing
Carlos Garcia Campos
Comment 15 2019-08-01 03:41:01 PDT
Created attachment 375301 [details] Patch for landing
Carlos Garcia Campos
Comment 16 2019-08-01 03:51:01 PDT
Created attachment 375302 [details] Patch for landing
Carlos Garcia Campos
Comment 17 2019-08-01 04:46:14 PDT
Committed r248099: <https://trac.webkit.org/changeset/248099>
Radar WebKit Bug Importer
Comment 18 2019-08-01 04:47:20 PDT
<rdar://problem/53809124>
Thibault Saunier
Comment 19 2019-08-06 11:29:34 PDT
This broke appr.tc, with that patch I am now always getting "WebSocket error" trying to create a call in appr.tc.
Note You need to log in before you can comment on or make changes to this bug.