[webkit-reviews] review denied: [Bug 81878] [WebSocket]Reserved1 bit should be 0 when no negotiated extension : [Attachment 133201] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 22 17:15:06 PDT 2012


Benjamin Poulain <benjamin at webkit.org> has denied joey <li.yin at intel.com>'s
request for review:
Bug 81878: [WebSocket]Reserved1 bit should be 0 when no negotiated extension
https://bugs.webkit.org/show_bug.cgi?id=81878

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=133201&action=review


> Source/WebCore/ChangeLog:7
> +	   [WebSocket]Reserved1 bit should be 0 when no negotiated extension
> +	   https://bugs.webkit.org/show_bug.cgi?id=81878
> +
> +	   Reviewed by NOBODY (OOPS!).
> +

I think it is worth adding a short explanation in this bug, linking the
relevant spec.

> Source/WebCore/ChangeLog:8
> +	   No new tests, because it was covered by reserved-bits.html

This is not right.
If it was covered, why did you have to modify the test?

> LayoutTests/http/tests/websocket/tests/hybi/reserved-bits.html:49
> -doTest(2);
> +doTest(1);

This seems bogus, you are changing the coverage of an existing test.

The function doTest() also has a comment made false with this patch: "//
bitNumber must be 2 or 3.".

> LayoutTests/http/tests/websocket/tests/hybi/reserved-bits_wsh.py:7
> -    pass
> +    request.ws_extension_processors = []

This is most mysterious. An explanation in the ChangeLog would be welcome.


More information about the webkit-reviews mailing list