Fixed a bug in HandlerManager where removing the last handler of a given EventType twice would result in an NPE because we assume that the list of event handlers is not null.  Now, this results in an AssertionError.

Patch by: jlabanca
Review by: rjrjr



git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@5266 8db76d5a-ed1c-0410-87a9-c151d255dfc7

user/src/com/google/gwt/event/shared/HandlerManager.java

diff --git a/user/src/com/google/gwt/event/shared/HandlerManager.java b/user/src/com/google/gwt/event/shared/HandlerManager.java
index db8617e..b4269ea 100644
--- a/user/src/com/google/gwt/event/shared/HandlerManager.java
+++ b/user/src/com/google/gwt/event/shared/HandlerManager.java
@@ -32,7 +32,7 @@
    * Interface for queued add/remove operations.
    */
   private interface AddOrRemoveCommand {
-    public void execute();
+    void execute();
   }
 
   /**
@@ -90,8 +90,8 @@
 
     private <H> void removeHandler(GwtEvent.Type<H> eventKey, H handler) {
       ArrayList<H> l = get(eventKey);
-      boolean result = l.remove(handler);
-      if (l.size() == 0) {
+      boolean result = (l == null) ? false : l.remove(handler);
+      if (result && l.size() == 0) {
         map.remove(eventKey);
       }
       assert result : "Tried to remove unknown handler: " + handler + " from "

user/test/com/google/gwt/event/shared/HandlerManagerTest.java

diff --git a/user/test/com/google/gwt/event/shared/HandlerManagerTest.java b/user/test/com/google/gwt/event/shared/HandlerManagerTest.java
index 48bc156..9d9dbaf 100644
--- a/user/test/com/google/gwt/event/shared/HandlerManagerTest.java
+++ b/user/test/com/google/gwt/event/shared/HandlerManagerTest.java
@@ -260,6 +260,25 @@
     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 */

+      }

+    } else {

+      reg.removeHandler();

+      /* pass, we didn't hit an NPE */

+    }

+  }

+

   public void testReverseOrder() {

     // Add some handlers to a manager

     final HandlerManager manager = new HandlerManager("source1", true);