[gwt-contrib] Issue 5700 (issue1388804)

[akito . nozaki]

Reviewers: ,

Description:
http://code.google.com/p/google-web-toolkit/issues/detail?id=5700
Please review this at http://gwt-code-reviews.appspot.com/1388804/

Affected files:
  user/src/com/google/gwt/event/shared/ResettableEventBus.java
  user/test/com/google/gwt/event/shared/ResettableEventBusTest.java


Index: user/test/com/google/gwt/event/shared/ResettableEventBusTest.java
===================================================================
--- user/test/com/google/gwt/event/shared/ResettableEventBusTest.java	 
(revision 9877)
+++ user/test/com/google/gwt/event/shared/ResettableEventBusTest.java	 
(working copy)
@@ -58,4 +58,48 @@
     });
     assertNotFired(mouse1, mouse2, mouse3);
   }
+
+  public void testManualRemove() {
+    SimpleEventBus eventBus = new SimpleEventBus();
+    ResettableEventBus subject = new ResettableEventBus(eventBus);
+
+    Type<MouseDownHandler> type = MouseDownEvent.getType();
+
+    HandlerRegistration registration1 = subject.addHandler(type, mouse1);
+    HandlerRegistration registration2 = subject.addHandler(type, mouse2);
+    HandlerRegistration registration3 = subject.addHandler(type, mouse3);
+
+    registration1.removeHandler();
+    // how do we check for memory leak? The  
ResettableEventBus.registration.size() should be 2
+
+    registration2.removeHandler();
+    registration3.removeHandler();
+
+    // The ResettableEventBus.registration.size() should be 0
+
+    subject.removeHandlers();
+  }
+
+  public void testNested() {
+    SimpleEventBus eventBus = new SimpleEventBus();
+    ResettableEventBus eventBus2 = new ResettableEventBus(eventBus);
+    ResettableEventBus subject = new ResettableEventBus(eventBus2);
+
+    Type<MouseDownHandler> type = MouseDownEvent.getType();
+    eventBus2.addHandler(type, mouse1);
+    subject.addHandler(type, mouse2);
+    subject.addHandler(type, mouse3);
+
+    subject.removeHandlers();
+    // eventBus2.registration.size() == 1 && subject.registration.size()  
== 0
+    eventBus2.removeHandlers();
+    // eventBus2.registration.size() == 0 && subject.registration.size()  
== 0
+
+    eventBus2.addHandler(type, mouse1);
+    subject.addHandler(type, mouse2);
+
+    eventBus2.removeHandlers();
+    // eventBus2.registration.size() == 0 && subject.registration.size()  
== 0
+    subject.removeHandlers();
+  }
 }
Index: user/src/com/google/gwt/event/shared/ResettableEventBus.java
===================================================================
--- user/src/com/google/gwt/event/shared/ResettableEventBus.java	(revision  
9877)
+++ user/src/com/google/gwt/event/shared/ResettableEventBus.java	(working  
copy)
@@ -18,6 +18,7 @@
 import com.google.gwt.event.shared.GwtEvent.Type;

 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.Set;

 /**
@@ -35,18 +36,30 @@
   @Override
   public <H extends EventHandler> HandlerRegistration addHandler(Type<H>  
type,
       H handler) {
-    HandlerRegistration rtn = wrapped.addHandler(type, handler);
+    final HandlerRegistration rtn = wrapped.addHandler(type, handler);
     registrations.add(rtn);
-    return rtn;
+    // Need to wrap this to cleanup registrations.
+    return new HandlerRegistration() {
+      @Override
+      public void removeHandler() {
+        doUnregisterHandler(rtn);
+      }
+    };
   }

   @Override
   public <H extends EventHandler> HandlerRegistration addHandlerToSource(
       GwtEvent.Type<H> type, Object source, H handler) {
-    HandlerRegistration rtn = wrapped.addHandlerToSource(type, source,
+    final HandlerRegistration rtn = wrapped.addHandlerToSource(type,  
source,
         handler);
     registrations.add(rtn);
-    return rtn;
+    // Need to wrap this to cleanup registrations.
+    return new HandlerRegistration() {
+      @Override
+      public void removeHandler() {
+        doUnregisterHandler(rtn);
+      }
+    };
   }

   @Override
@@ -63,9 +76,21 @@
    * Remove all handlers that have been added through this wrapper.
    */
   public void removeHandlers() {
-    for (HandlerRegistration r : registrations) {
-      r.removeHandler();
+    Iterator<HandlerRegistration> it = registrations.iterator();
+    while(it.hasNext()) {
+      HandlerRegistration r = it.next();
+
+      // must remove before we call removeHandler. Might came from nested  
ResettableEventBus
+      it.remove();
+
+      r.removeHandler();
     }
-    registrations.clear();
   }
+
+  private void doUnregisterHandler(HandlerRegistration registration) {
+    if(registrations.contains(registration)) {
+      registration.removeHandler();
+      registrations.remove(registration);
+    }
+  }
 }


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors