[webkit-reviews] review granted: [Bug 98475] [WebSocket] ExtensionParser should have its own file : [Attachment 167238] Patch

[bugzilla-daemon at webkit.org]

Yuta Kitamura <yutak at chromium.org> has granted Kenichi Ishibashi
<bashi at chromium.org>'s request for review:
Bug 98475: [WebSocket] ExtensionParser should have its own file
https://bugs.webkit.org/show_bug.cgi?id=98475

Attachment 167238: Patch
https://bugs.webkit.org/attachment.cgi?id=167238&action=review

------- Additional Comments from Yuta Kitamura <yutak at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167238&action=review


Mostly looks good. Please fix the nits before landing.

> Source/WebCore/Modules/websockets/WebSocketExtensionParser.cpp:94
> +    m_currentToken = String::fromUTF8(buffer.data(), buffer.size());

(I know this code is moved from another file without modification, but I'm just
curious.
 If you need to change the code, you can do it in a later patch.)

1. Is there any normative reference that says the content of a quoted string
must be
   decoded as UTF-8?
2. What happens if the quoted string contains invalid UTF-8 sequence?

> Source/WebCore/Modules/websockets/WebSocketExtensionParser.h:37
> +#include <wtf/text/StringHash.h>

Is this necessary?

> Source/WebCore/Modules/websockets/WebSocketExtensionParser.h:56
> +    // The following member functions basically follow the grammer defined

nit: "grammar"