Hook up the browser in the bookmark editor. Adjust the logic controlling the OK button enabling.

In the BookmarkEditor.xib: Connect the NSBrowser's delegate (showing the bookmark tree) to the File's Owner (BookmarkEditorController). 
Patch from Mike Rossetti (mrosseti@chromium.org)
Original review at: http://codereview.chromium.org/287014
BUG=17613
TEST=none

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

4 files changed, 396 insertions, 68 deletions

diff --git a/chrome/app/nibs/BookmarkEditor.xib b/chrome/app/nibs/BookmarkEditor.xib
index 8986d3c..5b8cc25 100644
--- a/chrome/app/nibs/BookmarkEditor.xib
+++ b/chrome/app/nibs/BookmarkEditor.xib
@@ -1,28 +1,19 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<archive type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="7.03">
+<archive type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="7.02">
 	<data>
 		<int key="IBDocument.SystemTarget">1050</int>
 		<string key="IBDocument.SystemVersion">9L30</string>
-		<string key="IBDocument.InterfaceBuilderVersion">677</string>
+		<string key="IBDocument.InterfaceBuilderVersion">670</string>
 		<string key="IBDocument.AppKitVersion">949.54</string>
 		<string key="IBDocument.HIToolboxVersion">353.00</string>
 		<object class="NSMutableArray" key="IBDocument.EditedObjectIDs">
 			<bool key="EncodedWithXMLCoder">YES</bool>
-			<integer value="44"/>
+			<integer value="2"/>
 		</object>
 		<object class="NSArray" key="IBDocument.PluginDependencies">
 			<bool key="EncodedWithXMLCoder">YES</bool>
 			<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 		</object>
-		<object class="NSMutableDictionary" key="IBDocument.Metadata">
-			<bool key="EncodedWithXMLCoder">YES</bool>
-			<object class="NSArray" key="dict.sortedKeys">
-				<bool key="EncodedWithXMLCoder">YES</bool>
-			</object>
-			<object class="NSMutableArray" key="dict.values">
-				<bool key="EncodedWithXMLCoder">YES</bool>
-			</object>
-		</object>
 		<object class="NSMutableArray" key="IBDocument.RootObjects" id="1000">
 			<bool key="EncodedWithXMLCoder">YES</bool>
 			<object class="NSCustomObject" id="1001">
@@ -71,7 +62,7 @@
 							<int key="NSColumnResizingType">1</int>
 							<double key="NSPreferedColumnWidth">1.000000e+02</double>
 							<bool key="NSAllowsTypeSelect">YES</bool>
-							<int key="NSBrFlags">469843968</int>
+							<int key="NSBrFlags">1544765440</int>
 						</object>
 						<object class="NSCustomView" id="138844528">
 							<reference key="NSNextResponder" ref="1006"/>
@@ -432,6 +423,14 @@
 					</object>
 					<int key="connectionID">43</int>
 				</object>
+				<object class="IBConnectionRecord">
+					<object class="IBOutletConnection" key="connection">
+						<string key="label">delegate</string>
+						<reference key="source" ref="723395462"/>
+						<reference key="destination" ref="1001"/>
+					</object>
+					<int key="connectionID">53</int>
+				</object>
 			</object>
 			<object class="IBMutableOrderedSet" key="objectRecords">
 				<object class="NSArray" key="orderedObjects">
@@ -681,15 +680,15 @@
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
-					<string>{{555, 472}, {480, 270}}</string>
+					<string>{{626, 873}, {480, 270}}</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
-					<boolean value="NO" id="6"/>
-					<string>{{555, 472}, {480, 270}}</string>
+					<integer value="0" id="6"/>
+					<string>{{626, 873}, {480, 270}}</string>
 					<reference ref="6"/>
 					<string>{196, 240}</string>
 					<string>{{357, 418}, {480, 270}}</string>
 					<reference ref="6"/>
-					<boolean value="YES"/>
+					<integer value="1"/>
 					<string>{331, 270}</string>
 					<string>{331, 270}</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
