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
-~----------~----~----~----~------~----~------~--~---

Reply via email to