Bug 199943 – [SOUP] WebSockets: add support for extensions when using web sockets libsoup API

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

This still requires new API in libsoup, see https://gitlab.gnome.org/GNOME/libsoup/merge_requests/76

[Carlos Garcia Campos]

Created attachment 374464 [details]
WIP

[Carlos Garcia Campos]

Youenn, do we have a "formal" way of importing blink tests? I want to use the permessage-deflate tests from blink. Importing them in imported/blink would not work for websocket tests, so it would be easier to just copy them to our websocket tests directory.

[youenn fablet]

Ideally these would be WPT tests, is it something we could do?

Otherwise, I guess we could use http/tests/websockets/imported/blink.

[Carlos Garcia Campos]

(In reply to youenn fablet from comment #3)
> Ideally these would be WPT tests, is it something we could do?

I don't see permessage-deflate tests in wpt

> Otherwise, I guess we could use http/tests/websockets/imported/blink.

Good point, that will work.

[Carlos Garcia Campos]

Created attachment 375306 [details]
Patch

Alex or Youenn, I need a review here, please, I'm currently preparing a new release including the new WebSockets code path that I have to make tomorrow.

[Alex Christensen]

Comment on attachment 375306 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375306&action=review

great

> Source/WebKit/WebProcess/Network/WebSocketChannel.cpp:79
> +    return m_extensions.isNull() ? emptyString(): m_extensions;

space before colon

[Alex Christensen]

Comment on attachment 375306 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=375306&action=review

> Source/WebKit/NetworkProcess/cocoa/WebSocketTaskCocoa.mm:87
> +    m_channel.didConnect(protocol, { });

Actually, could you move the "FIXME: support extensions." comment to here?

[Carlos Garcia Campos]

Committed r248102: <https://trac.webkit.org/changeset/248102>

[Carlos Garcia Campos]

Thanks for the review, fixed both things.