Reviewers: rjrjr,
Description:
HandlerRegistration#removeHandler() on an event type that is not handled
results in an NPE because we assume that the ArrayList of handlers
exists. This can also be triggered by calling
HandlerRegistration#removeHandler() twice on the last handler of a type.
The problem is, we remove the ArrayList of handlers for the type when
the last one is removed, but we never check that its not null. This
patch checks for null and throws the same assertion error that we throw
when the handler is not part of the HandlerManager, which is better than
an NPE.
Please review this at http://gwt-code-reviews.appspot.com/25801
Affected files:
user/src/com/google/gwt/event/shared/HandlerManager.java
user/test/com/google/gwt/event/shared/HandlerManagerTest.java
Index: user/test/com/google/gwt/event/shared/HandlerManagerTest.java
===
--- user/test/com/google/gwt/event/shared/HandlerManagerTest.java
(revision
5265)
+++ user/test/com/google/gwt/event/shared/HandlerManagerTest.java
(working
copy)
@@ -260,6 +260,22 @@
assertFired(mouse1, adaptor1, mouse3);
}
+ public void testRemoveUnhandledType() {
+final HandlerManager manager = new HandlerManager("bogus source");
+HandlerRegistration reg = manager.addHandler(MouseDownEvent.getType(),
+mouse1);
+reg.removeHandler();
+
+if (!GWT.isScript()) {
+ try {
+reg.removeHandler();
+fail("Should have thrown assertion error");
+ } catch (AssertionError e) {
+/* pass */
+ }
+}
+ }
+
public void testReverseOrder() {
// Add some handlers to a manager
final HandlerManager manager = new HandlerManager("source1", true);
Index: user/src/com/google/gwt/event/shared/HandlerManager.java
===
--- user/src/com/google/gwt/event/shared/HandlerManager.java(revision 5265)
+++ user/src/com/google/gwt/event/shared/HandlerManager.java(working copy)
@@ -32,7 +32,7 @@
* Interface for queued add/remove operations.
*/
private interface AddOrRemoveCommand {
-public void execute();
+void execute();
}
/**
@@ -90,9 +90,14 @@
private void removeHandler(GwtEvent.Type eventKey, H handler) {
ArrayList l = get(eventKey);
- boolean result = l.remove(handler);
- if (l.size() == 0) {
-map.remove(eventKey);
+ boolean result;
+ if (l == null) {
+result = false;
+ } else {
+result = l.remove(handler);
+if (l.size() == 0) {
+ map.remove(eventKey);
+}
}
assert result : "Tried to remove unknown handler: " + handler + "
from "
+ eventKey;
--~--~-~--~~~---~--~~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~--~~~~--~~--~--~---
