[Webkit-unassigned] [Bug 65835] Need a way to selectively use hixie-76 for websocket connections depending on destination and/or origin

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=65835





--- Comment #2 from Yuta Kitamura <yutak at chromium.org>  2011-08-07 19:42:06 PST ---
(From update of attachment 103192)
View in context: https://bugs.webkit.org/attachment.cgi?id=103192&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

You need to justify the reason why you don't add a new test.

> Source/WebCore/page/Settings.cpp:810
> +    KURL tmp(url);

We usually avoid using abbreviated words. See "Names" section of <http://www.webkit.org/coding/coding-style.html>.

> Source/WebCore/page/Settings.h:566
> +        HashMap<AtomicString, bool> m_hixie76WebSocketProtocolExceptionList;

I think this line should be moved somewhere else because it divides the bit field into two sections.

Why AtomicString? I think String is better because we may compare it with any arbitrary URLs.

> Source/WebCore/websockets/WebSocketChannel.cpp:115
> +    ASSERT(m_handle); // Hixie76 vs HiBy decision is not made before connect() call.

This assumption conflicts with what I'm working on; the protocol decision must be available after WebSocketChannel construction (not after connect()), because we need to check the value of subprotocols (provided in JavaScript's WebSocket constructor) before the actual connection is attempted.

You probably need to move the "url" argument from connect() to WebSocketChannel constructor. See also: bug 65367

Typo: "HiBy" -> "HyBi"

> Source/WebKit/chromium/src/WebSettingsImpl.cpp:453
> +    UNUSED_PARAM(origin_list);

origin_list -> url

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.