@@ -735,7 +734,7 @@
 				</object>
 			</object>
 			<nil key="sourceID"/>
-			<int key="maxID">44</int>
+			<int key="maxID">54</int>
 		</object>
 		<object class="IBClassDescriber" key="IBDocument.Classes">
 			<object class="NSMutableArray" key="referencedPartialClassDescriptions">
@@ -870,6 +869,13 @@
 						<string key="minorKey">browser/cocoa/tab_strip_model_observer_bridge.h</string>
 					</object>
 				</object>
+				<object class="IBPartialClassDescription">
+					<string key="className">NSWindow</string>
+					<object class="IBClassDescriptionSource" key="sourceIdentifier">
+						<string key="majorKey">IBProjectSource</string>
+						<string key="minorKey">browser/cocoa/nswindow_local_state.h</string>
+					</object>
+				</object>
 			</object>
 		</object>
 		<int key="IBDocument.localizationMode">0</int>
diff --git a/chrome/browser/cocoa/bookmark_editor_controller.h b/chrome/browser/cocoa/bookmark_editor_controller.h
index 89bdac3..a0b8a95 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller.h
+++ b/chrome/browser/cocoa/bookmark_editor_controller.h
@@ -53,6 +53,7 @@
 @property (assign) NSString* displayName;
 @property (assign) NSString* displayURL;
 @property (readonly) BOOL okButtonEnabled;
+- (void)selectTestNodeInBrowser:(const BookmarkNode*)node;
 @end
 
 
diff --git a/chrome/browser/cocoa/bookmark_editor_controller.mm b/chrome/browser/cocoa/bookmark_editor_controller.mm
index 3c45556..5ded618 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller.mm
@@ -10,6 +10,23 @@
 #include "chrome/browser/profile.h"
 #import "chrome/browser/cocoa/bookmark_editor_controller.h"
 
+@interface BookmarkEditorController (Private)
+
+// Grab the url from the text field and convert.
+- (GURL)GURLFromUrlField;
+
+// Determine and returns the rightmost selected/highlighted element (node)
+// in the bookmark tree view if the tree view is showing, otherwise returns
+// the original parentNode_.  If the tree view is showing but nothing is
+// selected then return the root node.
+- (const BookmarkNode*)selectedParentNode;
+
+// Select/highlight the given node within the browser tree view.  If the
+// node is not found then the selection will not be changed.
+- (void)selectNodeInBrowser:(const BookmarkNode*)node;
+
+@end
+
 // static; implemented for each platform.
 void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
                           Profile* profile,
@@ -32,6 +49,39 @@ void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
   [controller runAsModalSheet];
 }
 
+#pragma mark Bookmark TreeNode Helpers
+
+namespace {
+
+// Find the index'th folder child of a parent, ignoring bookmarks (leafs).
+const BookmarkNode* GetFolderChildForParent(const BookmarkNode* parent_node,
+                                            NSInteger folder_index) {
+  const BookmarkNode* child_node = nil;
+  int i = 0;
+  int child_count = parent_node->GetChildCount();
+  do {
+    child_node = parent_node->GetChild(i);
+    if (child_node->type() != BookmarkNode::URL)
+      --folder_index;
+    ++i;
+  } while (folder_index >= 0 && i < child_count);
+  return child_node;
+}
+
+// Determine the index of a child within its parent ignoring
+// bookmarks (leafs).
+int IndexOfFolderChild(const BookmarkNode* child_node) {
+  const BookmarkNode* node_parent = child_node->GetParent();
+  int child_index = node_parent->IndexOfChild(child_node);
+  for (int i = child_index - 1; i >= 0; --i) {
+    const BookmarkNode* sibling = node_parent->GetChild(i);
+    if (sibling->type() == BookmarkNode::URL)
+      --child_index;
+  }
+  return child_index;
+}
+
+} // namespace
 
 @implementation BookmarkEditorController
 
@@ -77,10 +127,7 @@ void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
   [urlField_ setDelegate:self];
   [self controlTextDidChange:nil];
 
