New folders in the bookmarks bar are created at the end of it

BUG=http://crbug.com/9340

TEST=Empty bookmark bar.  Context menu --> Add Folder named "one".  Do it.
   (Note you can only click on left side; I will file bug about that).
Repeat on bar, add folder named "three" --> adds to end.
Repeat with menu OVER "one" --> adds folder between one and three.
Add a bookmark (however; e.g. with *) in a folder.  Call it SUB.
Right click on SUB --> Add folder ENDO.  Should put after SUB.
Right click on SUB --> Add folder.  Should put after SUB but before ENDO.


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

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

4 files changed, 106 insertions, 19 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index 87b1d7e..a913148 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -1353,13 +1353,28 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
                        nil);
 }
 
-// Might be from the context menu over the bar OR over a button.
+// Might be called from the context menu over the bar OR over a
+// button.  If called from a button, that button becomes a sibling of
+// the new node.  If called from the bar, add to the end of the bar.
 - (IBAction)addFolder:(id)sender {
+  const BookmarkNode* senderNode = [self nodeFromMenuItem:sender];
+  const BookmarkNode* parent = NULL;
+  int newIndex = 0;
+  // If triggered from a folder or mark, that is our sibling.
+  // If triggered from the bar, add to the end.
+  if (senderNode->type() == BookmarkNode::BOOKMARK_BAR) {
+    parent = senderNode;
+    newIndex = parent->GetChildCount();
+  } else {
+    parent = senderNode->GetParent();
+    newIndex = parent->IndexOfChild(senderNode) + 1;
+  }
   BookmarkNameFolderController* controller =
     [[BookmarkNameFolderController alloc]
       initWithParentWindow:[[self view] window]
                    profile:browser_->profile()
-                      node:NULL];
+                    parent:parent
+                  newIndex:newIndex];
   [controller runAsModalSheet];
 }
 
diff --git a/chrome/browser/cocoa/bookmark_name_folder_controller.h b/chrome/browser/cocoa/bookmark_name_folder_controller.h
index 4a0c75f..ba4c3f4 100644
--- a/chrome/browser/cocoa/bookmark_name_folder_controller.h
+++ b/chrome/browser/cocoa/bookmark_name_folder_controller.h
@@ -23,18 +23,32 @@ class BookmarkModelObserverForCocoa;
 
   NSWindow* parentWindow_;  // weak
   Profile* profile_;  // weak
-  // Weak; owned by the model.  Can be NULL (see below).
+
+  // Weak; owned by the model.  Can be NULL (see below).  Either node_
+  // is non-NULL (renaming a folder), or parent_ is non-NULL (adding a
+  // new one).
   const BookmarkNode* node_;
+  const BookmarkNode* parent_;
+  int newIndex_;
+
   scoped_nsobject<NSString> initialName_;
 
   // Ping me when things change out from under us.
   scoped_ptr<BookmarkModelObserverForCocoa> observer_;
 }
-// If |node| is NULL, this is an "add folder" request.
-// Else it is a "rename an existing folder" request.
+
+// Use the 1st initializer for a "rename existing folder" request.
+//
+// Use the 2nd initializer for an "add folder" request.  If creating a
+// new folder |parent| and |newIndex| specify where to put the new
+// node.
 - (id)initWithParentWindow:(NSWindow*)window
                    profile:(Profile*)profile
                       node:(const BookmarkNode*)node;
+- (id)initWithParentWindow:(NSWindow*)window
+                   profile:(Profile*)profile
+                    parent:(const BookmarkNode*)parent
+                  newIndex:(int)newIndex;
 - (void)runAsModalSheet;
 - (IBAction)cancel:(id)sender;
 - (IBAction)ok:(id)sender;
diff --git a/chrome/browser/cocoa/bookmark_name_folder_controller.mm b/chrome/browser/cocoa/bookmark_name_folder_controller.mm
index d34304d..ef6cdc1 100644
--- a/chrome/browser/cocoa/bookmark_name_folder_controller.mm
+++ b/chrome/browser/cocoa/bookmark_name_folder_controller.mm
@@ -13,9 +13,12 @@
 
 @implementation BookmarkNameFolderController
 
+// Common initializer (private).
 - (id)initWithParentWindow:(NSWindow*)window
                    profile:(Profile*)profile
