Thanks for the reviews, committed to abstractui and merged down to farewellSwt in r6360.
http://gwt-code-reviews.appspot.com/77818/diff/1/24 File dev/core/src/com/google/gwt/dev/HostedModeBase.java (right): http://gwt-code-reviews.appspot.com/77818/diff/1/24#newcode630 Line 630: * Callback for the UI to indicate it is down. On 2009/10/13 19:16:52, rdayal wrote: > down --> done, or exited. Done. http://gwt-code-reviews.appspot.com/77818/diff/1/37 File dev/core/src/com/google/gwt/dev/ui/CloseModuleCallback.java (right): http://gwt-code-reviews.appspot.com/77818/diff/1/37#newcode26 Line 26: * The user has requested the a module should be closed. In the event the On 2009/10/13 19:16:52, rdayal wrote: > The user has requested the-->that a module should be closed. In the event <that> > the... Done. http://gwt-code-reviews.appspot.com/77818/diff/1/33 File dev/core/src/com/google/gwt/dev/ui/DevModeUI.java (right): http://gwt-code-reviews.appspot.com/77818/diff/1/33#newcode47 Line 47: private final Map<UiEvent.Type<?>, UiCallback> callbacks = new HashMap< On 2009/10/13 19:16:52, rdayal wrote: > You can use ? instead of UICallback, can't you? ? extends UiCallback, done http://gwt-code-reviews.appspot.com/77818/diff/1/33#newcode149 Line 149: UiEvent.Type<?> eventType) { On 2009/10/13 19:16:52, rdayal wrote: > Can't you use UIcallback instead of ? here? ? extends UiCallback, done http://gwt-code-reviews.appspot.com/77818/diff/1/33#newcode177 Line 177: protected final boolean hasCallback(UiEvent.Type<?> eventType) { On 2009/10/13 19:16:52, rdayal wrote: > Can't you use UICallback instead of ? here? ? extends UiCallback, done http://gwt-code-reviews.appspot.com/77818/diff/1/32 File dev/core/src/com/google/gwt/dev/ui/UiEvent.java (right): http://gwt-code-reviews.appspot.com/77818/diff/1/32#newcode35 Line 35: private static int nextId = 0; On 2009/10/13 19:16:52, rdayal wrote: > Is this completely thread-safe? That is, are all instances of the Type object > created by the same thread? That could be an issue, and on further reflection we don't need the optimization that GwtEvent does, which is where this was copied from. Removing the id entirely and letting it fall back to the system identity hash code. http://gwt-code-reviews.appspot.com/77818/diff/1/32#newcode47 Line 47: // Since we require only one instance of each type object to be created, On 2009/10/13 19:16:52, rdayal wrote: > I agree with your invariant, but I could see someone making a mistake. Why not > create a static map with names and ids and use that to assign the same id for > those objects with the same names at construction time? New UI events will not be created by users, but only by GWT developers as part of the GWT code base. I don't think we want to protect against this sort of error. Removing a custom hash code also means we don't need the equals here. http://gwt-code-reviews.appspot.com/77818/diff/1/43 File user/src/com/google/gwt/junit/JUnitShell.java (left): http://gwt-code-reviews.appspot.com/77818/diff/1/43#oldcode557 Line 557: if (unitTestShell.thread != Thread.currentThread()) { On 2009/10/13 18:59:27, jlabanca wrote: > This was added intentionally to ensure we don't run a test outside of the main > thread that created JUnitShell. It may not matter anymore because it was > handling an exception in SWT Display, which is now gone. This is relative to the farewellSwt branch, where SWT has been removed, so this is no longer needed. http://gwt-code-reviews.appspot.com/77818/diff/1/42 File user/src/com/google/gwt/junit/RunStyleHtmlUnit.java (right): http://gwt-code-reviews.appspot.com/77818/diff/1/42#newcode198 Line 198: getLogger().log(TreeLogger.WARN, "RunStyleHtmlUnit: Ignoring unknown " On 2009/10/13 18:59:27, jlabanca wrote: > This should throw an ERROR. The WARN can be lost in the output, and a user may > not even realize that they accidently typed the wrong browser name. Done. http://gwt-code-reviews.appspot.com/77818 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---