Hi Pavel,
several comments:
1. WebSocket.request(long n) is documented as "
@throws IllegalArgumentException if n < -1
"
It looks meaningless to allow 0.
2. Some concern on the way of handling close. Consider following scenario.
* obtain a ws connection
* message communications balabala...
* initialize closing handshake on either client/server peer
* if the client side didn't request adequate size or not requested at
all, it actually doesn't know connection is closed. "onClose" will
never be invoked, because close message is at the end of "queue".
* it looks to me that:
o the closing handshake can be blocked by UN-requested messages.
o a websocket can be in a state that isClosed() return false but
it can't send any message. Note that the state is not transient.
Filed a bug https://bugs.openjdk.java.net/browse/JDK-8156191. It looks
current design treats CloseMessage in similar way with other messages.
I'm not sure if it is worth something like WSClosingHandshake.
3. I'm concerning the bug in #2 is side-effect of reactive mode. May I
ask what is the benefit of reactive mode. WebSocket.request(long n) seem
to be good in first glance but:
* It looks end users need to calculate how many messages are expected.
The another question comes
o what messages will be taken into account. How about PING/PONG or
else?
* We may use request( a big value up to Long.MAX). But, in this case,
why we need it? This looks simple but a bit problematic.
My $0.02,
Felix
On 2016/5/3 23:23, Pavel Rappo wrote:
Hello,
Here's an updated webrev with the latest implementation:
http://cr.openjdk.java.net/~prappo/8087113/webrev.04/
If you're going to review it, please be advised that there is a number of known
API and implementation issues yet to be resolved [1]. More API tests will be
provided in the next couple of months.
All applicable tests (1.1.1-10.1.1, 305 in total) from Autobahn Testsuite [2]
are green.
Thanks,
-Pavel
--------------------------------------------------------------------------------
[1] https://bugs.openjdk.java.net/browse/JDK-8155621
[2] http://autobahn.ws/testsuite/
On 25 Mar 2016, at 16:21, Pavel Rappo <pavel.ra...@oracle.com> wrote:
Hi,
Could you please review my change for JDK-8087113
http://cr.openjdk.java.net/~prappo/8087113/webrev.03/
This webrev contains initial implementation and basic tests for WebSocket API.
Specification conformance & interoperability testing has been performed with
Autobahn Testsuite [1]. As for API tests, I will provide more of them later.
Thanks,
-Pavel
--------------------------------------------------------------------------------
[1] http://autobahn.ws/testsuite/
