http://gwt-code-reviews.appspot.com/841804/diff/11002/17020 File user/src/com/google/gwt/event/shared/EventBus.java (right):
http://gwt-code-reviews.appspot.com/841804/diff/11002/17020#newcode34 user/src/com/google/gwt/event/shared/EventBus.java:34: public <H extends EventHandler> void removeHandler(GwtEvent.Type<H> type, I vaguely remember the conversation, but I really don't like having a HandlerRegistration and a remove method. What about giving EventBusa remove method that returns void, and no concept of HandlerRegistration. HandlerManager#addHandler can return a HandlerRegistration that calls EventBus.removeHandler(). http://gwt-code-reviews.appspot.com/841804/diff/11002/17020#newcode79 user/src/com/google/gwt/event/shared/EventBus.java:79: GwtEvent.Type<H> type, Object source, H handler); Its okay to be null from an implementation standpoint, but it probably indicates an error on the part of the user. Otherwise, they would have used addHandler() without the source arg. I also don't see how it increases complexity. addHandler(type, handler) { addHandlerImpl(type, null, handler); } addHandler(type, source handler) { assert source != null : "Doh"; addHandlerImpl(type, source, handler); } addHandlerImpl(type, source, handler) { // your existing code. } http://gwt-code-reviews.appspot.com/841804/diff/11002/17023 File user/src/com/google/gwt/event/shared/SimpleEventBus.java (right): http://gwt-code-reviews.appspot.com/841804/diff/11002/17023#newcode167 user/src/com/google/gwt/event/shared/SimpleEventBus.java:167: prune(type, source); SGTM http://gwt-code-reviews.appspot.com/841804/diff/11002/17024 File user/src/com/google/gwt/event/shared/testing/CountingEventBus.java (right): http://gwt-code-reviews.appspot.com/841804/diff/11002/17024#newcode52 user/src/com/google/gwt/event/shared/testing/CountingEventBus.java:52: counts.put(type, count - 1); Why would we want getCount() to return a negative value? If you remove a handler that doesn't exist, you end up with 0 handlers, not -1. The information is still useful in a test because users know how many handlers they expect to have. Perhaps EventBus.removeHandler() should return a boolean, or the handler that was removed. http://gwt-code-reviews.appspot.com/841804/diff/31001/3025 File user/src/com/google/gwt/event/shared/SimpleEventBus.java (right): http://gwt-code-reviews.appspot.com/841804/diff/31001/3025#newcode132 user/src/com/google/gwt/event/shared/SimpleEventBus.java:132: public <H extends EventHandler> void removeHandlerFromSource( Public method after a private method. http://gwt-code-reviews.appspot.com/841804/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
