In a few words: everything's in c.g.g.dom, bringing a dependency from
c.g.g.dom to c.g.g.event and breaking the assumption that an
EventHandler is coupled with a GwtEvent (rather than a JSO-derivative),
and it starts to refactor event handling at the low-level, while Joel is
working on it (see
https://wave.google.com/wave/waveref/googlewave.com/w+ux7zL81XA ).

I'd rather:
 - introduce a c.g.g.websocket module
 - depend on what's being discussed in the "Refactoring event handling"
Wave (cast to EventTarget and use EventTarget.addEventListener), or at
least make sure it could be moved to that API/model later on (I haven't
looked closely but it might already be the case in this proposal, just
that I think EventListener and the like should probably be a distinct
review); WebSocket support shouldn't have any impact in itself on
c.g.g.dom
 - use GwtEvent/EventHandler pairs (if performance is an issue –it has
been benchmarked as not being one for other very-frequent events such as
mousemove– then use callbacks instead)

My 2c.


http://gwt-code-reviews.appspot.com/646803/diff/1/3
File user/src/com/google/gwt/core/client/impl/RawWebSocketImpl.java
(right):

http://gwt-code-reviews.appspot.com/646803/diff/1/3#newcode46
user/src/com/google/gwt/core/client/impl/RawWebSocketImpl.java:46: var
callback = function(e) {
Shouldn't there be an $entry() here?

http://gwt-code-reviews.appspot.com/646803/diff/1/15
File user/src/com/google/gwt/dom/client/WebSocket.java (right):

http://gwt-code-reviews.appspot.com/646803/diff/1/15#newcode16
user/src/com/google/gwt/dom/client/WebSocket.java:16: package
com.google.gwt.dom.client;
Why isn't all of this on some new module? e.g. com.google.gwt.websocket?

http://gwt-code-reviews.appspot.com/646803/diff/1/16
File user/src/com/google/gwt/dom/client/WebSocketCloseHandler.java
(right):

http://gwt-code-reviews.appspot.com/646803/diff/1/16#newcode18
user/src/com/google/gwt/dom/client/WebSocketCloseHandler.java:18: import
com.google.gwt.event.shared.EventHandler;
This introduces a dependency from dom to event :-(

http://gwt-code-reviews.appspot.com/646803/diff/1/16#newcode22
user/src/com/google/gwt/dom/client/WebSocketCloseHandler.java:22: void
onClose(CloseEvent event);
CloseEvent (and related) should really be a kind of GwtEvent IMO.

http://gwt-code-reviews.appspot.com/646803/show

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to