[webkit-reviews] review granted: [Bug 215928] Enable inspection of WebSocket when opening Web Inspector in the middle of the connection like done for the legacy WebSocket implementation : [Attachment 407592] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 215928: Enable inspection of WebSocket when opening Web Inspector in the
middle of the connection like done for the legacy WebSocket implementation
https://bugs.webkit.org/show_bug.cgi?id=215928

Attachment 407592: Patch

https://bugs.webkit.org/attachment.cgi?id=407592&action=review




--- Comment #7 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 407592
  --> https://bugs.webkit.org/attachment.cgi?id=407592
Patch

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

r=me

> Source/WebCore/Modules/websockets/ThreadableWebSocketChannel.h:86
> +    virtual ResourceRequest clientHandshakeRequest(Function<String(const
URL&)>&&) const = 0;

NIT: it'd be nice to create a `using` for this so that it's clear what it's for
(or include a parameter name):
```
    using RequestHeaderCookieExtractor = Function<String(const URL&)>;
    virtual ResourceRequest
clientHandshakeRequest(RequestHeaderCookieExtractor&&) const = 0;
```
(better names welcome ��)

> Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.h:163
> +    // Dummy implementation of inspector related APIs.

please include a comment for the bug for this
```
    // FIXME: <https://webkit.org/b/168475> Web Inspector: Correctly display
iframe's and worker's WebSockets
```

> Source/WebKit/WebProcess/Network/WebSocketChannel.h:106
> +    WebCore::ResourceRequest clientHandshakeRequest(Function<String(const
URL&)>&&) const final { return m_handshakeRequest; }

Will this have the right `Set-Cookie` header in it?