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
