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
>>
>