-  if (configuration_ == BookmarkEditor::SHOW_TREE) {
-    // build the tree et al
-    NOTIMPLEMENTED();
-  } else {
+  if (configuration_ != BookmarkEditor::SHOW_TREE) {
     // Remember the NSBrowser's height; we will shrink our frame by that
     // much.
     NSRect frame = [[self window] frame];
@@ -95,6 +142,35 @@ void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
   }
 }
 
+- (void)selectNodeInBrowser:(const BookmarkNode*)node {
+  DCHECK(configuration_ == BookmarkEditor::SHOW_TREE);
+  // Find and select the node in the browser by walking
+  // back to the root node, then selecting forward.
+  std::deque<NSInteger> rowsToSelect;
+  const BookmarkNode* nodeParent = node->GetParent();
+  // There should always be a parent node.
+  DCHECK(nodeParent);
+  while (nodeParent) {
+    int nodeRow = IndexOfFolderChild(node);
+    rowsToSelect.push_front(nodeRow);
+    node = nodeParent;
+    nodeParent = nodeParent->GetParent();
+  }
+  for (std::deque<NSInteger>::size_type column = 0;
+       column < rowsToSelect.size();
+       ++column) {
+    [browser_ selectRow:rowsToSelect[column] inColumn:column];
+  }
+  [self controlTextDidChange:nil];
+}
+
+- (void)windowDidLoad {
+  if (configuration_ == BookmarkEditor::SHOW_TREE) {
+    // Find and select the |parent| bookmark node.
+    [self selectNodeInBrowser:parentNode_];
+  }
+}
+
 /* TODO(jrg):
 // Implementing this informal protocol allows us to open the sheet
 // somewhere other than at the top of the window.  NOTE: this means
@@ -126,7 +202,7 @@ void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
 
 // If possible, return a valid GURL from the URL text field.
 - (GURL)GURLFromUrlField {
-  NSString *url = [urlField_ stringValue];
+  NSString* url = [urlField_ stringValue];
   GURL newURL = GURL([url UTF8String]);
   if (!newURL.is_valid()) {
     // Mimic observed friendliness from Windows
@@ -135,59 +211,73 @@ void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
   return newURL;
 }
 
-// When the URL changes we may enable or disable the OK button.
+- (const BookmarkNode*)selectedParentNode {
+  BookmarkModel* model = profile_->GetBookmarkModel();
+  const BookmarkNode* selectedParentNode = NULL;
+  // Determine a new parent node only if the browser is showing.
+  if (configuration_ == BookmarkEditor::SHOW_TREE) {
+    selectedParentNode = model->root_node();
+    NSInteger column = 0;
+    NSInteger selectedRow = [browser_ selectedRowInColumn:column];
+    while (selectedRow >= 0) {
+      selectedParentNode = GetFolderChildForParent(selectedParentNode,
+                                                   selectedRow);
+      ++column;
+      selectedRow = [browser_ selectedRowInColumn:column];
+    }
+  } else {
+    // If the tree is not showing then we use the original parent.
+    selectedParentNode = parentNode_;
+  }
+  return selectedParentNode;
+}
+
+// Enable the OK button if there is a bookmark name and there is a valid URL.
 // We set ourselves as the delegate of urlField_ so this gets called.
 // (Yes, setting ourself as a delegate automatically registers us for
 // the notification.)
-- (void)controlTextDidChange:(NSNotification *)aNotification {
-  GURL newURL = [self GURLFromUrlField];
+- (void)controlTextDidChange:(NSNotification*)aNotification {
+  // Name must not be empty, but it can be whitespace.
   NSString* name = [nameField_ stringValue];
-
-  // if empty or only whitespace, name is not valid.
-  bool name_valid = true;
-  if (([name length] == 0) ||
-      ([[name stringByTrimmingCharactersInSet:[NSCharacterSet
-              whitespaceAndNewlineCharacterSet]] length] == 0)) {
-    name_valid = false;
-  }
-
-  if (node_ && (node_->is_folder() || newURL.is_valid()) && name_valid) {
-    [okButton_ setEnabled:YES];
-  } else {
-    [okButton_ setEnabled:NO];
-  }
+  GURL newURL = [self GURLFromUrlField];
+  [okButton_ setEnabled:([name length] != 0 && newURL.is_valid()) ? YES : NO];
 }
 
-// TODO(jrg): Once the tree is available edits may be more extensive
-// than just name/url.
+// The ok: action is connected to the OK button in the Edit Bookmark window
+// of the BookmarkEditor.xib.  The the bookmark's name and URL are assumed
+// to be valid (otherwise the OK button should not be enabled).  If the
+// bookmark previously existed then it is removed from its old folder.
+// The bookmark is then added to its new folder.  If the folder has not
+// changed then the bookmark stays in its original position (index) otherwise
+// it is added to the end of the new folder.
 - (IBAction)ok:(id)sender {
-  NSString *name = [nameField_ stringValue];
-  NSString *url = [urlField_ stringValue];
-
-  if ((![name isEqual:initialName_]) ||
-      (![url isEqual:initialUrl_])) {
-    std::wstring newTitle = base::SysNSStringToWide(name);
-    GURL newURL = [self GURLFromUrlField];
-    if (!newURL.is_valid()) {
-      // Shouldn't be reached -- OK button disabled if not valid!
-      NOTREACHED();
-      return;
-    }
-    int index = 0;
-    BookmarkModel* model = profile_->GetBookmarkModel();
-    if (node_) {
-      index = parentNode_->IndexOfChild(node_);
-      model->Remove(parentNode_, index);
-    } else {
-      index = parentNode_->GetChildCount();
-    }
-    const BookmarkNode* node = model->AddURL(parentNode_, index,
-                                             newTitle, newURL);
-    // Honor handler semantics: callback on node creation
-    if (handler_.get())
-      handler_->NodeCreated(node);
+  NSString* name = [nameField_ stringValue];
+  std::wstring newTitle = base::SysNSStringToWide(name);
+  GURL newURL = [self GURLFromUrlField];
+  if (!newURL.is_valid()) {
+    // Shouldn't be reached -- OK button disabled if not valid!
+    NOTREACHED();
+    return;
   }
 
+  // Determine where the new/replacement bookmark is to go.
+  const BookmarkNode* newParentNode = [self selectedParentNode];
+  BookmarkModel* model = profile_->GetBookmarkModel();
+  int newIndex = newParentNode->GetChildCount();
+  if (node_ && parentNode_) {
+    // Replace the old bookmark with the updated bookmark.
+    int oldIndex = parentNode_->IndexOfChild(node_);
+    if (oldIndex >= 0)
+      model->Remove(parentNode_, oldIndex);
+    if (parentNode_ == newParentNode)
+      newIndex = oldIndex;
+  }
+  // Add bookmark as new node at the end of the newly selected folder.
+  const BookmarkNode* node = model->AddURL(newParentNode, newIndex,
+                                           newTitle, newURL);
+  // Honor handler semantics: callback on node creation.
+  if (handler_.get())
+    handler_->NodeCreated(node);
   [NSApp endSheet:[self window]];
 }
 
@@ -207,6 +297,7 @@ void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
   [self autorelease];
 }
 
+#pragma mark For Unit Test Use Only
 
 - (NSString*)displayName {
   return [nameField_ stringValue];
@@ -230,5 +321,57 @@ void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
   return [okButton_ isEnabled];
 }
 
+- (void)selectTestNodeInBrowser:(const BookmarkNode*)node {
+  [self selectNodeInBrowser:node];
+}
+
+#pragma mark NSBrowser delegate methods
+
+// Given a column number, determine the parent bookmark folder node for the
+// bookmarks being shown in that column.  This is done by scanning forward
+// from column zero and following the selected row for each column up
+// to the parent of the desired column.
+- (const BookmarkNode*)parentNodeForColumn:(NSInteger)column {
+  DCHECK(column >= 0);
+  BookmarkModel* model = profile_->GetBookmarkModel();
+  const BookmarkNode* node = model->root_node();
+  for (NSInteger i = 0; i < column; ++i) {
+    NSInteger selectedRowInColumn = [browser_ selectedRowInColumn:i];
+    node = node->GetChild(selectedRowInColumn);
+  }
+  return node;
+}
+
+// This implementation returns the number of folders contained in the parent
+// folder node for this column.
+// TOTO(mrossetti): Decide if bookmark (i.e. non-folder) nodes should be
+// shown, perhaps in gray.
+- (NSInteger)browser:(NSBrowser*)sender numberOfRowsInColumn:(NSInteger)col {
+  NSInteger rowCount = 0;
+  const BookmarkNode* parentNode = [self parentNodeForColumn:col];
+  if (parentNode) {
+    int childCount = parentNode->GetChildCount();
+    for (int i = 0; i < childCount; ++i) {
+      const BookmarkNode* childNode = parentNode->GetChild(i);
+      if (childNode->type() != BookmarkNode::URL)
+        ++rowCount;
+    }
+  }
+  return rowCount;
+}
+
+- (void)browser:(NSBrowser*)sender
+    willDisplayCell:(NSBrowserCell*)cell
+              atRow:(NSInteger)row
+            column:(NSInteger)column {
+  DCHECK(row >= 0);  // Trust AppKit, but verify.
+  DCHECK(column >= 0);
+  const BookmarkNode* parentNode = [self parentNodeForColumn:column];
+  const BookmarkNode* childNode = GetFolderChildForParent(parentNode, row);
+  DCHECK(childNode);
+  [cell setTitle:base::SysWideToNSString(childNode->GetTitle())];
+  [cell setLeaf:childNode->GetChildCount() == 0];
+}
+
 @end  // BookmarkEditorController
 
diff --git a/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm b/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
index d338b4b..79a3142 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
@@ -133,10 +133,188 @@ TEST_F(BookmarkEditorControllerTest, EditAndConfirmOKButton) {
   [default_controller_ setDisplayName:@""];
   EXPECT_FALSE([default_controller_ okButtonEnabled]);
   [default_controller_ setDisplayName:@"                   "];
-  EXPECT_FALSE([default_controller_ okButtonEnabled]);
+  EXPECT_TRUE([default_controller_ okButtonEnabled]);
   // Then little mix of both.
   [default_controller_ setDisplayName:@"name"];
   EXPECT_TRUE([default_controller_ okButtonEnabled]);
   [default_controller_ setDisplayURL:@""];
   EXPECT_FALSE([default_controller_ okButtonEnabled]);
 }
+
+class BookmarkEditorControllerTreeTest : public PlatformTest {
+ public:
+  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
+  BrowserTestHelper helper_;
+  scoped_nsobject<BookmarkEditorController> default_controller_;
+  const BookmarkNode* group_a_;
+  const BookmarkNode* group_b_;
+  const BookmarkNode* group_bb_;
+  const BookmarkNode* group_c_;
+  const BookmarkNode* bookmark_bb_3_;
+
+  BookmarkEditorControllerTreeTest() {
+    // Set up a small bookmark hierarchy, which will look as follows:
+    //    a      b      c    d
+    //     a-0    b-0    c-0
+    //     a-1     bb-0  c-1
+    //     a-2     bb-1  c-2
+    //             bb-2
+    //             bb-3
+    //             bb-4
+    //            b-1
+    //            b-2
+    BookmarkModel& model(*(helper_.profile()->GetBookmarkModel()));
+    const BookmarkNode* root = model.GetBookmarkBarNode();
+    group_a_ = model.AddGroup(root, 0, L"a");
+    model.AddURL(group_a_, 0, L"a-0", GURL("http://a-0.com"));
+    model.AddURL(group_a_, 1, L"a-1", GURL("http://a-1.com"));
+    model.AddURL(group_a_, 2, L"a-2", GURL("http://a-2.com"));
+
+    group_b_ = model.AddGroup(root, 1, L"b");
+    model.AddURL(group_b_, 0, L"b-0", GURL("http://b-0.com"));
+    group_bb_ = model.AddGroup(group_b_, 1, L"bb");
+    model.AddURL(group_bb_, 0, L"bb-0", GURL("http://bb-0.com"));
+    model.AddURL(group_bb_, 1, L"bb-1", GURL("http://bb-1.com"));
+    model.AddURL(group_bb_, 2, L"bb-2", GURL("http://bb-2.com"));
+    bookmark_bb_3_ =
+      model.AddURL(group_bb_, 3, L"bb-3", GURL("http://bb-3.com"));
+    model.AddURL(group_bb_, 4, L"bb-4", GURL("http://bb-4.com"));
+    model.AddURL(group_b_, 2, L"b-2", GURL("http://b-2.com"));
+    model.AddURL(group_b_, 3, L"b-2", GURL("http://b-3.com"));
+
+    group_c_ = model.AddGroup(root, 2, L"c");
+    model.AddURL(group_c_, 0, L"c-0", GURL("http://c-0.com"));
+    model.AddURL(group_c_, 1, L"c-1", GURL("http://c-1.com"));
+    model.AddURL(group_c_, 2, L"c-2", GURL("http://c-2.com"));
+    model.AddURL(group_c_, 3, L"c-3", GURL("http://c-3.com"));
+
+    model.AddURL(root, 3, L"d", GURL("http://d-0.com"));
+
+    default_controller_.reset([[BookmarkEditorController alloc]
+                               initWithParentWindow:cocoa_helper_.window()
+                               profile:helper_.profile()
+                               parent:group_bb_
+                               node:bookmark_bb_3_
+                               configuration:BookmarkEditor::SHOW_TREE
+                               handler:nil]);
+    [default_controller_ window];  // Forces a nib load
+  }
+};
+
+TEST_F(BookmarkEditorControllerTreeTest, VerifyBookmarkTestModel) {
+  BookmarkModel& model(*(helper_.profile()->GetBookmarkModel()));
+  const BookmarkNode& root(*model.GetBookmarkBarNode());
+  EXPECT_EQ(4, root.GetChildCount());
+  const BookmarkNode* child = root.GetChild(0);
+  EXPECT_EQ(3, child->GetChildCount());
+  const BookmarkNode* subchild = child->GetChild(0);
+  EXPECT_EQ(0, subchild->GetChildCount());
+  subchild = child->GetChild(1);
+  EXPECT_EQ(0, subchild->GetChildCount());
+  subchild = child->GetChild(2);
+  EXPECT_EQ(0, subchild->GetChildCount());
+
+  child = root.GetChild(1);
+  EXPECT_EQ(4, child->GetChildCount());
+  subchild = child->GetChild(0);
+  EXPECT_EQ(0, subchild->GetChildCount());
+  subchild = child->GetChild(1);
+  EXPECT_EQ(5, subchild->GetChildCount());
+  const BookmarkNode* subsubchild = subchild->GetChild(0);
+  EXPECT_EQ(0, subsubchild->GetChildCount());
+  subsubchild = subchild->GetChild(1);
+  EXPECT_EQ(0, subsubchild->GetChildCount());
+  subsubchild = subchild->GetChild(2);
+  EXPECT_EQ(0, subsubchild->GetChildCount());
+  subsubchild = subchild->GetChild(3);
+  EXPECT_EQ(0, subsubchild->GetChildCount());
+  subsubchild = subchild->GetChild(4);
+  EXPECT_EQ(0, subsubchild->GetChildCount());
+  subchild = child->GetChild(2);
+  EXPECT_EQ(0, subchild->GetChildCount());
+  subchild = child->GetChild(3);
+  EXPECT_EQ(0, subchild->GetChildCount());
+
+  child = root.GetChild(2);
+  EXPECT_EQ(4, child->GetChildCount());
+  subchild = child->GetChild(0);
+  EXPECT_EQ(0, subchild->GetChildCount());
+  subchild = child->GetChild(1);
+  EXPECT_EQ(0, subchild->GetChildCount());
+  subchild = child->GetChild(2);
+  EXPECT_EQ(0, subchild->GetChildCount());
+  subchild = child->GetChild(3);
+  EXPECT_EQ(0, subchild->GetChildCount());
+
+  child = root.GetChild(3);
+  EXPECT_EQ(0, child->GetChildCount());
+}
+
+TEST_F(BookmarkEditorControllerTreeTest, RenameBookmarkInPlace) {
+  const BookmarkNode* oldParent = bookmark_bb_3_->GetParent();
+  [default_controller_ setDisplayName:@"NEW NAME"];
+  [default_controller_ ok:nil];
+  const BookmarkNode* newParent = bookmark_bb_3_->GetParent();
+  std::cout << "oldParent: " << oldParent->GetTitle() << "\r";
+  std::cout << "newParent: " << newParent->GetTitle() << "\r";
+  ASSERT_EQ(newParent, oldParent);
+  int childIndex = newParent->IndexOfChild(bookmark_bb_3_);
+  ASSERT_EQ(3, childIndex);
+}
+
+TEST_F(BookmarkEditorControllerTreeTest, ChangeBookmarkURLInPlace) {
+  const BookmarkNode* oldParent = bookmark_bb_3_->GetParent();
+  [default_controller_ setDisplayURL:@"http://NEWURL.com"];
+  [default_controller_ ok:nil];
+  const BookmarkNode* newParent = bookmark_bb_3_->GetParent();
+  ASSERT_EQ(newParent, oldParent);
+  int childIndex = newParent->IndexOfChild(bookmark_bb_3_);
+  ASSERT_EQ(3, childIndex);
+}
+
+TEST_F(BookmarkEditorControllerTreeTest, ChangeBookmarkGroup) {
+  [default_controller_ selectTestNodeInBrowser:group_c_];
+  [default_controller_ ok:nil];
+  const BookmarkNode* parent = bookmark_bb_3_->GetParent();
+  ASSERT_EQ(parent, group_c_);
+  int childIndex = parent->IndexOfChild(bookmark_bb_3_);
+  ASSERT_EQ(4, childIndex);
+}
+
+TEST_F(BookmarkEditorControllerTreeTest, ChangeNameAndBookmarkGroup) {
+  [default_controller_ setDisplayName:@"NEW NAME"];
+  [default_controller_ selectTestNodeInBrowser:group_c_];
+  [default_controller_ ok:nil];
+  const BookmarkNode* parent = bookmark_bb_3_->GetParent();
+  ASSERT_EQ(parent, group_c_);
+  int childIndex = parent->IndexOfChild(bookmark_bb_3_);
+  ASSERT_EQ(4, childIndex);
+  EXPECT_EQ(bookmark_bb_3_->GetTitle(), L"NEW NAME");
+}
+
+class BookmarkEditorControllerTreeNoNodeTest :
+    public BookmarkEditorControllerTreeTest {
+ public:
+  BookmarkEditorControllerTreeNoNodeTest() {
+    // Reset the controller so that we have no |node|.
+    default_controller_.reset([[BookmarkEditorController alloc]
+                               initWithParentWindow:cocoa_helper_.window()
+                               profile:helper_.profile()
+                               parent:group_bb_
+                               node:nil
+                               configuration:BookmarkEditor::SHOW_TREE
+                               handler:nil]);
+    [default_controller_ window];  // Forces a nib load
+  }
+};
+
+TEST_F(BookmarkEditorControllerTreeNoNodeTest, NewBookmarkNoNode) {
+  [default_controller_ setDisplayName:@"NEW BOOKMARK"];
+  [default_controller_ setDisplayURL:@"http://NEWURL.com"];
+  [default_controller_ ok:nil];
+  const BookmarkNode* new_node = group_bb_->GetChild(5);
+  ASSERT_EQ(0, new_node->GetChildCount());
+  EXPECT_EQ(new_node->GetTitle(), L"NEW BOOKMARK");
+  EXPECT_EQ(new_node->GetURL(), GURL("http://NEWURL.com"));
+}
+