----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19355/#review37556 -----------------------------------------------------------
Thank you for working on this issue. I did only a fast read of the diff with some minor comments. I'll review and test more in detail these days. PS: Can you add akaplanov as reviewer? He did the last efforts related with websocket. src/org/waveprotocol/box/server/rpc/atmosphere/AtmosphereClientInterceptor.java <https://reviews.apache.org/r/19355/#comment69153> I don't understand why this class is needed, so maybe, a javadoc explanation of the class will be useful also for others. src/org/waveprotocol/box/webclient/client/atmosphere/AtmosphereConnectionImpl.java <https://reviews.apache.org/r/19355/#comment69152> We also take in to account ssl servers & websocket (and alternatives) connections. See WebClient.getWebSocketBaseUrl() - Vicente J. Ruiz Jurado On March 18, 2014, 3:34 p.m., Pablo Ojanguren wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19355/ > ----------------------------------------------------------- > > (Updated March 18, 2014, 3:34 p.m.) > > > Review request for wave, Ali Lown, Vicente J. Ruiz Jurado, and Yuri Zelikov. > > > Bugs: WAVE-405 > https://issues.apache.org/jira/browse/WAVE-405 > > > Repository: wave > > > Description > ------- > > This patch provides a full replacement of the Socket.IO as an alternative to > websockets when they are not available. Atmosphere framework is configured to > use long-polling protocol but additional are available. > > Server and GWT client has been affected. > > NOT all references to Socket.IO has been removed from the source code yet. > And .jar dependencies have been kept also. Is it safe to remove them? > > New dependencies has been included in build.xml, task get-third-party, so > none special process is needed to build this version. > The atmosphe.js client file is also handled as a third-party dep and it's > served from the class path > > > Diffs > ----- > > build.xml 0681b164cf580dd161d110dbf1032337243db79d > src/org/waveprotocol/box/server/rpc/ServerRpcProvider.java > 9b0f2a927bf75b92fb708c3abfdb4666d9cd6e63 > src/org/waveprotocol/box/server/rpc/atmosphere/AtmosphereChannel.java > PRE-CREATION > > src/org/waveprotocol/box/server/rpc/atmosphere/AtmosphereClientInterceptor.java > PRE-CREATION > src/org/waveprotocol/box/server/rpc/atmosphere/GuiceAtmosphereFactory.java > PRE-CREATION > src/org/waveprotocol/box/webclient/WebClient.gwt.xml > 387d0c78206bfca61412f31bdac26ec9a67224c9 > src/org/waveprotocol/box/webclient/client/WaveSocketFactory.java > 4a1788fc6f89fa07cbc41ab99335b25861388d8a > src/org/waveprotocol/box/webclient/client/WaveWebSocketClient.java > 65746d2348bf55a6c6f22b8b4404dfca6c8de302 > > src/org/waveprotocol/box/webclient/client/atmosphere/AtmosphereConnection.java > PRE-CREATION > > src/org/waveprotocol/box/webclient/client/atmosphere/AtmosphereConnectionImpl.java > PRE-CREATION > > src/org/waveprotocol/box/webclient/client/atmosphere/AtmosphereConnectionListener.java > PRE-CREATION > > src/org/waveprotocol/box/webclient/client/atmosphere/AtmosphereConnectionState.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/19355/diff/ > > > Testing > ------- > > Only basic test has been performed so far: wiab server with two users > connected, all in the same dev computer. Using Firefoz 23 with websockets > disabled. > > More test and feedback is needed covering different browsers, work load and > network environments. All configuration values of the atmosphere framework > are set to default ones (thread pool size, buffers size...) so I think they > will need fine tuning. > > > Thanks, > > Pablo Ojanguren > >
