Fixes a bug in AbstractPager where clearing the display and resetting it causes an NPE.
Review at http://gwt-code-reviews.appspot.com/1500803
Review by: andycheng@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10475 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/cellview/client/AbstractPager.java b/user/src/com/google/gwt/user/cellview/client/AbstractPager.java
index 1ea9866..bb2457b 100644
--- a/user/src/com/google/gwt/user/cellview/client/AbstractPager.java
+++ b/user/src/com/google/gwt/user/cellview/client/AbstractPager.java
@@ -27,6 +27,10 @@
*/
public abstract class AbstractPager extends Composite {
+ // Visible for testing.
+ HandlerRegistration rangeChangeHandler;
+ HandlerRegistration rowCountChangeHandler;
+
private HasRows display;
/**
@@ -39,9 +43,6 @@
*/
private int lastRowCount;
- private HandlerRegistration rangeChangeHandler;
- private HandlerRegistration rowCountChangeHandler;
-
/**
* Get the {@link HasRows} being paged.
*
@@ -109,29 +110,28 @@
}
if (rowCountChangeHandler != null) {
rowCountChangeHandler.removeHandler();
- rangeChangeHandler = null;
+ rowCountChangeHandler = null;
}
// Set the new display.
this.display = display;
if (display != null) {
- rangeChangeHandler = display.addRangeChangeHandler(
- new RangeChangeEvent.Handler() {
- public void onRangeChange(RangeChangeEvent event) {
- if (AbstractPager.this.display != null) {
- onRangeOrRowCountChanged();
- }
- }
- });
- rowCountChangeHandler = display.addRowCountChangeHandler(
- new RowCountChangeEvent.Handler() {
- public void onRowCountChange(RowCountChangeEvent event) {
- if (AbstractPager.this.display != null) {
- handleRowCountChange(
- event.getNewRowCount(), event.isNewRowCountExact());
- }
- }
- });
+ rangeChangeHandler = display.addRangeChangeHandler(new RangeChangeEvent.Handler() {
+ @Override
+ public void onRangeChange(RangeChangeEvent event) {
+ if (AbstractPager.this.display != null) {
+ onRangeOrRowCountChanged();
+ }
+ }
+ });
+ rowCountChangeHandler = display.addRowCountChangeHandler(new RowCountChangeEvent.Handler() {
+ @Override
+ public void onRowCountChange(RowCountChangeEvent event) {
+ if (AbstractPager.this.display != null) {
+ handleRowCountChange(event.getNewRowCount(), event.isNewRowCountExact());
+ }
+ }
+ });
// Initialize the pager.
onRangeOrRowCountChanged();
diff --git a/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java b/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java
index 7e93928..ef59c6b 100644
--- a/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java
@@ -19,6 +19,8 @@
import com.google.gwt.view.client.HasRows;
import com.google.gwt.view.client.MockHasData;
import com.google.gwt.view.client.Range;
+import com.google.gwt.view.client.RangeChangeEvent;
+import com.google.gwt.view.client.RowCountChangeEvent;
/**
* Tests for {@link AbstractPager}.
@@ -236,6 +238,45 @@
assertEquals(new Range(25, 20), display.getVisibleRange());
}
+ public void testSetDisplay() {
+ AbstractPager pager = createPager();
+ assertNull(pager.getDisplay());
+
+ // Set display to a value.
+ MockHasData<String> display0 = new MockHasData<String>();
+ pager.setDisplay(display0);
+ assertEquals(display0, pager.getDisplay());
+ assertEquals(1, display0.getHandlerCount(RangeChangeEvent.getType()));
+ assertEquals(1, display0.getHandlerCount(RowCountChangeEvent.getType()));
+ assertNotNull(pager.rangeChangeHandler);
+ assertNotNull(pager.rowCountChangeHandler);
+
+ /*
+ * Set display to null.
+ *
+ * Verify that the handlers are removed.
+ */
+ pager.setDisplay(null);
+ assertNull(pager.getDisplay());
+ assertEquals(0, display0.getHandlerCount(RangeChangeEvent.getType()));
+ assertEquals(0, display0.getHandlerCount(RowCountChangeEvent.getType()));
+ assertNull(pager.rangeChangeHandler);
+ assertNull(pager.rowCountChangeHandler);
+
+ /*
+ * Set display again.
+ *
+ * Verify that the handlers are re-added.
+ */
+ MockHasData<String> display1 = new MockHasData<String>();
+ pager.setDisplay(display1);
+ assertEquals(display1, pager.getDisplay());
+ assertEquals(1, display1.getHandlerCount(RangeChangeEvent.getType()));
+ assertEquals(1, display1.getHandlerCount(RowCountChangeEvent.getType()));
+ assertNotNull(pager.rangeChangeHandler);
+ assertNotNull(pager.rowCountChangeHandler);
+ }
+
public void testSetPage() {
AbstractPager pager = createPager();