-                      node:(const BookmarkNode*)node {
+                      node:(const BookmarkNode*)node
+                    parent:(const BookmarkNode*)parent
+                  newIndex:(int)newIndex {
   NSString* nibpath = [mac_util::MainAppBundle()
                         pathForResource:@"BookmarkNameFolder"
                         ofType:@"nib"];
@@ -23,6 +26,11 @@
     parentWindow_ = window;
     profile_ = profile;
     node_ = node;
+    parent_ = parent;
+    newIndex_ = newIndex;
+    if (parent) {
+      DCHECK_LE(newIndex, parent->GetChildCount());
+    }
     if (node_) {
       initialName_.reset([base::SysWideToNSString(node_->GetTitle()) retain]);
     } else {
@@ -34,6 +42,29 @@
   return self;
 }
 
+- (id)initWithParentWindow:(NSWindow*)window
+                   profile:(Profile*)profile
+                      node:(const BookmarkNode*)node {
+  DCHECK(node);
+  return [self initWithParentWindow:window
+                            profile:profile
+                               node:node
+                             parent:nil
+                           newIndex:0];
+}
+
+- (id)initWithParentWindow:(NSWindow*)window
+                   profile:(Profile*)profile
+                    parent:(const BookmarkNode*)parent
+                  newIndex:(int)newIndex {
+  DCHECK(parent);
+  return [self initWithParentWindow:window
+                            profile:profile
+                               node:nil
+                             parent:parent
+                           newIndex:newIndex];
+}
+
 - (void)awakeFromNib {
   [nameField_ setStringValue:initialName_.get()];
 }
@@ -63,13 +94,8 @@
   if (node_) {
     model->SetTitle(node_, base::SysNSStringToWide(name));
   } else {
-    // TODO(jrg): check sender to accomodate creating a folder while
-    // NOT over the bar (e.g. when over an expanded folder itself).
-    // Need to wait until I add folders before I can do that
-    // properly.
-    // For now only add the folder at the top level.
-    model->AddGroup(model->GetBookmarkBarNode(),
-                    model->GetBookmarkBarNode()->GetChildCount(),
+    model->AddGroup(parent_,
+                    newIndex_,
                     base::SysNSStringToWide(name));
   }
   [NSApp endSheet:[self window]];
diff --git a/chrome/browser/cocoa/bookmark_name_folder_controller_unittest.mm b/chrome/browser/cocoa/bookmark_name_folder_controller_unittest.mm
index 6df1645..5b3e3b5 100644
--- a/chrome/browser/cocoa/bookmark_name_folder_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_name_folder_controller_unittest.mm
@@ -17,17 +17,18 @@ class BookmarkNameFolderControllerTest : public CocoaTest {
 };
 
 
+// Simple add of a node (at the end).
 TEST_F(BookmarkNameFolderControllerTest, AddNew) {
   BookmarkModel* model = helper_.profile()->GetBookmarkModel();
   const BookmarkNode* parent = model->GetBookmarkBarNode();
-  const BookmarkNode* node = NULL;
   EXPECT_EQ(0, parent->GetChildCount());
 
   scoped_nsobject<BookmarkNameFolderController>
     controller([[BookmarkNameFolderController alloc]
                  initWithParentWindow:test_window()
                               profile:helper_.profile()
-                                 node:node]);
+                               parent:parent
+                             newIndex:0]);
   [controller window];  // force nib load
 
   // Do nothing.
@@ -46,6 +47,33 @@ TEST_F(BookmarkNameFolderControllerTest, AddNew) {
   EXPECT_EQ(L"Bozo", parent->GetChild(0)->GetTitle());
 }
 
+// Add new but specify a sibling.
+TEST_F(BookmarkNameFolderControllerTest, AddNewWithSibling) {
+  BookmarkModel* model = helper_.profile()->GetBookmarkModel();
+  const BookmarkNode* parent = model->GetBookmarkBarNode();
+
+  // Add 2 nodes.  We will place the new folder in the middle of these.
+  model->AddURL(parent, 0, L"title 1", GURL("http://www.google.com"));
+  model->AddURL(parent, 1, L"title 3", GURL("http://www.google.com"));
+  EXPECT_EQ(2, parent->GetChildCount());
+
+  scoped_nsobject<BookmarkNameFolderController>
+    controller([[BookmarkNameFolderController alloc]
+                 initWithParentWindow:test_window()
+                              profile:helper_.profile()
+                               parent:parent
+                             newIndex:1]);
+  [controller window];  // force nib load
+
+  // Add a new folder.
+  [controller setFolderName:@"middle"];
+  [controller ok:nil];
+
+  // Confirm we now have 3, and that the new one is in the middle.
+  EXPECT_EQ(3, parent->GetChildCount());
+  EXPECT_TRUE(parent->GetChild(1)->is_folder());
+  EXPECT_EQ(L"middle", parent->GetChild(1)->GetTitle());
+}
 
 // Make sure we are allowed to create a folder named "New Folder".
 TEST_F(BookmarkNameFolderControllerTest, AddNewDefaultName) {
@@ -57,7 +85,9 @@ TEST_F(BookmarkNameFolderControllerTest, AddNewDefaultName) {
     controller([[BookmarkNameFolderController alloc]
                  initWithParentWindow:test_window()
                               profile:helper_.profile()
-                                 node:NULL]);
+                               parent:parent
+                             newIndex:0]);
+
   [controller window];  // force nib load
 
   // Click OK without changing the name
@@ -76,7 +106,8 @@ TEST_F(BookmarkNameFolderControllerTest, AddNewBlankName) {
   controller([[BookmarkNameFolderController alloc]
               initWithParentWindow:test_window()
                            profile:helper_.profile()
-                              node:NULL]);
+                            parent:parent
+                          newIndex:0]);
   [controller window];  // force nib load
 
   // Change the name to blank, click OK.
@@ -113,14 +144,14 @@ TEST_F(BookmarkNameFolderControllerTest, Rename) {
 TEST_F(BookmarkNameFolderControllerTest, EditAndConfirmOKButton) {
   BookmarkModel* model = helper_.profile()->GetBookmarkModel();
   const BookmarkNode* parent = model->GetBookmarkBarNode();
-  const BookmarkNode* node = NULL;
   EXPECT_EQ(0, parent->GetChildCount());
 
   scoped_nsobject<BookmarkNameFolderController>
     controller([[BookmarkNameFolderController alloc]
                  initWithParentWindow:test_window()
                               profile:helper_.profile()
-                                 node:node]);
+                               parent:parent
+                             newIndex:0]);
   [controller window];  // force nib load
 
   // We start enabled since the default "New Folder" is added for us.
@@ -134,3 +165,4 @@ TEST_F(BookmarkNameFolderControllerTest, EditAndConfirmOKButton) {
   [controller setFolderName:@""];
   EXPECT_TRUE([[controller okButton] isEnabled]);
 }
+