Fixing a bug in CellBrowser where using a shared SelectionModel across all lists and using KeyboardSelectionPolicy.BOUND_TO_SELECTION causes the lists to fight for selection.  The result is that items in lower index lists (to the left) end up being selected instead of items in the highest index list (rightmost).  This is a fairly complicated problem that requires custom handling of selection state within CellBrowser.

Also fixing a bug in HasDataPresenter where changes to the temporary state aren't propagated to the new pending state if a selection event interupts the SelectionModel part of the state resolution loop.  The problem is that we update the new pending state as we go along, but in most cases the new pending state won't exist until we're halfway through the SelectionModel resolution code.  The fix is to copy all modified fields from the temporary state into the new pending state at the end of the selection resolution logic, when we know the new pending state exists.

Review at http://gwt-code-reviews.appspot.com/1585803

Review by: skybrian@google.com

git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10731 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/cellview/client/CellBrowser.java b/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
index fd1965f..32e81df 100644
--- a/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
@@ -214,6 +214,11 @@
     private Object focusedKey;
 
     /**
+     * The currently selected value in this list.
+     */
+    private T selectedValue;
+
+    /**
      * A boolean indicating that this widget is no longer used.
      */
     private boolean isDestroyed;
@@ -233,6 +238,13 @@
       this.level = level;
     }
 
+    protected void deselectValue() {
+      SelectionModel<? super T> selectionModel = getSelectionModel();
+      if (selectionModel != null && selectedValue != null) {
+        selectionModel.setSelected(selectedValue, false);
+      }
+    }
+
     @Override
     protected Element getCellParent(Element item) {
       return item.getFirstChildElement().getNextSiblingElement();
@@ -370,6 +382,31 @@
     }
 
     /**
+     * Set the selected value in this list. If there is already a selected
+     * value, the old value will be deselected.
+     * 
+     * @param value the selected value
+     */
+    protected void setSelectedValue(T value) {
+      // Early exit if the value is unchanged.
+      Object oldKey = getValueKey(selectedValue);
+      Object newKey = getValueKey(value);
+      if (newKey != null && newKey.equals(oldKey)) {
+        return;
+      }
+
+      // Deselect the current value. Only one thing is selected at a time.
+      deselectValue();
+
+      // Select the new value.
+      SelectionModel<? super T> selectionModel = getSelectionModel();
+      if (selectionModel != null) {
+        selectedValue = value;
+        selectionModel.setSelected(selectedValue, true);
+      }
+    }
+
+    /**
      * Check if the specified index is currently open. An index is open if it is
      * the keyboard selected index, there is an associated keyboard selected
      * value, and the value is not a leaf.
@@ -444,6 +481,7 @@
 
       // Trim to the current level if the open node disappears.
       valueChangeHandler = display.addValueChangeHandler(new ValueChangeHandler<List<C>>() {
+        @Override
         public void onValueChange(ValueChangeEvent<List<C>> event) {
           Object focusedKey = display.focusedKey;
           if (focusedKey != null) {
@@ -463,38 +501,45 @@
       });
     }
 
+    @Override
     public int getChildCount() {
       assertNotDestroyed();
       return display.getPresenter().getVisibleItemCount();
     }
 
+    @Override
     public C getChildValue(int index) {
       assertNotDestroyed();
       checkChildBounds(index);
       return display.getVisibleItem(index);
     }
 
+    @Override
     public int getIndex() {
       assertNotDestroyed();
       TreeNodeImpl<?> parent = getParent();
       return (parent == null) ? 0 : parent.getOpenIndex();
     }
 
+    @Override
     public TreeNodeImpl<?> getParent() {
       assertNotDestroyed();
       return getParentImpl();
     }
 
+    @Override
     public Object getValue() {
       return value;
     }
 
+    @Override
     public boolean isChildLeaf(int index) {
       assertNotDestroyed();
       checkChildBounds(index);
       return isLeaf(getChildValue(index));
     }
 
+    @Override
     public boolean isChildOpen(int index) {
       assertNotDestroyed();
       checkChildBounds(index);
@@ -502,6 +547,7 @@
           .equals(display.getValueKey(getChildValue(index)));
     }
 
+    @Override
     public boolean isDestroyed() {
       if (nodeInfo != null) {
         /*
@@ -516,10 +562,12 @@
       return nodeInfo == null;
     }
 
+    @Override
     public TreeNode setChildOpen(int index, boolean open) {
       return setChildOpen(index, open, true);
     }
 
+    @Override
     public TreeNode setChildOpen(int index, boolean open, boolean fireEvents) {
       assertNotDestroyed();
       checkChildBounds(index);
@@ -582,6 +630,7 @@
     private void destroy() {
       display.isDestroyed = true;
       valueChangeHandler.removeHandler();
+      display.deselectValue();
       display.setSelectionModel(null);
       nodeInfo.unsetDataDisplay();
       getSplitLayoutPanel().remove(widget);
@@ -621,10 +670,12 @@
       this.style = new CellListStyleImpl(delegate.cellBrowserStyle());
     }
 
+    @Override
     public ImageResource cellListSelectedBackground() {
       return delegate.cellBrowserSelectedBackground();
     }
 
+    @Override
     public CellList.Style cellListStyle() {
       return style;
     }
@@ -642,35 +693,43 @@
       this.delegate = delegate;
     }
 
+    @Override
     public String cellListEvenItem() {
       return delegate.cellBrowserEvenItem();
     }
 
+    @Override
     public String cellListKeyboardSelectedItem() {
       return delegate.cellBrowserKeyboardSelectedItem();
     }
 
+    @Override
     public String cellListOddItem() {
       return delegate.cellBrowserOddItem();
     }
 
+    @Override
     public String cellListSelectedItem() {
       return delegate.cellBrowserSelectedItem();
     }
 
+    @Override
     public String cellListWidget() {
       // Do not apply any style to the list itself.
       return null;
     }
 
+    @Override
     public boolean ensureInjected() {
       return delegate.ensureInjected();
     }
 
+    @Override
     public String getName() {
       return delegate.getName();
     }
 
+    @Override
     public String getText() {
       return delegate.getText();
     }
@@ -725,8 +784,8 @@
   /**
    * The element used in place of an image when a node has no children.
    */
