Fix for listener wrapper window listener removal, bug listed at: http://gwt-code-reviews.appspot.com/603/show Review by:jlabanca git-svn-id: https://google-web-toolkit.googlecode.com/svn/releases/1.6@4298 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/ListenerWrapper.java b/user/src/com/google/gwt/user/client/ListenerWrapper.java index 45bf6ac..bcd0115 100644 --- a/user/src/com/google/gwt/user/client/ListenerWrapper.java +++ b/user/src/com/google/gwt/user/client/ListenerWrapper.java
@@ -131,24 +131,29 @@ } } + // This is an internal helper method with the current formulation, we have // lost the info needed to make it safe by this point. @SuppressWarnings("unchecked") - protected static void baseRemove(HandlerManager manager, + // 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) { if (manager != null) { - for (Type key : keys) { + for (Type<H> key : keys) { int handlerCount = manager.getHandlerCount(key); - for (int i = 0; i < handlerCount; i++) { - EventHandler handler = manager.getHandler(key, i); - if (handler instanceof ListenerWrapper && ((ListenerWrapper) handler).listener.equals(listener)) { + // We are removing things as we traverse, have to go backward + for (int i = handlerCount - 1; i >= 0; i--) { + H handler = manager.getHandler(key, i); + if (handler instanceof ListenerWrapper + && ((ListenerWrapper) handler).listener.equals(listener)) { manager.removeHandler(key, handler); } } } } } - + /** * Listener being wrapped. */
diff --git a/user/src/com/google/gwt/user/client/Window.java b/user/src/com/google/gwt/user/client/Window.java index 50b2fc4..dae2416 100644 --- a/user/src/com/google/gwt/user/client/Window.java +++ b/user/src/com/google/gwt/user/client/Window.java
@@ -392,12 +392,13 @@ } } + // Package protected for testing. + static WindowHandlers handlers; private static boolean closeHandlersInitialized; private static boolean scrollHandlersInitialized; private static boolean resizeHandlersInitialized; private static final WindowImpl impl = GWT.create(WindowImpl.class); - private static WindowHandlers handlers; /** * Adds a {@link CloseEvent} handler.
diff --git a/user/test/com/google/gwt/user/client/WindowTest.java b/user/test/com/google/gwt/user/client/WindowTest.java index 04e2302..b0e0ea6 100644 --- a/user/test/com/google/gwt/user/client/WindowTest.java +++ b/user/test/com/google/gwt/user/client/WindowTest.java
@@ -15,6 +15,7 @@ */ package com.google.gwt.user.client; +import com.google.gwt.event.logical.shared.ResizeEvent; import com.google.gwt.junit.client.GWTTestCase; import com.google.gwt.user.client.ui.Label; import com.google.gwt.user.client.ui.RootPanel; @@ -176,4 +177,40 @@ // Cleanup the window RootPanel.get().remove(largeDOM); } + + @SuppressWarnings("deprecation") + static class ListenerTester implements WindowResizeListener { + static int resize = 0; + + public void onWindowResized(int width, int height) { + ++resize; + } + + public static void fire() { + resize = 0; + ResizeEvent.fire(Window.handlers, 0, 0); + } + } + + @SuppressWarnings("deprecation") + public void testListenerRemoval() { + + WindowResizeListener r1 = new ListenerTester(); + WindowResizeListener r2 = new ListenerTester(); + + Window.addWindowResizeListener(r1); + Window.addWindowResizeListener(r2); + + ListenerTester.fire(); + assertEquals(ListenerTester.resize, 2); + + Window.removeWindowResizeListener(r1); + ListenerTester.fire(); + assertEquals(ListenerTester.resize, 1); + + Window.removeWindowResizeListener(r2); + ListenerTester.fire(); + assertEquals(ListenerTester.resize, 0); + } + }
diff --git a/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java b/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java index 4421754..c7ba8e2 100644 --- a/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java +++ b/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java
@@ -15,8 +15,10 @@ */ package com.google.gwt.user.client.ui; +import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.logical.shared.ValueChangeEvent; import com.google.gwt.event.logical.shared.ValueChangeHandler; +import com.google.gwt.event.shared.HandlerManager; import com.google.gwt.junit.client.GWTTestCase; import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Element; @@ -29,7 +31,7 @@ @Override public String getModuleName() { return "com.google.gwt.user.DebugTest"; - } + } @Override protected void gwtSetUp() throws Exception { @@ -85,7 +87,7 @@ UIObjectTest.assertDebugId("myCheck-input", newInput); UIObjectTest.assertDebugIdContents("myCheck-label", "myLabel"); } - + public void testValueChangeEvent() { CheckBox cb = new CheckBox(); Handler h = new Handler(); @@ -94,7 +96,7 @@ assertNull(h.received); cb.setChecked(true); assertNull(h.received); - + cb.setValue(false); assertNull(h.received); cb.setValue(true); @@ -102,13 +104,13 @@ cb.setValue(true, true); assertNull(h.received); - + cb.setValue(false, true); assertFalse(h.received); cb.setValue(true, true); assertTrue(h.received); - + try { cb.setValue(null); fail("Should throw IllegalArgumentException"); @@ -116,10 +118,47 @@ /* pass */ } } - + + static class ListenerTester implements ClickListener { + static int fired = 0; + static HandlerManager manager; + + public static void fire() { + fired = 0; + manager.fireEvent(new ClickEvent() { + }); + } + + public void onClick(Widget sender) { + ++fired; + } + } + + @SuppressWarnings("deprecation") + public void testListenerRemoval() { + CheckBox b = new CheckBox(); + + ClickListener r1 = new ListenerTester(); + ClickListener r2 = new ListenerTester(); + ListenerTester.manager = b.ensureHandlers(); + b.addClickListener(r1); + b.addClickListener(r2); + + ListenerTester.fire(); + assertEquals(ListenerTester.fired, 2); + + b.removeClickListener(r1); + ListenerTester.fire(); + assertEquals(ListenerTester.fired, 1); + + b.removeClickListener(r2); + ListenerTester.fire(); + assertEquals(ListenerTester.fired, 0); + } + private static class Handler implements ValueChangeHandler<Boolean> { Boolean received = null; - + public void onValueChange(ValueChangeEvent<Boolean> event) { received = event.getValue(); }