TreeView on node deletion, select adjacent node before selecting parent node.

Previous behavior would always select the parent node when a child was deleted. Unless the node was a child of the root node and the root wasn't visible, then select the first child of the root node.

Now the behavior will select the child node after the deleted node(s). If the deleted node was the last node and more children remain, then select the new last node. If there are no remaining children, then select the parent node.

Scenario 1 - Simple delete of child node

root
+--child1
   +--child1
   +--|child2|  <--- Delete this node.
   +--child3

Result:

root
+--child1
   +--child1
   +--|child3|

Scenario 2 - Delete of a immediate child of the non-visible root.

>Child1
>Child2
>|Child3|   <--- Delete this node.
>Child4

Result:

>Child1
>Child2
>|Child4|

Scenario 3 - Delete of last child node (other children remain).

root
+--child1
   +--child1
   +--child2
   +--|child3|  <--- Delete this node.

Result:

root
+--child1
   +--child1
   +--|child2|

Scenario 4 - Delete of last child node (no children remain).

root
+--child1
   +--|child1|  <--- Delete this node.

Result:

root
+--|child1|

Bug: 991304
Change-Id: I26bf90fd89b3d0d946bf73b7c1047621ca35308a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761299
Auto-Submit: Allen Bauer <kylixrd@chromium.org>
Commit-Queue: Robert Liao <robliao@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689248}

ui/views/controls/tree/tree_view.cc

@@ -481,11 +481,10 @@ void TreeView::TreeNodesRemoved(TreeModel* model,
     // rather than invoking SetSelectedNode() otherwise, we'll try and use a
     // deleted value.
     selected_node_ = nullptr;
-    TreeModelNode* to_select = parent;
-    if (parent == root_.model_node() && !root_shown_) {
-      const auto& children = model_->GetChildren(parent);
-      to_select = children.empty() ? nullptr : children.front();
-    }
+    const auto& children = model_->GetChildren(parent);
+    TreeModelNode* to_select =
+        children.empty() ? parent
+                         : children[std::min(start, children.size() - 1)];
     SetSelectedNode(to_select);
   }
   if (IsExpanded(parent))

ui/views/controls/tree/tree_view_unittest.cc

@@ -274,18 +274,40 @@ TEST_F(TreeViewTest, TreeNodesRemoved) {
   EXPECT_EQ("c1", GetSelectedNodeTitle());
   model_.Remove(GetNodeByTitle("c")->parent(), GetNodeByTitle("c"));
   EXPECT_EQ("root [a]", TreeViewContentsAsString());
-  EXPECT_EQ("root", GetSelectedNodeTitle());
+  EXPECT_EQ("a", GetSelectedNodeTitle());
+  EXPECT_EQ(2, GetRowCount());
+
+  // Add 'c1', 'c2', 'c3', select 'c2', remove it and 'c3" should be selected.
+  Add(GetNodeByTitle("a"), 0, "c1");
+  Add(GetNodeByTitle("a"), 1, "c2");
+  Add(GetNodeByTitle("a"), 2, "c3");
+  tree_.SetSelectedNode(GetNodeByTitle("c2"));
+  model_.Remove(GetNodeByTitle("c2")->parent(), GetNodeByTitle("c2"));
+  EXPECT_EQ("root [a [c1 c3]]", TreeViewContentsAsString());
+  EXPECT_EQ("c3", GetSelectedNodeTitle());
+  EXPECT_EQ(4, GetRowCount());
+
+  // Now delete 'c3' and then 'c1' should be selected.
+  model_.Remove(GetNodeByTitle("c3")->parent(), GetNodeByTitle("c3"));
+  EXPECT_EQ("root [a [c1]]", TreeViewContentsAsString());
+  EXPECT_EQ("c1", GetSelectedNodeTitle());
+  EXPECT_EQ(3, GetRowCount());
+
+  // Finally delete 'c1' and then 'a' should be selected.
+  model_.Remove(GetNodeByTitle("c1")->parent(), GetNodeByTitle("c1"));
+  EXPECT_EQ("root [a]", TreeViewContentsAsString());
+  EXPECT_EQ("a", GetSelectedNodeTitle());
   EXPECT_EQ(2, GetRowCount());
 
   tree_.SetRootShown(false);
-  // Add 'b' select it and remove it. Because we're not showing the root
-  // selection should change to 'a'.
+  // Add 'b' and 'c', select 'b' and remove it. Selection should change to 'c'.
   Add(GetNodeByTitle("root"), 1, "b");
+  Add(GetNodeByTitle("root"), 2, "c");
   tree_.SetSelectedNode(GetNodeByTitle("b"));
   model_.Remove(GetNodeByTitle("b")->parent(), GetNodeByTitle("b"));
-  EXPECT_EQ("root [a]", TreeViewContentsAsString());
-  EXPECT_EQ("a", GetSelectedNodeTitle());
-  EXPECT_EQ(1, GetRowCount());
+  EXPECT_EQ("root [a c]", TreeViewContentsAsString());
+  EXPECT_EQ("c", GetSelectedNodeTitle());
+  EXPECT_EQ(2, GetRowCount());
 }
 
 // Verifies changing a node title works.