I think DevModeLogManager still needs some work, the rest is just style stuff.
http://gwt-code-reviews.appspot.com/437801/diff/1/3 File /dev/core/src/com/google/gwt/dev/shell/BrowserChannelServer.java (right): http://gwt-code-reviews.appspot.com/437801/diff/1/3#newcode43 /dev/core/src/com/google/gwt/dev/shell/BrowserChannelServer.java:43: * should be instantiating this class. Instead of this last sentence, just make the constructor private or default access. http://gwt-code-reviews.appspot.com/437801/diff/1/4 File /dev/core/src/com/google/gwt/dev/shell/DevModeLogManager.java (right): http://gwt-code-reviews.appspot.com/437801/diff/1/4#newcode16 /dev/core/src/com/google/gwt/dev/shell/DevModeLogManager.java:16: No blank line here. http://gwt-code-reviews.appspot.com/437801/diff/1/4#newcode27 /dev/core/src/com/google/gwt/dev/shell/DevModeLogManager.java:27: public class DevModeLogManager extends LogManager { My main comment on this class is... a user can have multiple hosted mode sessions open concurrently, and each is supposed to live in its own isolated world. But this implementation appears to lump all of the individual clients into the same bucket. I think you need a per-client LogManager. A pretty easy way to do this is with a ThreadLocal<LogManager> for clients. If you go this route, individual delegation methods could look like this: public boolean addLogger(Logger logger) { return getDelegate().addLogger(logger); } where getDelegate() returns either the serverLogManager, or a client-specific one. http://gwt-code-reviews.appspot.com/437801/diff/1/4#newcode34 /dev/core/src/com/google/gwt/dev/shell/DevModeLogManager.java:34: protected static LogManager delegate; Suggest naming this "serverLogManager" or something like that. http://gwt-code-reviews.appspot.com/437801/diff/1/4#newcode39 /dev/core/src/com/google/gwt/dev/shell/DevModeLogManager.java:39: // delegate calls to there. The code you want will probably look something like this: Class<? extends LogManager> clazz = Class.forName(logManagerClass).asSubclass(LogManager.class); Constructor<? extends LogManager> ctor = clazz.getDeclaredConstructor(); ctor.setAccessible(true); delegate = ctor.newInstance(); (mostly copied from ModuleSpace.rebindAndCreate()) The nice thing about implementing this is, you won't need LogManagerWithExposedConstructor anymore, since you can instantiate it via reflection. http://gwt-code-reviews.appspot.com/437801/diff/1/4#newcode63 /dev/core/src/com/google/gwt/dev/shell/DevModeLogManager.java:63: It seems like you'd want to forward all of the public methods, right? Otherwise you could break e.g. servlet code that uses loggging. http://gwt-code-reviews.appspot.com/437801/diff/1/5 File /dev/core/test/com/google/gwt/dev/DevModeBaseTest.java (right): http://gwt-code-reviews.appspot.com/437801/diff/1/5#newcode24 /dev/core/test/com/google/gwt/dev/DevModeBaseTest.java:24: String kManagerProperty = "java.util.logging.manager"; We don't use this style in GWT. If you want to make it a constant, make it a static final field in the class and use THIS_FORMAT for the name. http://gwt-code-reviews.appspot.com/437801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
