RFR [9] 8179021: Latest bugfixes to WebSocket/HPACK from the sandbox repo

[Daniel Fuchs]

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
>