-  private static final SafeHtml LEAF_IMAGE =
-      SafeHtmlUtils.fromSafeConstant("<div style='position:absolute;display:none;'></div>");
+  private static final SafeHtml LEAF_IMAGE = SafeHtmlUtils
+      .fromSafeConstant("<div style='position:absolute;display:none;'></div>");
 
   private static Template template;
 
@@ -878,6 +937,7 @@
     return treeNodes.get(0);
   }
 
+  @Override
   public boolean isAnimationEnabled() {
     return isAnimationEnabled;
   }
@@ -893,10 +953,12 @@
     super.onBrowserEvent(event);
   }
 
+  @Override
   public void onResize() {
     getSplitLayoutPanel().onResize();
   }
 
+  @Override
   public void setAnimationEnabled(boolean enable) {
     this.isAnimationEnabled = enable;
   }
@@ -911,20 +973,6 @@
     this.defaultWidth = width;
   }
 
-  @Override
-  public void setKeyboardSelectionPolicy(KeyboardSelectionPolicy policy) {
-    super.setKeyboardSelectionPolicy(policy);
-
-    /*
-     * Set the policy on all lists. We use keyboard selection to track the open
-     * node, so we never actually disable keyboard selection on the lists.
-     */
-    policy = getKeyboardSelectionPolicyForLists();
-    for (TreeNodeImpl<?> treeNode : treeNodes) {
-      treeNode.display.setKeyboardSelectionPolicy(policy);
-    }
-  }
-
   /**
    * Set the minimum width of columns.
    * 
@@ -1030,9 +1078,13 @@
         new BrowserCellList<C>(nodeInfo.getCell(), level, nodeInfo.getProvidesKey());
     display.setValueUpdater(nodeInfo.getValueUpdater());
 
-    // Set the keyboard selection policy, but never disable it.
-    KeyboardSelectionPolicy keyboardPolicy = getKeyboardSelectionPolicyForLists();
-    display.setKeyboardSelectionPolicy(keyboardPolicy);
+    /*
+     * A CellBrowser has a single keyboard selection policy and multiple lists,
+     * so we're not using the selection policy in each list. Leave them on all
+     * the time because we use keyboard selection to keep track of which item is
+     * open (selected) at each level.
+     */
+    display.setKeyboardSelectionPolicy(KeyboardSelectionPolicy.ENABLED);
     return display;
   }
 
