On 3 May 2016, at 16:23, Pavel Rappo <pavel.ra...@oracle.com> wrote: > Hello, > > Here's an updated webrev with the latest implementation: > > http://cr.openjdk.java.net/~prappo/8087113/webrev.04/
This looks much better, more straight forward. I will ignore any TODO’s and items mentioned in 8155621, which I assume will be addressed as incremental improvements after this goes in. FWIW, I’m glad that you will be making simplifications around handling of TEXT. WSMessagePublisher - The Consumer<Throwabe> that is passed to the sender, calls react() when a message is completed. It does this so that the removal and completion are done in a single place, but won’t this blow the stack if there is a continuous flow of outbound messages available? If so, would it be better to complete the future, remove from the backlog, and then signal ? WSShared - I see that there are number of simplifications that can be made once 8150785 is resolved. Good. Also, there us some overlap where with ByteBufferGenerator/Consumer, so there may be a future opportunity for consolidation, at least in some parts WSReceivedMessages - I think that the back pressure is implemented, correctly, as the number of invocations of the listener callbacks, but the spec may need a minor clarification. It is not a unit of message or frame. It can be as little as a single byte. Not an immediate issue, but we should revisit this as part of 8155621 -Chris. > 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/ >> >
