Adds an option to HandlerManager to fire handlers in reverse order, and converts NativePreviewEvent to use the HandlerManager instead of an ArrayList. The HandlerManager automatically handles concurrent adds and removes, which caused a problem in MenuBar.
Patch by: jlabanca
Review by: ecc (desk)
Issue: 3289
git-svn-id: https://google-web-toolkit.googlecode.com/svn/releases/1.6@4474 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/event/logical/shared/BeforeSelectionEvent.java b/user/src/com/google/gwt/event/logical/shared/BeforeSelectionEvent.java
index 7ff45c6..62066a2 100644
--- a/user/src/com/google/gwt/event/logical/shared/BeforeSelectionEvent.java
+++ b/user/src/com/google/gwt/event/logical/shared/BeforeSelectionEvent.java
@@ -91,7 +91,7 @@
// field itself does not, so we have to do an unsafe cast here.
@SuppressWarnings("unchecked")
@Override
- public Type<BeforeSelectionHandler<I>> getAssociatedType() {
+ public final Type<BeforeSelectionHandler<I>> getAssociatedType() {
return (Type) TYPE;
}
diff --git a/user/src/com/google/gwt/event/logical/shared/ResizeEvent.java b/user/src/com/google/gwt/event/logical/shared/ResizeEvent.java
index f3b1393..a931456 100644
--- a/user/src/com/google/gwt/event/logical/shared/ResizeEvent.java
+++ b/user/src/com/google/gwt/event/logical/shared/ResizeEvent.java
@@ -76,7 +76,7 @@
}
@Override
- public Type<ResizeHandler> getAssociatedType() {
+ public final Type<ResizeHandler> getAssociatedType() {
return TYPE;
}
diff --git a/user/src/com/google/gwt/event/logical/shared/SelectionEvent.java b/user/src/com/google/gwt/event/logical/shared/SelectionEvent.java
index 342b179..967b557 100644
--- a/user/src/com/google/gwt/event/logical/shared/SelectionEvent.java
+++ b/user/src/com/google/gwt/event/logical/shared/SelectionEvent.java
@@ -76,7 +76,7 @@
// field itself does not, so we have to do an unsafe cast here.
@SuppressWarnings("unchecked")
@Override
- public Type<SelectionHandler<I>> getAssociatedType() {
+ public final Type<SelectionHandler<I>> getAssociatedType() {
return (Type) TYPE;
}
diff --git a/user/src/com/google/gwt/event/logical/shared/ValueChangeEvent.java b/user/src/com/google/gwt/event/logical/shared/ValueChangeEvent.java
index 5dfe97f..ab32e66 100644
--- a/user/src/com/google/gwt/event/logical/shared/ValueChangeEvent.java
+++ b/user/src/com/google/gwt/event/logical/shared/ValueChangeEvent.java
@@ -54,7 +54,6 @@
* safe handling of null.
*
* @param <I> the old value type
- * @param <S> The event source
* @param source the source of the handlers
* @param oldValue the oldValue, may be null
* @param newValue the newValue, may be null
@@ -110,7 +109,7 @@
// field itself does not, so we have to do an unsafe cast here.
@SuppressWarnings("unchecked")
@Override
- public Type<ValueChangeHandler<I>> getAssociatedType() {
+ public final Type<ValueChangeHandler<I>> getAssociatedType() {
return (Type) TYPE;
}
diff --git a/user/src/com/google/gwt/event/shared/HandlerManager.java b/user/src/com/google/gwt/event/shared/HandlerManager.java
index 44fe043..fc1a0a3 100644
--- a/user/src/com/google/gwt/event/shared/HandlerManager.java
+++ b/user/src/com/google/gwt/event/shared/HandlerManager.java
@@ -44,12 +44,20 @@
l.add(handler);
}
- private <H extends EventHandler> void fireEvent(GwtEvent<H> event) {
+ private <H extends EventHandler> void fireEvent(GwtEvent<H> event,
+ boolean isReverseOrder) {
Type<H> type = event.getAssociatedType();
int count = getHandlerCount(type);
- for (int i = 0; i < count; i++) {
- H handler = this.<H> getHandler(type, i);
- event.dispatch(handler);
+ if (isReverseOrder) {
+ for (int i = count - 1; i >= 0; i--) {
+ H handler = this.<H> getHandler(type, i);
+ event.dispatch(handler);
+ }
+ } else {
+ for (int i = 0; i < count; i++) {
+ H handler = this.<H> getHandler(type, i);
+ event.dispatch(handler);
+ }
}
}
@@ -79,6 +87,7 @@
}
private int firingDepth = 0;
+ private boolean isReverseOrder;
// map storing the actual handlers
private HandlerRegistry registry;
@@ -90,13 +99,26 @@
private List<Command> deferredDeltas;
/**
- * Creates a handler manager with the given source.
+ * Creates a handler manager with the given source. Handlers will be fired in
+ * the order that they are added.
*
* @param source the event source
*/
public HandlerManager(Object source) {
+ this(source, false);
+ }
+
+ /**
+ * Creates a handler manager with the given source, specifying the order in
+ * which handlers are fired.
+ *
+ * @param source the event source
+ * @param fireInReverseOrder true to fire handlers in reverse order
+ */
+ public HandlerManager(Object source, boolean fireInReverseOrder) {
registry = new HandlerRegistry();
this.source = source;
+ this.isReverseOrder = fireInReverseOrder;
}
/**
@@ -140,7 +162,7 @@
try {
firingDepth++;
- registry.fireEvent(event);
+ registry.fireEvent(event, isReverseOrder);
} finally {
firingDepth--;
diff --git a/user/src/com/google/gwt/user/client/Event.java b/user/src/com/google/gwt/user/client/Event.java
index 4ef3269..2a72281 100644
--- a/user/src/com/google/gwt/user/client/Event.java
+++ b/user/src/com/google/gwt/user/client/Event.java
@@ -20,11 +20,9 @@
import com.google.gwt.event.dom.client.HasNativeEvent;
import com.google.gwt.event.shared.EventHandler;
import com.google.gwt.event.shared.GwtEvent;
+import com.google.gwt.event.shared.HandlerManager;
import com.google.gwt.event.shared.HandlerRegistration;
-import java.util.ArrayList;
-import java.util.List;
-
/**
* <p>
* An opaque handle to a native DOM Event. An <code>Event</code> cannot be
@@ -67,29 +65,18 @@
/**
* Fire a {@link NativePreviewEvent} for the native event.
*
- * @param handlers the list of {@link NativePreviewHandler}
+ * @param handlers the {@link HandlerManager}
* @param nativeEvent the native event
* @return true to fire the event normally, false to cancel the event
*/
- static boolean fire(List<NativePreviewHandler> handlers, Event nativeEvent) {
+ private static boolean fire(HandlerManager handlers, Event nativeEvent) {
if (TYPE != null && handlers != null) {
// Revive the event
- if (singleton == null) {
- singleton = new NativePreviewEvent();
- } else {
- singleton.revive();
- }
+ singleton.revive();
singleton.setNativeEvent(nativeEvent);
- // Fire the event to all handlers
- int numHandlers = handlers.size();
- for (int i = numHandlers - 1; i >= 0; i--) {
- handlers.get(i).onPreviewNativeEvent(singleton);
- singleton.isFirstHandler = false;
- }
-
- // Kill the event and return
- singleton.kill();
+ // Fire the event
+ handlers.fireEvent(singleton);
return !(singleton.isCanceled() && !singleton.isConsumed());
}
return true;
@@ -139,7 +126,7 @@
}
@Override
- public Type<NativePreviewHandler> getAssociatedType() {
+ public final Type<NativePreviewHandler> getAssociatedType() {
return TYPE;
}
@@ -181,6 +168,7 @@
@Override
protected void dispatch(NativePreviewHandler handler) {
handler.onPreviewNativeEvent(this);
+ singleton.isFirstHandler = false;
}
@Override
@@ -360,7 +348,7 @@
* handler manager for efficiency and because we want to fire the handlers in
* reverse order. When the last handler is removed, handlers is reset to null.
*/
- static ArrayList<NativePreviewHandler> handlers;
+ static HandlerManager handlers;
/**
* Adds an event preview to the preview stack. As long as this preview remains
@@ -403,20 +391,10 @@
// Initialize the type
NativePreviewEvent.getType();
if (handlers == null) {
- handlers = new ArrayList<NativePreviewHandler>();
+ handlers = new HandlerManager(null, true);
+ NativePreviewEvent.singleton = new NativePreviewEvent();
}
- handlers.add(handler);
- return new HandlerRegistration() {
- public void removeHandler() {
- if (handlers != null) {
- handlers.remove(handler);
- if (handlers.size() == 0) {
- // Set handlers to null so we can optimize fireNativePreviewEvent
- handlers = null;
- }
- }
- }
- };
+ return handlers.addHandler(NativePreviewEvent.TYPE, handler);
}
/**
diff --git a/user/src/com/google/gwt/user/client/ListenerWrapper.java b/user/src/com/google/gwt/user/client/ListenerWrapper.java
index 6a80c85..77f51ad 100644
--- a/user/src/com/google/gwt/user/client/ListenerWrapper.java
+++ b/user/src/com/google/gwt/user/client/ListenerWrapper.java
@@ -27,8 +27,6 @@
import com.google.gwt.event.shared.GwtEvent.Type;
import com.google.gwt.user.client.Event.NativePreviewEvent;
-import java.util.EventListener;
-
/**
* Legacy listener support for <code>com.google.gwt.user.client</code>. Gathers
* the bulk of the legacy glue code in one place, for easy deletion when
@@ -66,19 +64,7 @@
}
public static void remove(EventPreview listener) {
- if (Event.handlers == null) {
- return;
- }
- int handlerCount = Event.handlers.size();
- // We only want to remove the first instance, as the legacy listener does
- for (int i = 0; i < handlerCount; i++) {
- Event.NativePreviewHandler handler = Event.handlers.get(i);
- if (handler instanceof NativePreview
- && ((NativePreview) handler).listener.equals(listener)) {
- Event.handlers.remove(handler);
- return;
- }
- }
+ baseRemove(Event.handlers, listener, NativePreviewEvent.getType());
}
private NativePreview(EventPreview listener) {
@@ -175,7 +161,7 @@
// This is a direct copy of the baseRemove from
// com.google.gwt.user.client.ui.ListenerWrapper. Change in parallel.
static <H extends EventHandler> void baseRemove(HandlerManager manager,
- EventListener listener, Type... keys) {
+ Object listener, Type... keys) {
if (manager != null) {
for (Type<H> key : keys) {
int handlerCount = manager.getHandlerCount(key);
diff --git a/user/src/com/google/gwt/user/client/Window.java b/user/src/com/google/gwt/user/client/Window.java
index 58ab514..8f966f5 100644
--- a/user/src/com/google/gwt/user/client/Window.java
+++ b/user/src/com/google/gwt/user/client/Window.java
@@ -64,7 +64,7 @@
private String message = null;
@Override
- public Type<ClosingHandler> getAssociatedType() {
+ public final Type<ClosingHandler> getAssociatedType() {
return TYPE;
}
@@ -332,7 +332,7 @@
}
@Override
- public Type<ScrollHandler> getAssociatedType() {
+ public final Type<ScrollHandler> getAssociatedType() {
return TYPE;
}
diff --git a/user/src/com/google/gwt/user/client/ui/FormPanel.java b/user/src/com/google/gwt/user/client/ui/FormPanel.java
index 4aa7b90..d810ef8 100644
--- a/user/src/com/google/gwt/user/client/ui/FormPanel.java
+++ b/user/src/com/google/gwt/user/client/ui/FormPanel.java
@@ -87,7 +87,7 @@
private String resultHtml;
/**
- * Create a submit complete event
+ * Create a submit complete event.
*
* @param resultsHtml the results from submitting the form
*/
@@ -96,7 +96,7 @@
}
@Override
- public Type<SubmitCompleteHandler> getAssociatedType() {
+ public final Type<SubmitCompleteHandler> getAssociatedType() {
return TYPE;
}
@@ -162,7 +162,7 @@
}
@Override
- public Type<FormPanel.SubmitHandler> getAssociatedType() {
+ public final Type<FormPanel.SubmitHandler> getAssociatedType() {
return TYPE;
}
diff --git a/user/test/com/google/gwt/event/shared/HandlerManagerTest.java b/user/test/com/google/gwt/event/shared/HandlerManagerTest.java
index ec450f2..480db2f 100644
--- a/user/test/com/google/gwt/event/shared/HandlerManagerTest.java
+++ b/user/test/com/google/gwt/event/shared/HandlerManagerTest.java
@@ -223,7 +223,6 @@
assertFired(one, mouse1);
}
- @SuppressWarnings("deprecation")
public void testMultiFiring() {
HandlerManager manager = new HandlerManager("source1");
@@ -261,4 +260,38 @@
});
assertFired(mouse1, adaptor1, mouse3);
}
+
+ public void testReverseOrder() {
+ // Add some handlers to a manager
+ final HandlerManager manager = new HandlerManager("source1", true);
+ final MouseDownHandler handler0 = new MouseDownHandler() {
+ public void onMouseDown(MouseDownEvent event) {
+ add(this);
+ }
+ };
+ final MouseDownHandler handler1 = new MouseDownHandler() {
+ public void onMouseDown(MouseDownEvent event) {
+ assertNotFired(handler0);
+ add(this);
+ }
+ };
+ final MouseDownHandler handler2 = new MouseDownHandler() {
+ public void onMouseDown(MouseDownEvent event) {
+ assertNotFired(handler0, handler1);
+ add(this);
+ }
+ };
+ HandlerRegistration reg0 = manager.addHandler(MouseDownEvent.getType(),
+ handler0);
+ HandlerRegistration reg1 = manager.addHandler(MouseDownEvent.getType(),
+ handler1);
+ HandlerRegistration reg2 = manager.addHandler(MouseDownEvent.getType(),
+ handler2);
+
+ // Fire the event
+ reset();
+ manager.fireEvent(new MouseDownEvent() {
+ });
+ assertFired(handler0, handler1, handler2);
+ }
}
diff --git a/user/test/com/google/gwt/user/client/EventTest.java b/user/test/com/google/gwt/user/client/EventTest.java
index 42c7516..1ed7e7b 100644
--- a/user/test/com/google/gwt/user/client/EventTest.java
+++ b/user/test/com/google/gwt/user/client/EventTest.java
@@ -94,6 +94,36 @@
}
/**
+ * Test that concurrent removal of a {@link NativePreviewHandler} does not
+ * trigger an exception. The handler should not actually be removed until the
+ * end of the event loop.
+ */
+ public void testConcurrentRemovalOfNativePreviewHandler() {
+ // Add handler0
+ final TestNativePreviewHandler handler0 = new TestNativePreviewHandler(
+ false, false);
+ final HandlerRegistration reg0 = Event.addNativePreviewHandler(handler0);
+
+ // Add handler 1
+ final TestNativePreviewHandler handler1 = new TestNativePreviewHandler(
+ false, false) {
+ @Override
+ public void onPreviewNativeEvent(NativePreviewEvent event) {
+ super.onPreviewNativeEvent(event);
+ handler0.assertIsFired(false);
+ reg0.removeHandler();
+ }
+ };
+ HandlerRegistration reg1 = Event.addNativePreviewHandler(handler1);
+ assertTrue(Event.fireNativePreviewEvent(null));
+
+ // Verify both handlers fired even though one was removed
+ handler0.assertIsFired(true);
+ handler1.assertIsFired(true);
+ reg1.removeHandler();
+ }
+
+ /**
* Test that {@link Event#fireNativePreviewEvent(Event)} returns the correct
* value if the native event is canceled.
*/
@@ -146,6 +176,7 @@
super.onPreviewNativeEvent(event);
preview0.assertIsFired(false);
preview1.assertIsFired(true);
+ assertFalse(event.isFirstHandler());
}
};
final TestNativePreviewHandler handler1 = new TestNativePreviewHandler(
@@ -156,6 +187,7 @@
handler0.assertIsFired(false);
preview0.assertIsFired(false);
preview1.assertIsFired(true);
+ assertFalse(event.isFirstHandler());
}
};
final TestNativePreviewHandler handler2 = new TestNativePreviewHandler(
@@ -167,6 +199,7 @@
handler1.assertIsFired(false);
preview0.assertIsFired(false);
preview1.assertIsFired(true);
+ assertFalse(event.isFirstHandler());
}
};
final TestNativePreviewHandler handler3 = new TestNativePreviewHandler(
@@ -179,6 +212,7 @@
handler2.assertIsFired(false);
preview0.assertIsFired(false);
preview1.assertIsFired(true);
+ assertFalse(event.isFirstHandler());
}
};
DOM.addEventPreview(preview0);
@@ -408,6 +442,7 @@
public void onPreviewNativeEvent(NativePreviewEvent event) {
assertFalse(event.isCanceled());
assertFalse(event.isConsumed());
+ assertTrue(event.isFirstHandler());
super.onPreviewNativeEvent(event);
}
};