@@ -1059,18 +1111,6 @@
   }
 
   /**
-   * Get the {@link KeyboardSelectionPolicy} to apply to lists. We use keyboard
-   * selection to track the open node, so we never actually disable keyboard
-   * selection on the lists.
-   * 
-   * @return the {@link KeyboardSelectionPolicy} to use on lists
-   */
-  private KeyboardSelectionPolicy getKeyboardSelectionPolicyForLists() {
-    KeyboardSelectionPolicy policy = getKeyboardSelectionPolicy();
-    return KeyboardSelectionPolicy.DISABLED == policy ? KeyboardSelectionPolicy.ENABLED : policy;
-  }
-
-  /**
    * Get the {@link SplitLayoutPanel} used to lay out the views.
    * 
    * @return the {@link SplitLayoutPanel}
@@ -1110,8 +1150,6 @@
    * {@link TreeNode}s as needed.
    * 
    * @param cellList the CellList that changed state.
-   * @param value the value to open
-   * @param open true to open, false to close
    * @param fireEvents true to fireEvents
    * @return the open {@link TreeNode}, or null if not opened
    */
@@ -1148,7 +1186,12 @@
         // The node is already open.
         openNode = cellList.isFocusedOpen ? treeNodes.get(cellList.level + 1) : null;
       } else {
-        // Add the child node.
+        // Select this value.
+        if (KeyboardSelectionPolicy.BOUND_TO_SELECTION == getKeyboardSelectionPolicy()) {
+          cellList.setSelectedValue(newValue);
+        }
+
+        // Add the child node if this node has children.
         cellList.focusedKey = newKey;
         NodeInfo<?> childNodeInfo = isLeaf(newValue) ? null : getNodeInfo(newValue);
         if (childNodeInfo != null) {
diff --git a/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java b/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
index 0c1e01f..76397f8 100644
--- a/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
+++ b/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
@@ -1056,7 +1056,7 @@
    * Resolve the pending state and push updates to the view.
    * 
    * @param modifiedRows the modified rows that need to be updated, or null if
-   *        none. The modified rows may be mutated.
+   *          none. The modified rows may be mutated.
    * @return true if the state changed, false if not
    */
   private boolean resolvePendingState(JsArrayInteger modifiedRows) {
@@ -1098,7 +1098,7 @@
      * create a new pendingState.
      */
     State<T> oldState = state;
-    PendingState<T> pending = pendingState;
+    PendingState<T> newState = pendingState;
     state = pendingState;
     pendingState = null;
 
@@ -1113,40 +1113,40 @@
     }
 
     // Get the values used for calculations.
-    int pageStart = pending.getPageStart();
-    int pageSize = pending.getPageSize();
+    int pageStart = newState.getPageStart();
+    int pageSize = newState.getPageSize();
     int pageEnd = pageStart + pageSize;
-    int rowDataCount = pending.getRowDataSize();
+    int rowDataCount = newState.getRowDataSize();
 
     /*
      * Resolve keyboard selection. If the row value still exists, use its index.
      * If the row value exists in multiple places, use the closest index. If the
      * row value no longer exists, use the current index.
      */
-    pending.keyboardSelectedRow =
-        Math.max(0, Math.min(pending.keyboardSelectedRow, rowDataCount - 1));
+    newState.keyboardSelectedRow =
+        Math.max(0, Math.min(newState.keyboardSelectedRow, rowDataCount - 1));
     if (KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy) {
       // Clear the keyboard selected state.
-      pending.keyboardSelectedRow = 0;
-      pending.keyboardSelectedRowValue = null;
-    } else if (pending.keyboardSelectedRowChanged) {
+      newState.keyboardSelectedRow = 0;
+      newState.keyboardSelectedRowValue = null;
+    } else if (newState.keyboardSelectedRowChanged) {
       // Choose the row value based on the index.
-      pending.keyboardSelectedRowValue =
-          rowDataCount > 0 ? pending.getRowDataValue(pending.keyboardSelectedRow) : null;
-    } else if (pending.keyboardSelectedRowValue != null) {
+      newState.keyboardSelectedRowValue =
+          rowDataCount > 0 ? newState.getRowDataValue(newState.keyboardSelectedRow) : null;
+    } else if (newState.keyboardSelectedRowValue != null) {
       // Choose the index based on the row value.
       int bestMatchIndex =
-          findIndexOfBestMatch(pending, pending.keyboardSelectedRowValue,
-              pending.keyboardSelectedRow);
+          findIndexOfBestMatch(newState, newState.keyboardSelectedRowValue,
+              newState.keyboardSelectedRow);
       if (bestMatchIndex >= 0) {
         // A match was found.
-        pending.keyboardSelectedRow = bestMatchIndex;
-        pending.keyboardSelectedRowValue =
-            rowDataCount > 0 ? pending.getRowDataValue(pending.keyboardSelectedRow) : null;
+        newState.keyboardSelectedRow = bestMatchIndex;
+        newState.keyboardSelectedRowValue =
+            rowDataCount > 0 ? newState.getRowDataValue(newState.keyboardSelectedRow) : null;
       } else {
         // No match was found, so reset to 0.
-        pending.keyboardSelectedRow = 0;
-        pending.keyboardSelectedRowValue = null;
+        newState.keyboardSelectedRow = 0;
+        newState.keyboardSelectedRowValue = null;
       }
     }
 
@@ -1157,11 +1157,11 @@
      */
     try {
       if (KeyboardSelectionPolicy.BOUND_TO_SELECTION == keyboardSelectionPolicy
-          && selectionModel != null && pending.viewTouched) {
+          && selectionModel != null && newState.viewTouched) {
         T oldValue = oldState.getSelectedValue();
         Object oldKey = getRowValueKey(oldValue);
         T newValue =
-            rowDataCount > 0 ? pending.getRowDataValue(pending.getKeyboardSelectedRow()) : null;
+            rowDataCount > 0 ? newState.getRowDataValue(newState.getKeyboardSelectedRow()) : null;
         Object newKey = getRowValueKey(newValue);
         /*
          * Do not deselect the old value unless we have a new value to select,
@@ -1183,21 +1183,15 @@
             }
 
             // Select the new value.
-            pending.selectedValue = newValue;
+            newState.selectedValue = newValue;
             if (newValue != null && !newValueWasSelected) {
               selectionModel.setSelected(newValue, true);
             }
           } else if (!newValueWasSelected) {
             // The value was programmatically deselected.
-            pending.selectedValue = null;
+            newState.selectedValue = null;
           }
         }
-
-        // User code may have created a new pending state, so we need to
-        // propagate this new selected value.
-        if (pendingState != null) {
-          pendingState.selectedValue = pending.selectedValue;
-        }
       }
     } catch (RuntimeException e) {
       // Unlock the rendering loop if the user SelectionModel throw an error.
@@ -1208,9 +1202,9 @@
 
     // If the keyboard row changes, add it to the modified set.
     boolean keyboardRowChanged =
-        pending.keyboardSelectedRowChanged
-            || (oldState.getKeyboardSelectedRow() != pending.keyboardSelectedRow)
-            || (oldState.getKeyboardSelectedRowValue() == null && pending.keyboardSelectedRowValue != null);
+        newState.keyboardSelectedRowChanged
+            || (oldState.getKeyboardSelectedRow() != newState.keyboardSelectedRow)
+            || (oldState.getKeyboardSelectedRowValue() == null && newState.keyboardSelectedRowValue != null);
 
     /*
      * Resolve selection. Check the selection status of all row values in the
@@ -1218,21 +1212,19 @@
      * longer have a SelectionModel but had selected rows, we still need to
      * update the rows.
      */
+    Set<Integer> newlySelectedRows = new HashSet<Integer>();
     try {
       for (int i = pageStart; i < pageStart + rowDataCount; i++) {
         // Check the new selection state.
-        T rowValue = pending.getRowDataValue(i - pageStart);
+        T rowValue = newState.getRowDataValue(i - pageStart);
         boolean isSelected =
             (rowValue != null && selectionModel != null && selectionModel.isSelected(rowValue));
 
         // Compare to the old selection state.
         boolean wasSelected = oldState.isRowSelected(i);
         if (isSelected) {
-          pending.selectedRows.add(i);
-          if (pendingState != null) {
-            // Propagate changes if user code created a new pending state.
-            pendingState.selectedRows.add(i);
-          }
+          newState.selectedRows.add(i);
+          newlySelectedRows.add(i);
           if (!wasSelected) {
             modifiedRows.push(i);
           }
@@ -1249,7 +1241,7 @@
 
     // Add the replaced ranges as modified rows.
     boolean replacedEmptyRange = false;
-    for (Range replacedRange : pending.replacedRanges) {
+    for (Range replacedRange : newState.replacedRanges) {
       int start = replacedRange.getStart();
       int length = replacedRange.getLength();
       // If the user set an empty range, pass it through to the view.
@@ -1264,7 +1256,7 @@
     // Add keyboard rows to modified rows if we are going to render anyway.
     if (modifiedRows.length() > 0 && keyboardRowChanged) {
       modifiedRows.push(oldState.getKeyboardSelectedRow());
-      modifiedRows.push(pending.keyboardSelectedRow);
+      modifiedRows.push(newState.keyboardSelectedRow);
     }
 
     /*
@@ -1275,16 +1267,28 @@
       isResolvingState = false;
       // Do not reset pendingStateLoop, or we will not detect the infinite loop.
 
+      // Propagate modifications to the temporary pending state into the new
+      // pending state instance.
+      pendingState.selectedValue = newState.selectedValue;
+      pendingState.selectedRows.addAll(newlySelectedRows);
+      if (keyboardRowChanged) {
+        pendingState.keyboardSelectedRowChanged = true;
+      }
+      if (newState.keyboardStealFocus) {
+        pendingState.keyboardStealFocus = true;
+      }
+
       /*
        * Add the keyboard selected rows to the modified rows so they can be
        * re-rendered in the new state. These rows may already be added, but
        * modifiedRows can contain duplicates.
        */
       modifiedRows.push(oldState.getKeyboardSelectedRow());
-      modifiedRows.push(pending.keyboardSelectedRow);
+      modifiedRows.push(newState.keyboardSelectedRow);
 
       /*
-       * Try to resolve state again. If we are successful, then the modified
+       * Make a recursive call to resolve the state again, using the new pending
+       * state that was just created. If we are successful, then the modified
        * rows will be redrawn. If we are not successful, then we still need to
        * redraw the modified rows.
        */
@@ -1308,7 +1312,7 @@
     int oldPageStart = oldState.getPageStart();
     int oldPageSize = oldState.getPageSize();
     int oldRowDataCount = oldState.getRowDataSize();
-    boolean redrawRequired = pending.redrawRequired;
+    boolean redrawRequired = newState.redrawRequired;
     if (pageStart != oldPageStart) {
       // Redraw if pageStart changes.
       redrawRequired = true;
@@ -1344,7 +1348,7 @@
       if (redrawRequired) {
         // Redraw the entire content.
         SafeHtmlBuilder sb = new SafeHtmlBuilder();
-        view.replaceAllChildren(pending.rowData, selectionModel, pending.keyboardStealFocus);
+        view.replaceAllChildren(newState.rowData, selectionModel, newState.keyboardStealFocus);
         view.resetFocus();
       } else if (range0 != null) {
         // Surgically replace specific rows.
@@ -1354,8 +1358,8 @@
           int absStart = range0.getStart();
           int relStart = absStart - pageStart;
           SafeHtmlBuilder sb = new SafeHtmlBuilder();
-          List<T> replaceValues = pending.rowData.subList(relStart, relStart + range0.getLength());
-          view.replaceChildren(replaceValues, relStart, selectionModel, pending.keyboardStealFocus);
+          List<T> replaceValues = newState.rowData.subList(relStart, relStart + range0.getLength());
+          view.replaceChildren(replaceValues, relStart, selectionModel, newState.keyboardStealFocus);
         }
 
         // Replace range1 if it exists.
@@ -1363,8 +1367,8 @@
           int absStart = range1.getStart();
           int relStart = absStart - pageStart;
           SafeHtmlBuilder sb = new SafeHtmlBuilder();
-          List<T> replaceValues = pending.rowData.subList(relStart, relStart + range1.getLength());
-          view.replaceChildren(replaceValues, relStart, selectionModel, pending.keyboardStealFocus);
+          List<T> replaceValues = newState.rowData.subList(relStart, relStart + range1.getLength());
+          view.replaceChildren(replaceValues, relStart, selectionModel, newState.keyboardStealFocus);
         }
 
         view.resetFocus();
@@ -1377,9 +1381,9 @@
         }
 
         // Select the new keyboard row.
-        int newSelectedRow = pending.getKeyboardSelectedRow();
+        int newSelectedRow = newState.getKeyboardSelectedRow();
         if (newSelectedRow >= 0 && newSelectedRow < rowDataCount) {
-          view.setKeyboardSelected(newSelectedRow, true, pending.keyboardStealFocus);
+          view.setKeyboardSelected(newSelectedRow, true, newState.keyboardStealFocus);
         }
       }
     } catch (Error e) {
@@ -1394,8 +1398,12 @@
       isResolvingState = false;
     }
 
-    // If any new state exists, resolve it now. If there is no new state, reset
-    // the pendingStateLoop.
+    /*
+     * Make a recursive call to resolve any pending state. We don't expect
+     * pending state here, but its always possible that pushing the changes into
+     * the view could update the presenter. If there is no new state, the
+     * recursive call will reset the pendingStateLoop.
+     */
     resolvePendingState(null);
     return true;
   }
diff --git a/user/test/com/google/gwt/user/cellview/client/CellBrowserTest.java b/user/test/com/google/gwt/user/cellview/client/CellBrowserTest.java
index 273f9d1..8dfda9c 100644
--- a/user/test/com/google/gwt/user/cellview/client/CellBrowserTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/CellBrowserTest.java
@@ -20,6 +20,8 @@
 import com.google.gwt.user.cellview.client.CellBrowser.BrowserCellList;
 import com.google.gwt.user.cellview.client.HasKeyboardSelectionPolicy.KeyboardSelectionPolicy;
 import com.google.gwt.view.client.ListDataProvider;
+import com.google.gwt.view.client.SelectionModel;
+import com.google.gwt.view.client.SingleSelectionModel;
 import com.google.gwt.view.client.TreeViewModel;
 
 /**
@@ -33,6 +35,17 @@
    */
   protected class MixedTreeViewModel implements TreeViewModel {
 
+    private final SelectionModel<Object> selectionModel;
+
+    public MixedTreeViewModel() {
+      this(null);
+    }
+
+    public MixedTreeViewModel(SelectionModel<Object> selectionModel) {
+      this.selectionModel = selectionModel;
+    }
+
+    @Override
     public <T> NodeInfo<?> getNodeInfo(T value) {
       if (value == null) {
         // Get the children of root, which are Integers.
@@ -40,15 +53,20 @@
         for (int i = 0; i < 10; i++) {
           provider.getList().add(new Integer(i));
         }
-        return new DefaultNodeInfo<Number>(provider, new NumberCell());
+        return new DefaultNodeInfo<Number>(provider, new NumberCell(), selectionModel, null);
       } else if (value instanceof Integer && !isLeaf(value)) {
         // Get the children of odd Integers, which are Strings.
         ListDataProvider<String> provider = new ListDataProvider<String>();
-        return new DefaultNodeInfo<String>(provider, new TextCell());
+        for (int i = 0; i < 10; i++) {
+          char c = (char) ('a' + i);
+          provider.getList().add("" + c);
+        }
+        return new DefaultNodeInfo<String>(provider, new TextCell(), selectionModel, null);
       }
       throw new IllegalArgumentException("Unexpected value: " + value);
     }
 
+    @Override
     public boolean isLeaf(Object value) {
       if (value == null) {
         // Root value is null.
@@ -56,6 +74,8 @@
       } else if (value instanceof Integer) {
         // Odd integers are leaf nodes
         return ((Integer) value % 2) != 0;
+      } else if (value instanceof String) {
+        return true;
       }
       return false;
     }
@@ -66,6 +86,57 @@
   }
 
   /**
+   * Test that the CellBrowser correctly updates a shared SelectionModel if it
+   * is bound to selection.
+   */
+  public void testBoundToSharedSelectionModel() {
+    SingleSelectionModel<Object> selectionModel = new SingleSelectionModel<Object>();
+    CellBrowser browser = new CellBrowser(new MixedTreeViewModel(selectionModel), null);
+    browser.setKeyboardSelectionPolicy(KeyboardSelectionPolicy.BOUND_TO_SELECTION);
+
+    // Select a leaf.
+    browser.treeNodes.get(0).getDisplay().getPresenter().setKeyboardSelectedRow(1, true, false);
+    browser.treeNodes.get(0).getDisplay().getPresenter().flush();
+    assertTrue(selectionModel.isSelected(1));
+
+    // Select another leaf.
+    browser.treeNodes.get(0).getDisplay().getPresenter().setKeyboardSelectedRow(3, true, false);
+    browser.treeNodes.get(0).getDisplay().getPresenter().flush();
+    assertTrue(selectionModel.isSelected(3));
+
+    // Select a non-leaf.
+    browser.treeNodes.get(0).getDisplay().getPresenter().setKeyboardSelectedRow(2, true, false);
+    browser.treeNodes.get(0).getDisplay().getPresenter().flush();
+    assertTrue(selectionModel.isSelected(2));
+
+    // Select a leaf in the child list.
+    browser.treeNodes.get(1).getDisplay().getPresenter().setKeyboardSelectedRow(5, true, false);
+    browser.treeNodes.get(1).getDisplay().getPresenter().flush();
+    assertTrue(selectionModel.isSelected("f"));
+
+    // Verify that flushing the selection model doesn't change the selection
+    // back to the parent list.
+    browser.treeNodes.get(0).getDisplay().getPresenter().flush();
+    assertTrue(selectionModel.isSelected("f"));
+
+    // Verify that redrawing the parent list doesn't change the selection back
+    // to the parent list.
+    browser.treeNodes.get(0).getDisplay().redraw();
+    browser.treeNodes.get(0).getDisplay().getPresenter().flush();
+    assertTrue(selectionModel.isSelected("f"));
+
+    // Select a non-leaf in the parent.
+    browser.treeNodes.get(0).getDisplay().getPresenter().setKeyboardSelectedRow(4, true, false);
+    browser.treeNodes.get(0).getDisplay().getPresenter().flush();
+    assertTrue(selectionModel.isSelected(4));
+
+    // isSelected() triggers the final SelectionChangeEvent. Verify the end
+    // state is still correct.
+    browser.treeNodes.get(0).getDisplay().getPresenter().flush();
+    assertTrue(selectionModel.isSelected(4));
+  }
+
+  /**
    * Verify that closing a leaf node sets the focused key to null.
    */
   public void testCloseLeafNode() {
@@ -128,8 +199,7 @@
 
     // Bind keyboard selection to the selection model.
     browser.setKeyboardSelectionPolicy(KeyboardSelectionPolicy.BOUND_TO_SELECTION);
-    assertEquals(KeyboardSelectionPolicy.BOUND_TO_SELECTION,
-        browser.getKeyboardSelectionPolicy());
+    assertEquals(KeyboardSelectionPolicy.BOUND_TO_SELECTION, browser.getKeyboardSelectionPolicy());
 
     // Select an item at depth 0. Nothing should be selected at depth 1.
     BrowserCellList<?> list0 = browser.treeNodes.get(0).getDisplay();
@@ -154,19 +224,16 @@
 
     // Disable keyboard selection.
     browser.setKeyboardSelectionPolicy(KeyboardSelectionPolicy.DISABLED);
-    assertEquals(KeyboardSelectionPolicy.DISABLED,
-        browser.getKeyboardSelectionPolicy());
+    assertEquals(KeyboardSelectionPolicy.DISABLED, browser.getKeyboardSelectionPolicy());
 
     // Verify that keyboard selection is enabled in the lists.
     BrowserCellList<?> list = browser.treeNodes.get(0).getDisplay();
-    assertEquals(KeyboardSelectionPolicy.ENABLED,
-        list.getKeyboardSelectionPolicy());
+    assertEquals(KeyboardSelectionPolicy.ENABLED, list.getKeyboardSelectionPolicy());
     assertTrue(list.isKeyboardNavigationSuppressed());
   }
 
   @Override
-  protected <T> CellBrowser createAbstractCellTree(TreeViewModel model,
-      T rootValue) {
+  protected <T> CellBrowser createAbstractCellTree(TreeViewModel model, T rootValue) {
     CellBrowser browser = new CellBrowser(model, rootValue);
     browser.setHeight("500px");
     return browser;