I committed the requested changes in abstractui at r6357, though some of them are rather different since the type-safe event changes.
I also realized we need to add an event type for the user closing an active module. There is more work to do to enable this (we were waiting on removing SWT), but the idea is if the server registers a callback for CloseModule then the UI should allow the user to close an active module while still leaving the UI running. http://gwt-code-reviews.appspot.com/77817/diff/1/35 File dev/core/src/com/google/gwt/dev/DevModeUI.java (right): http://gwt-code-reviews.appspot.com/77817/diff/1/35#newcode32 Line 32: * Callback interface for events from the UI. On 2009/10/13 17:06:12, rdayal wrote: > Nit: instead of "from" the UI, you could say "initiated by" the UI. Done. http://gwt-code-reviews.appspot.com/77817/diff/1/35#newcode81 Line 81: protected Type logLevel; On 2009/10/13 17:06:12, rdayal wrote: > Could make this private, and expose the value via a protected getter. Done. http://gwt-code-reviews.appspot.com/77817/diff/1/35#newcode146 Line 146: public void setCallback(String event, Callback callback) { On 2009/10/13 17:06:12, rdayal wrote: > Could this be final? Done. http://gwt-code-reviews.appspot.com/77817/diff/1/35#newcode164 Line 164: protected void callback(String event, Object callbackData) { On 2009/10/13 17:06:12, rdayal wrote: > Could this be final? Done. http://gwt-code-reviews.appspot.com/77817/diff/1/35#newcode174 Line 174: protected TreeLogger getConsoleLogger() { On 2009/10/13 17:06:12, rdayal wrote: > Could this be final? Done. http://gwt-code-reviews.appspot.com/77817/diff/1/35#newcode188 Line 188: protected boolean hasCallback(String event) { On 2009/10/13 17:06:12, rdayal wrote: > Could this be final? Done. http://gwt-code-reviews.appspot.com/77817/diff/1/30 File dev/core/src/com/google/gwt/dev/HostedModeBase.java (right): http://gwt-code-reviews.appspot.com/77817/diff/1/30#newcode69 Line 69: public class UiBrowserWidgetHostImpl extends BrowserWidgetHostImpl { On 2009/10/13 17:06:12, rdayal wrote: > Add some javadoc here. Done. http://gwt-code-reviews.appspot.com/77817/diff/1/36 File dev/core/src/com/google/gwt/dev/SwingUI.java (right): http://gwt-code-reviews.appspot.com/77817/diff/1/36#newcode107 Line 107: static ImageIcon loadImageIcon(String name, boolean prependPackage) { On 2009/10/13 17:06:12, rdayal wrote: > Document the return behavior. Done. http://gwt-code-reviews.appspot.com/77817/diff/1/36#newcode229 Line 229: tab.disconnect(); On 2009/10/13 17:06:12, rdayal wrote: > Is this thread-safe? There is nothing here that isn't -- tab.disconnect() might need to protect any data structures it uses. http://gwt-code-reviews.appspot.com/77817/diff/1/36#newcode247 Line 247: return ++sessionCounter; On 2009/10/13 17:06:12, rdayal wrote: > Is this thread-safe? I think it is not, but since this is existing code and the worst that could happen is a log file can be overwritten with another it should not be fixed here. I will add a TODO. http://gwt-code-reviews.appspot.com/77817/diff/1/19 File dev/oophm/src/com/google/gwt/dev/shell/BrowserChannel.java (right): http://gwt-code-reviews.appspot.com/77817/diff/1/19#newcode1332 Line 1332: stream.flush(); On 2009/10/13 17:06:12, rdayal wrote: > spurious indent? Done. http://gwt-code-reviews.appspot.com/77817/diff/1/4 File user/src/com/google/gwt/junit/BatchingStrategy.java (right): http://gwt-code-reviews.appspot.com/77817/diff/1/4#newcode51 Line 51: // TODO(jat): merge problem? On 2009/10/13 17:06:12, rdayal wrote: > Not sure what this refers to. Left-over - removed. http://gwt-code-reviews.appspot.com/77817/diff/1/6 File user/src/com/google/gwt/junit/JUnitShell.java (right): http://gwt-code-reviews.appspot.com/77817/diff/1/6#newcode2 Line 2: * Copyright 2008 Google Inc. On 2009/10/13 17:06:12, rdayal wrote: > Might be good to have someone else review this file; I'm not very familiar with > this part of the code. Ok. http://gwt-code-reviews.appspot.com/77817 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
