Hi Pavel, Looks good. Some files didn't have their copyright years updated - maybe you could fix that before pushing.
jdk/incubator/http/internal/websocket/OpeningHandshake.java line 121: It might be worth mentioning that '13' is mandated by [RFC 6455](https://tools.ietf.org/html/rfc6455) best regards, -- daniel On 10/05/2017 12:03, Pavel Rappo wrote:
Daniel, thanks a lot for looking into this! I've addressed most of your suggestions: http://cr.openjdk.java.net/~prappo/8179021/webrev.01/On 9 May 2017, at 15:49, Daniel Fuchs <daniel.fu...@oracle.com> wrote: Hi Pavel, A few nits: jdk/incubator/http/WebSocket.java: line 501: should use {@linkplain Also (line 46 and 501) the link target #sendClose may cause trouble if sendClose is overloaded - so I wonder if it may be better to have the full prototype there: #sendClose(int, String) (though I guess it's OK as long as there is only 1 sendClose method). jdk/incubator/http/internal/websocket/OutgoingMessage.java 75 // maskingKeys, CharsetDecoder and should be here what does this means? are some words missing? jdk/incubator/http/internal/websocket/Transmitter.java 58 * The supplied handler may be invoked in the calling thread, so watch out 59 * for stack overflow, if there's a possibility of send being called again 60 * from the handler. Can we reformulate this again? Something like: * The supplied handler may be invoked in the calling thread. * A {@code StackOverflowException} may thus occur if there's a * possibility that this method is called again by the supplied * handler. 93 // or tuned off I guess you meant 'turned off' jdk/incubator/http/internal/websocket/WebSocketImpl.java 196 if (alreadyCompleted) { 197 throw new InternalError(); ... 321 if (!added) { 322 throw new InternalError(); A comment similar to the other places where InternalError is thrown would be nice. best regards, -- daniel On 09/05/2017 14:52, Pavel Rappo wrote:Hello, Please review the following change: http://cr.openjdk.java.net/~prappo/8179021/webrev.00/ Along with refactoring this change contains a number of critical fixes for WebSocket. Critical fixes are applied to Receiver, WebSocketImpl and OpeningHandshake. Also this change removes 2 convenience methods and 1 constant from WebSocket interface. Since this interface is a part of incubation feature there shouldn't be any problem. Those members were initially controversial. Thanks, -Pavel
