ui/base/models: Simplify TreeNode::GetIndexOf() member function.

BUG=None
TEST=app_unittests --gtest_filter=TreeNodeModelTest.GetIndexOf

R=sky@chromium.org

Review URL: http://codereview.chromium.org/7113015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@88134 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 34 insertions, 29 deletions

diff --git a/ui/base/models/tree_node_model.h b/ui/base/models/tree_node_model.h
index f4b41f8..29443f9 100644
--- a/ui/base/models/tree_node_model.h
+++ b/ui/base/models/tree_node_model.h
@@ -102,6 +102,13 @@ class TreeNode : public TreeModelNode {
     children_->clear();
   }
 
+  // Returns the parent node, or NULL if this is the root node.
+  const NodeType* parent() const { return parent_; }
+  NodeType* parent() { return parent_; }
+
+  // Returns true if this is the root node.
+  bool is_root() const { return parent_ == NULL; }
+
   // Returns the number of children.
   int child_count() const { return static_cast<int>(children_->size()); }
 
@@ -125,21 +132,13 @@ class TreeNode : public TreeModelNode {
         static_cast<const NodeType&>(*this).GetChild(index));
   }
 
-  // Returns the parent node, or NULL if this is the root node.
-  const NodeType* parent() const { return parent_; }
-  NodeType* parent() { return parent_; }
-
-  // Returns true if this is the root node.
-  bool is_root() const { return parent_ == NULL; }
-
   // Returns the index of |node|, or -1 if |node| is not a child of this.
   int GetIndexOf(const NodeType* node) const {
     DCHECK(node);
     typename std::vector<NodeType*>::const_iterator i =
         std::find(children_->begin(), children_->end(), node);
-    if (i != children_->end())
-      return static_cast<int>(i - children_->begin());
-    return -1;
+    return
+        i != children_->end() ? static_cast<int>(i - children_->begin()) : -1;
   }
 
   // Sets the title of the node.
diff --git a/ui/base/models/tree_node_model_unittest.cc b/ui/base/models/tree_node_model_unittest.cc
index b6097c3..051f9ce 100644
--- a/ui/base/models/tree_node_model_unittest.cc
+++ b/ui/base/models/tree_node_model_unittest.cc
@@ -172,28 +172,31 @@ TEST_F(TreeNodeModelTest, RemoveAllNodes) {
 // |   |-- foo1
 // +-- child2
 TEST_F(TreeNodeModelTest, GetIndexOf) {
-  TreeNodeWithValue<int>* root =
-      new TreeNodeWithValue<int>(ASCIIToUTF16("root"), 0);
-  TreeNodeModel<TreeNodeWithValue<int> > model(root);
-  model.AddObserver(this);
-  ClearCounts();
+  TreeNodeWithValue<int> root;
 
-  TreeNodeWithValue<int>* child1 =
-      new TreeNodeWithValue<int>(ASCIIToUTF16("child 1"), 1);
-  model.Add(root, child1, 0);
+  TreeNodeWithValue<int>* child1 = new TreeNodeWithValue<int>(1);
+  root.Add(child1, 0);
 
-  TreeNodeWithValue<int>* child2 =
-      new TreeNodeWithValue<int>(ASCIIToUTF16("child 2"), 2);
-  model.Add(root, child2, 1);
+  TreeNodeWithValue<int>* child2 = new TreeNodeWithValue<int>(2);
+  root.Add(child2, 1);
 
-  TreeNodeWithValue<int>* foo1 =
-      new TreeNodeWithValue<int>(ASCIIToUTF16("foo1"), 0);
-  model.Add(child1, foo1, 0);
+  TreeNodeWithValue<int>* foo1 = new TreeNodeWithValue<int>(0);
+  child1->Add(foo1, 0);
+
+  ASSERT_EQ(-1, root.GetIndexOf(&root));
+  ASSERT_EQ(0, root.GetIndexOf(child1));
+  ASSERT_EQ(1, root.GetIndexOf(child2));
+  ASSERT_EQ(-1, root.GetIndexOf(foo1));
+
+  ASSERT_EQ(-1, child1->GetIndexOf(&root));
+  ASSERT_EQ(-1, child1->GetIndexOf(child1));
+  ASSERT_EQ(-1, child1->GetIndexOf(child2));
+  ASSERT_EQ(0, child1->GetIndexOf(foo1));
 
-  ASSERT_EQ(0, model.GetIndexOf(root, child1));
-  ASSERT_EQ(1, model.GetIndexOf(root, child2));
-  ASSERT_EQ(0, model.GetIndexOf(child1, foo1));
-  ASSERT_EQ(-1, model.GetIndexOf(root, foo1));
+  ASSERT_EQ(-1, child2->GetIndexOf(&root));
+  ASSERT_EQ(-1, child2->GetIndexOf(child2));
+  ASSERT_EQ(-1, child2->GetIndexOf(child1));
+  ASSERT_EQ(-1, child2->GetIndexOf(foo1));
 }
 
 // Verify whether a specified node has or not an ancestor.
diff --git a/views/view.cc b/views/view.cc
index b130be0..c9fabb0 100644
--- a/views/view.cc
+++ b/views/view.cc
@@ -212,7 +212,7 @@ bool View::Contains(const View* view) const {
 
 int View::GetIndexOf(const View* view) const {
   Views::const_iterator i(std::find(children_.begin(), children_.end(), view));
-  return i != children_.end() ? i - children_.begin() : -1;
+  return i != children_.end() ? static_cast<int>(i - children_.begin()) : -1;
 }
 
 // TODO(beng): remove
diff --git a/views/view_unittest.cc b/views/view_unittest.cc
index e2ea01b..8924acf 100644
--- a/views/view_unittest.cc
+++ b/views/view_unittest.cc
@@ -2045,16 +2045,19 @@ TEST_F(ViewTest, GetIndexOf) {
   View* foo1 = new View();
   child1->AddChildView(foo1);
 
+  ASSERT_EQ(-1, root.GetIndexOf(NULL));
   ASSERT_EQ(-1, root.GetIndexOf(&root));
   ASSERT_EQ(0, root.GetIndexOf(child1));
   ASSERT_EQ(1, root.GetIndexOf(child2));
   ASSERT_EQ(-1, root.GetIndexOf(foo1));
 
+  ASSERT_EQ(-1, child1->GetIndexOf(NULL));
   ASSERT_EQ(-1, child1->GetIndexOf(&root));
   ASSERT_EQ(-1, child1->GetIndexOf(child1));
   ASSERT_EQ(-1, child1->GetIndexOf(child2));
   ASSERT_EQ(0, child1->GetIndexOf(foo1));
 
+  ASSERT_EQ(-1, child2->GetIndexOf(NULL));
   ASSERT_EQ(-1, child2->GetIndexOf(&root));
   ASSERT_EQ(-1, child2->GetIndexOf(child2));
   ASSERT_EQ(-1, child2->GetIndexOf(child1));