Bug 200162

Summary:

[SOUP] Switch to use libsoup WebSockets API

Product:

WebKit

Reporter:

Carlos Garcia Campos <cgarcia>

Component:

WebKit2

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, bugs-noreply, mcatanzaro, tsaunier, webkit-bug-importer

Priority:

Keywords:

Gtk, InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

199943, 200161

Bug Blocks:

Attachments:

Description	Flags
Patch	mcatanzaro: review+
Patch for landing	none
Patch for landing	none
Patch for landing	none
Patch for landing	none
Patch for landing	none
Patch for landing	none

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+	Details Formatted Diff Diff
Patch for landing (389.29 KB, patch) 2019-08-01 02:56 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
Patch for landing (389.29 KB, patch) 2019-08-01 03:01 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
Patch for landing (390.23 KB, patch) 2019-08-01 03:20 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
Patch for landing (390.77 KB, patch) 2019-08-01 03:35 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
Patch for landing (390.80 KB, patch) 2019-08-01 03:41 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
Patch for landing (390.83 KB, patch) 2019-08-01 03:51 PDT, Carlos Garcia Campos	no flags	Details Formatted Diff Diff
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.