Revision: 10475
Author:   jlaba...@google.com
Date:     Thu Jul 28 04:00:59 2011
Log: 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: andych...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=10475

Modified:
 /trunk/user/src/com/google/gwt/user/cellview/client/AbstractPager.java
 /trunk/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java

=======================================
--- /trunk/user/src/com/google/gwt/user/cellview/client/AbstractPager.java Tue Oct 12 07:55:56 2010 +++ /trunk/user/src/com/google/gwt/user/cellview/client/AbstractPager.java Thu Jul 28 04:00:59 2011
@@ -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();
=======================================
--- /trunk/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java Tue Aug 17 10:14:36 2010 +++ /trunk/user/test/com/google/gwt/user/cellview/client/AbstractPagerTest.java Thu Jul 28 04:00:59 2011
@@ -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}.
@@ -235,6 +237,45 @@
     pager.previousPage();
     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();

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

Reply via email to