Implement Bookmark All Tabs... Added an abstraction of BookmarkEditorController called BookmarkEditorBaseController, from which BookmarkEditorController and the new BookmarkAllTabsController derive. The bookmark/folder name, URL and OK button now use bindings/KVO. Added BookmarkAllTabs.xib which is nearly identical to BookmarkEditor.xib. Changed unit test since an empty bookmark name is now acceptable. Added unit test for the Bookmark All Tabs...

BookmarkEditor.xib changes: Removed the bookmark name and url as outlets and instead bound their values to controller's displayName and displayURL respectively.  Bound the OK button enabling to the controller's okEnabled.

BUG=25099
TEST=Open two or more tabs.  Bring up a contextual menu by control-clicking on one of the tabs.  Choose the Bookmark All Tabs... menu item.  A dialog will be presented in which you can type a folder name and select the parent folder in which the new folder will be placed.  By default, the most recently touched folder will be selected as the parent.  Click OK.  Now check to parent folder to see that the newly created folder has been added and then check within that folder to see that the open tabs have been added.

Perform the above operation again only this time choose the Bookmark All Tabs... menu item found in the Bookmarks menu.

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

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

9 files changed, 911 insertions, 428 deletions

diff --git a/chrome/browser/cocoa/bookmark_all_tabs_controller.h b/chrome/browser/cocoa/bookmark_all_tabs_controller.h
new file mode 100644
index 0000000..e861c50
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_all_tabs_controller.h
@@ -0,0 +1,44 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_COCOA_BOOKMARK_ALL_TABS_CONTROLLER_H_
+#define CHROME_BROWSER_COCOA_BOOKMARK_ALL_TABS_CONTROLLER_H_
+
+#include <utility>
+#include <vector>
+
+#import "chrome/browser/cocoa/bookmark_editor_base_controller.h"
+
+// A list of pairs containing the name and URL associated with each
+// currently active tab in the active browser window.
+typedef std::pair<std::wstring, GURL> ActiveTabNameURLPair;
+typedef std::vector<ActiveTabNameURLPair> ActiveTabsNameURLPairVector;
+
+// A controller for the bookmark editor, opened with Edit... from the
+// context menu of a bookmark button.
+@interface BookmarkAllTabsController : BookmarkEditorBaseController {
+ @private
+  ActiveTabsNameURLPairVector activeTabPairsVector_;
+}
+
+- (id)initWithParentWindow:(NSWindow*)parentWindow
+                   profile:(Profile*)profile
+                    parent:(const BookmarkNode*)parent
+             configuration:(BookmarkEditor::Configuration)configuration
+                   handler:(BookmarkEditor::Handler*)handler;
+
+@end
+
+@interface BookmarkAllTabsController(TestingAPI)
+
+// Initializes the list of all tab names and URLs.  Overridden by unit test
+// to provide canned test data.
+- (void)UpdateActiveTabPairs;
+
+// Provides testing access to tab pairs list.
+- (ActiveTabsNameURLPairVector*)activeTabPairsVector;
+
+@end
+
+#endif  /* CHROME_BROWSER_COCOA_BOOKMARK_ALL_TABS_CONTROLLER_H_ */
diff --git a/chrome/browser/cocoa/bookmark_all_tabs_controller.mm b/chrome/browser/cocoa/bookmark_all_tabs_controller.mm
new file mode 100644
index 0000000..93b4026
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_all_tabs_controller.mm
@@ -0,0 +1,90 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/cocoa/bookmark_all_tabs_controller.h"
+#include "app/l10n_util_mac.h"
+#include "base/sys_string_conversions.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/browser.h"
+#include "chrome/browser/browser_list.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+#include "grit/generated_resources.h"
+
+@implementation BookmarkAllTabsController
+
+- (id)initWithParentWindow:(NSWindow*)parentWindow
+                   profile:(Profile*)profile
+                    parent:(const BookmarkNode*)parent
+             configuration:(BookmarkEditor::Configuration)configuration
+                   handler:(BookmarkEditor::Handler*)handler {
+  NSString* nibName = @"BookmarkAllTabs";
+  if ((self = [super initWithParentWindow:parentWindow
+                                  nibName:nibName
+                                  profile:profile
+                                   parent:parent
+                            configuration:configuration
+                                  handler:handler])) {
+  }
+  return self;
+}
+
+- (void)awakeFromNib {
+  [self
+   setInitialName:
+       l10n_util::GetNSStringWithFixup(IDS_BOOMARK_EDITOR_NEW_FOLDER_NAME)];
+  [super awakeFromNib];
+}
+
+#pragma mark Bookmark Editing
+
+- (void)UpdateActiveTabPairs {
+  activeTabPairsVector_.clear();
+  Browser* browser = BrowserList::GetLastActive();
+  TabStripModel* tabstrip_model = browser->tabstrip_model();
+  const int tabCount = tabstrip_model->count();
+  for (int i = 0; i < tabCount; ++i) {
+    TabContents* tc = tabstrip_model->GetTabContentsAt(i);
+    const std::wstring tabTitle = UTF16ToWideHack(tc->GetTitle());
+    const GURL& tabURL(tc->GetURL());
+    ActiveTabNameURLPair tabPair(tabTitle, tabURL);
+    activeTabPairsVector_.push_back(tabPair);
+  }
+}
+
+// The the name for the folder into which the tabs will be recorded as
+// bookmarks is assumed to be non-empty.  The folder is then created
+// and a bookmark for each open tab added therein.
+- (IBAction)ok:(id)sender {
+  NSString* name = [[self displayName] stringByTrimmingCharactersInSet:
+                    [NSCharacterSet newlineCharacterSet]];
+  std::wstring newTitle = base::SysNSStringToWide(name);
+  const BookmarkNode* newParentNode = [self selectedNode];
+  int newIndex = newParentNode->GetChildCount();
+  // Create the new folder which will contain all of the tab URLs.
+  NSString* newFolderName = [self displayName];
+  std::wstring newFolderString = base::SysNSStringToWide(newFolderName);
+  BookmarkModel* model = [self bookmarkModel];
+  const BookmarkNode* newFolder = model->AddGroup(newParentNode, newIndex,
+                                                  newFolderString);
+  [self NotifyHandlerCreatedNode:newFolder];
+  // Get a list of all open tabs, create nodes for them, and add
+  // to the new folder node.
+  [self UpdateActiveTabPairs];
+  int i = 0;
+  for (ActiveTabsNameURLPairVector::const_iterator it =
+           activeTabPairsVector_.begin();
+       it != activeTabPairsVector_.end(); ++it, ++i) {
+    const BookmarkNode* node = model->AddURL(newFolder, i,
+                                             it->first, it->second);
+    [self NotifyHandlerCreatedNode:node];
+  }
+  [super ok:sender];
+}
+
+- (ActiveTabsNameURLPairVector*)activeTabPairsVector {
+  return &activeTabPairsVector_;
+}
+
+@end  // BookmarkAllTabsController
+
diff --git a/chrome/browser/cocoa/bookmark_all_tabs_controller_unittest.mm b/chrome/browser/cocoa/bookmark_all_tabs_controller_unittest.mm
new file mode 100644
index 0000000..db452b1
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_all_tabs_controller_unittest.mm
@@ -0,0 +1,81 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/scoped_nsobject.h"
+#include "base/sys_string_conversions.h"
+#import "chrome/browser/cocoa/bookmark_all_tabs_controller.h"
+#include "chrome/browser/cocoa/browser_test_helper.h"
+#import "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+@interface BookmarkAllTabsControllerOverride : BookmarkAllTabsController
+@end
+
+@implementation BookmarkAllTabsControllerOverride
+
+- (void)UpdateActiveTabPairs {
+  ActiveTabsNameURLPairVector* activeTabPairsVector =
+      [self activeTabPairsVector];
+  activeTabPairsVector->clear();
+  activeTabPairsVector->push_back(
+      ActiveTabNameURLPair(L"at-0", GURL("http://at-0.com")));
+  activeTabPairsVector->push_back(
+      ActiveTabNameURLPair(L"at-1", GURL("http://at-1.com")));
+  activeTabPairsVector->push_back(
+      ActiveTabNameURLPair(L"at-2", GURL("http://at-2.com")));
+}
+
+@end
+
+class BookmarkAllTabsControllerTest : public CocoaTest {
+ public:
+  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
+  BrowserTestHelper helper_;
+  const BookmarkNode* parent_node_;
+  BookmarkAllTabsControllerOverride* controller_;
+  const BookmarkNode* group_a_;
+
+  BookmarkAllTabsControllerTest() {
+    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"));
+  }
+
+  virtual BookmarkAllTabsControllerOverride* CreateController() {
+    return [[BookmarkAllTabsControllerOverride alloc]
+            initWithParentWindow:cocoa_helper_.window()
+                         profile:helper_.profile()
+                          parent:group_a_
+                   configuration:BookmarkEditor::SHOW_TREE
+                         handler:nil];
+  }
+
+  virtual void SetUp() {
+    CocoaTest::SetUp();
+    controller_ = CreateController();
+    EXPECT_TRUE([controller_ window]);
+  }
+
+  virtual void TearDown() {
+    [controller_ close];
+    CocoaTest::TearDown();
+  }
+};
+
+TEST_F(BookmarkAllTabsControllerTest, BookmarkAllTabs) {
+  // OK button should always be enabled.
+  EXPECT_TRUE([controller_ okButtonEnabled]);
+  [controller_ setDisplayName:@"ALL MY TABS"];
+  [controller_ ok:nil];
+  EXPECT_EQ(4, group_a_->GetChildCount());
+  const BookmarkNode* folderChild = group_a_->GetChild(3);
+  EXPECT_EQ(folderChild->GetTitle(), L"ALL MY TABS");
+  EXPECT_EQ(3, folderChild->GetChildCount());
+}
diff --git a/chrome/browser/cocoa/bookmark_editor_base_controller.h b/chrome/browser/cocoa/bookmark_editor_base_controller.h
new file mode 100644
index 0000000..cb0725d
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_editor_base_controller.h
@@ -0,0 +1,97 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_COCOA_BOOKMARK_EDITOR_BASE_CONTROLLER_H_
+#define CHROME_BROWSER_COCOA_BOOKMARK_EDITOR_BASE_CONTROLLER_H_
+
+#import <Cocoa/Cocoa.h>
+
+#import "base/cocoa_protocols_mac.h"
+#include "base/scoped_ptr.h"
+#include "base/scoped_nsobject.h"
+#include "chrome/browser/bookmarks/bookmark_editor.h"
+
+@class BookmarkTreeBrowserCell;
+class BookmarkModel;
+
+// A base controller class for bookmark creation and editing dialogs which
+// present the current bookmark folder structure in a tree view.  Do not
+// instantiate this controller directly -- use one of its derived classes.
+// NOTE: If your derived classes is intended to be dispatched via the
+// BookmarkEditor::Show static function found in the accompanying
+// implementation will need to update that function.
+@interface BookmarkEditorBaseController :
+    NSWindowController<NSMatrixDelegate, NSTextFieldDelegate> {
+ @private
+  IBOutlet NSBrowser* folderBrowser_;
+  IBOutlet NSButton* newFolderButton_;
+  IBOutlet NSButton* okButton_;  // Used for unit testing only.
+
+  NSWindow* parentWindow_;
+  Profile* profile_;  // weak
+  const BookmarkNode* parentNode_;  // weak; owned by the model
+  BookmarkEditor::Configuration configuration_;
+  scoped_ptr<BookmarkEditor::Handler> handler_;  // we take ownership
+  NSString* initialName_;
+  NSString* displayName_;  // Bound to a text field in the dialog.
+  BOOL okEnabled_;  // Bound to the OK button.
+}
+
+@property (copy) NSString* initialName;
+@property (copy) NSString* displayName;
+@property (assign) BOOL okEnabled;
+
+// Designated initializer.  Derived classes should call through to this init.
+- (id)initWithParentWindow:(NSWindow*)parentWindow
+                   nibName:(NSString*)nibName
+                   profile:(Profile*)profile
+                    parent:(const BookmarkNode*)parent
+             configuration:(BookmarkEditor::Configuration)configuration
+                   handler:(BookmarkEditor::Handler*)handler;
+
+// Run the bookmark editor as a modal sheet.  Does not block.
+- (void)runAsModalSheet;
+
+// Create a new folder at the end of the selected parent folder, give it
+// an untitled name, and put it into editing mode.
+- (IBAction)newFolder:(id)sender;
+
+// Actions for the buttons at the bottom of the window.
+- (IBAction)cancel:(id)sender;
+
+// The OK action will record the edited bookmark.  The default dismisses
+// the dialog and should be called by the derived class if overridden.
+- (IBAction)ok:(id)sender;
+
+// Methods for use by derived classes only.
+
+// 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.  This assumes that leaf nodes (pure
+// bookmarks) are not being presented.
+- (const BookmarkNode*)selectedNode;
+
+// Select/highlight the given node within the browser tree view.  If the
+// node is nil then select the bookmark bar node.  Exposed for unit test.
+- (void)selectNodeInBrowser:(const BookmarkNode*)node;
+
+// Notify the handler, if any, of a node creation.
+- (void)NotifyHandlerCreatedNode:(const BookmarkNode*)node;
+
+// Accessors
+- (BookmarkModel*)bookmarkModel;
+- (const BookmarkNode*)parentNode;
+
+@end
+
+@interface BookmarkEditorBaseController(TestingAPI)
+@property (readonly) BOOL okButtonEnabled;
+- (void)selectTestNodeInBrowser:(const BookmarkNode*)node;
++ (const BookmarkNode*)folderChildForParent:(const BookmarkNode*)parent
+                                  withFolderIndex:(NSInteger)index;
++ (int)indexOfFolderChild:(const BookmarkNode*)child;
+@end
+
+#endif  /* CHROME_BROWSER_COCOA_BOOKMARK_EDITOR_BASE_CONTROLLER_H_ */
diff --git a/chrome/browser/cocoa/bookmark_editor_base_controller.mm b/chrome/browser/cocoa/bookmark_editor_base_controller.mm
new file mode 100644
index 0000000..d97874a
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_editor_base_controller.mm
@@ -0,0 +1,398 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/cocoa/bookmark_editor_base_controller.h"
+#include "app/l10n_util.h"
+#include "base/logging.h"
+#include "base/mac_util.h"
+#include "base/sys_string_conversions.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
+#import "chrome/browser/cocoa/bookmark_all_tabs_controller.h"
+#import "chrome/browser/cocoa/bookmark_editor_controller.h"
+#import "chrome/browser/cocoa/bookmark_tree_browser_cell.h"
+#include "chrome/browser/profile.h"
+#include "grit/generated_resources.h"
+
+@interface BookmarkEditorBaseController (Private)
+
+// Given a cell in the folder browser, make that cell editable so that the
+// bookmark folder name can be modified by the user.
+- (void)editFolderNameInCell:(BookmarkTreeBrowserCell*)cell;
+
+// The action called by the bookmark folder name cell being edited by
+// the user when editing has been completed (such as by pressing <return>).
+- (void)cellEditingCompleted:(id)sender;
+
+// Update the folder name from the current edit in the given cell
+// and return the focus to the folder tree browser.
+- (void)saveFolderNameForCell:(BookmarkTreeBrowserCell*)cell;
+
+// A custom action handler called by the bookmark folder browser when the
+// user has double-clicked on a folder name.
+- (void)browserDoubleClicked:(id)sender;
+
+@end
+
+// static; implemented for each platform.
+void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
+                          Profile* profile,
+                          const BookmarkNode* parent,
+                          const EditDetails& details,
+                          Configuration configuration,
+                          Handler* handler) {
+  BookmarkEditorBaseController* controller = nil;
+  if (details.type == EditDetails::NEW_FOLDER) {
+    controller = [[BookmarkAllTabsController alloc]
+                  initWithParentWindow:parent_hwnd
+                               profile:profile
+                                parent:parent
+                         configuration:configuration
+                               handler:handler];
+  } else {
+    controller = [[BookmarkEditorController alloc]
+                  initWithParentWindow:parent_hwnd
+                               profile:profile
+                                parent:parent
+                                  node:details.existing_node
+                         configuration:configuration
+                               handler:handler];
+  }
+  [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 BookmarkEditorBaseController
+
+@synthesize initialName = initialName_;
+@synthesize displayName = displayName_;
+@synthesize okEnabled = okEnabled_;
+
+- (id)initWithParentWindow:(NSWindow*)parentWindow
+                   nibName:(NSString*)nibName
+                   profile:(Profile*)profile
+                    parent:(const BookmarkNode*)parent
+             configuration:(BookmarkEditor::Configuration)configuration
+                   handler:(BookmarkEditor::Handler*)handler {
+  NSString* nibpath = [mac_util::MainAppBundle()
+                        pathForResource:nibName
+                                 ofType:@"nib"];
+  if ((self = [super initWithWindowNibPath:nibpath owner:self])) {
+    parentWindow_ = parentWindow;
+    profile_ = profile;
+    parentNode_ = parent;
+    configuration_ = configuration;
+    handler_.reset(handler);
+    initialName_ = [@"" retain];
+  }
+  return self;
+}
+
+- (void)dealloc {
+  [initialName_ release];
+  [displayName_ release];
+  [super dealloc];
+}
+
+- (void)awakeFromNib {
+  [self setDisplayName:[self initialName]];
+
+  if (configuration_ != BookmarkEditor::SHOW_TREE) {
+    // Remember the NSBrowser's height; we will shrink our frame by that
+    // much.
+    NSRect frame = [[self window] frame];
+    CGFloat browserHeight = [folderBrowser_ frame].size.height;
+    frame.size.height -= browserHeight;
+    frame.origin.y += browserHeight;
+    // Remove the NSBrowser and "new folder" button.
+    [folderBrowser_ removeFromSuperview];
+    [newFolderButton_ removeFromSuperview];
+    // Finally, commit the size change.
+    [[self window] setFrame:frame display:YES];
+  }
+
+  [folderBrowser_ setCellClass:[BookmarkTreeBrowserCell class]];
+  [folderBrowser_ setDoubleAction:@selector(browserDoubleClicked:)];
+}
+
+- (void)windowDidLoad {
+  if (configuration_ == BookmarkEditor::SHOW_TREE) {
+    [self selectNodeInBrowser:parentNode_];
+  }
+}
+
+- (void)windowWillClose:(NSNotification *)notification {
+  // If a folder name cell is being edited then force it to end editing
+  // so that any changes are recorded.
+  [[self window] makeFirstResponder:nil];
+  [self autorelease];
+}
+
+/* TODO(jrg):
+// Implementing this informal protocol allows us to open the sheet
+// somewhere other than at the top of the window.  NOTE: this means
+// that I, the controller, am also the window's delegate.
+- (NSRect)window:(NSWindow*)window willPositionSheet:(NSWindow*)sheet
+        usingRect:(NSRect)rect {
+  // adjust rect.origin.y to be the bottom of the toolbar
+  return rect;
+}
+*/
+
+// TODO(jrg): consider NSModalSession.
+- (void)runAsModalSheet {
+  [NSApp beginSheet:[self window]
+     modalForWindow:parentWindow_
+      modalDelegate:self
+     didEndSelector:@selector(didEndSheet:returnCode:contextInfo:)
+        contextInfo:nil];
+}
+
+- (void)selectNodeInBrowser:(const BookmarkNode*)node {
+  DCHECK(configuration_ == BookmarkEditor::SHOW_TREE);
+  std::deque<NSInteger> rowsToSelect;
+  const BookmarkNode* nodeParent = nil;
+  if (node) {
+    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();
+    }
+  } else {
+    BookmarkModel* model = profile_->GetBookmarkModel();
+    nodeParent = model->GetBookmarkBarNode();
+    rowsToSelect.push_front(0);
+  }
+  for (std::deque<NSInteger>::size_type column = 0;
+       column < rowsToSelect.size();
+       ++column) {
+    [folderBrowser_ selectRow:rowsToSelect[column] inColumn:column];
+  }
+
+  // Force the OK button state to be re-evaluated.
+  [self willChangeValueForKey:@"okEnabled"];
+  [self didChangeValueForKey:@"okEnabled"];
+}
+
+- (const BookmarkNode*)selectedNode {
+  BookmarkModel* model = profile_->GetBookmarkModel();
+  const BookmarkNode* selectedNode = NULL;
+  // Determine a new parent node only if the browser is showing.
+  if (configuration_ == BookmarkEditor::SHOW_TREE) {
+    selectedNode = model->root_node();
+    NSInteger column = 0;
+    NSInteger selectedRow = [folderBrowser_ selectedRowInColumn:column];
+    while (selectedRow >= 0) {
+      selectedNode = GetFolderChildForParent(selectedNode,
+                                             selectedRow);
+      ++column;
+      selectedRow = [folderBrowser_ selectedRowInColumn:column];
+    }
+  } else {
+    // If the tree is not showing then we use the original parent.
+    selectedNode = parentNode_;
+  }
+  return selectedNode;
+}
+
+- (void)NotifyHandlerCreatedNode:(const BookmarkNode*)node {
+  if (handler_.get())
+    handler_->NodeCreated(node);
+}
+
+#pragma mark New Folder Handler & Folder Cell Editing
+
+- (void)editFolderNameInCell:(BookmarkTreeBrowserCell*)cell {
+  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
+  [cell setEditable:YES];
+  [cell setTarget:self];
+  [cell setAction:@selector(cellEditingCompleted:)];
+  [cell setSendsActionOnEndEditing:YES];
+  NSMatrix* matrix = [cell matrix];
+  // Set the delegate so that we get called when editing completes.
+  [matrix setDelegate:self];
+  [matrix selectText:self];
+}
+
+- (void)cellEditingCompleted:(id)sender {
+  DCHECK([sender isKindOfClass:[NSMatrix class]]);
+  BookmarkTreeBrowserCell* cell = [sender selectedCell];
+  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
+  [self saveFolderNameForCell:cell];
+}
+
+- (void)saveFolderNameForCell:(BookmarkTreeBrowserCell*)cell {
+  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
+  // It's possible that the cell can get reused so clean things up
+  // to prevent inadvertant notifications.
+  [cell setTarget:nil];
+  [cell setAction:nil];
+  [cell setEditable:NO];
+  [cell setSendsActionOnEndEditing:NO];
+  // Force a responder change here to force the editing of the cell's text
+  // to complete otherwise the call to -[cell title] could return stale text.
+  // The focus does not automatically get reset to the browser when the
+  // cell gives up focus.
+  [[folderBrowser_ window] makeFirstResponder:folderBrowser_];
+  const BookmarkNode* bookmarkNode = [cell bookmarkNode];
+  BookmarkModel* model = profile_->GetBookmarkModel();
+  NSString* newTitle = [cell title];
+  model->SetTitle(bookmarkNode, base::SysNSStringToWide(newTitle));
+}
+
+- (void)browserDoubleClicked:(id)sender {
+  BookmarkTreeBrowserCell* cell = [folderBrowser_ selectedCell];
+  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
+  [self editFolderNameInCell:cell];
+}
+
+- (IBAction)newFolder:(id)sender {
+  BookmarkModel* model = profile_->GetBookmarkModel();
+  const BookmarkNode* newParentNode = [self selectedNode];
+  int newIndex = newParentNode->GetChildCount();
+  std::wstring newFolderString =
+      l10n_util::GetString(IDS_BOOMARK_EDITOR_NEW_FOLDER_NAME);
+  const BookmarkNode* newFolder = model->AddGroup(newParentNode, newIndex,
+                                                  newFolderString);
+  [self selectNodeInBrowser:newFolder];
+  BookmarkTreeBrowserCell* cell = [folderBrowser_ selectedCell];
+  [self editFolderNameInCell:cell];
+}
+
+- (BOOL)okEnabled {
+  return YES;
+}
+
+- (IBAction)ok:(id)sender {
+  [NSApp endSheet:[self window]];
+}
+
+- (IBAction)cancel:(id)sender {
+  [NSApp endSheet:[self window]];
+}
+
+- (void)didEndSheet:(NSWindow*)sheet
+         returnCode:(int)returnCode
+        contextInfo:(void*)contextInfo {
+  [sheet close];
+}
+
+- (BookmarkModel*)bookmarkModel {
+  return profile_->GetBookmarkModel();
+}
+
+- (const BookmarkNode*)parentNode {
+  return parentNode_;
+}
+
+#pragma mark For Unit Test Use Only
+
+- (BOOL)okButtonEnabled {
+  return [okButton_ isEnabled];
+}
+
+- (void)selectTestNodeInBrowser:(const BookmarkNode*)node {
+  [self selectNodeInBrowser:node];
+}
+
++ (const BookmarkNode*)folderChildForParent:(const BookmarkNode*)parent
+                            withFolderIndex:(NSInteger)index {
+  return GetFolderChildForParent(parent, index);
+}
+
++ (int)indexOfFolderChild:(const BookmarkNode*)child {
+  return IndexOfFolderChild(child);
+}
+
+
+#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 = [folderBrowser_ selectedRowInColumn:i];
+    node = GetFolderChildForParent(node, 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);
+  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
+  const BookmarkNode* parentNode = [self parentNodeForColumn:column];
+  const BookmarkNode* childNode = GetFolderChildForParent(parentNode, row);
+  DCHECK(childNode);
+  BookmarkTreeBrowserCell* browserCell =
+      static_cast<BookmarkTreeBrowserCell*>(cell);
+  [browserCell setTitle:base::SysWideToNSString(childNode->GetTitle())];
+  [browserCell setBookmarkNode:childNode];
+  [browserCell setMatrix:[folderBrowser_ matrixInColumn:column]];
+}
+
+@end  // BookmarkEditorBaseController
+
diff --git a/chrome/browser/cocoa/bookmark_editor_base_controller_unittest.mm b/chrome/browser/cocoa/bookmark_editor_base_controller_unittest.mm
new file mode 100644
index 0000000..908fe25
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_editor_base_controller_unittest.mm
@@ -0,0 +1,149 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/scoped_nsobject.h"
+#include "base/sys_string_conversions.h"
+#import "chrome/browser/cocoa/bookmark_editor_controller.h"
+#include "chrome/browser/cocoa/browser_test_helper.h"
+#import "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+class BookmarkEditorBaseControllerTest : public CocoaTest {
+ public:
+  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
+  BrowserTestHelper helper_;
+  BookmarkEditorBaseController* controller_;
+  const BookmarkNode* group_a_;
+  const BookmarkNode* group_b_;
+  const BookmarkNode* group_b_0_;
+  const BookmarkNode* group_b_3_;
+  const BookmarkNode* group_c_;
+
+  BookmarkEditorBaseControllerTest() {
+    // Set up a small bookmark hierarchy, which will look as follows:
+    //    a      b      c    d
+    //     a-0    b-0    c-0
+    //     a-1     b-00  c-1
+    //     a-2    b-1    c-2
+    //            b-2    c-3
+    //            b-3
+    //             b-30
+    //             b-31
+    //            b-4
+    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");
+    group_b_0_ = model.AddGroup(group_b_, 0, L"b-0");
+    model.AddURL(group_b_0_, 0, L"bb-0", GURL("http://bb-0.com"));
+    model.AddURL(group_b_, 1, L"b-1", GURL("http://b-1.com"));
+    model.AddURL(group_b_, 2, L"b-2", GURL("http://b-2.com"));
+    group_b_3_ = model.AddGroup(group_b_, 3, L"b-3");
+    model.AddURL(group_b_3_, 0, L"b-30", GURL("http://b-30.com"));
+    model.AddURL(group_b_3_, 1, L"b-31", GURL("http://b-31.com"));
+    model.AddURL(group_b_, 4, L"b-4", GURL("http://b-4.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"));
+  }
+
+  virtual BookmarkEditorBaseController* CreateController() {
+    return [[BookmarkEditorBaseController alloc]
+            initWithParentWindow:test_window()
+                         nibName:@"BookmarkAllTabs"
+                         profile:helper_.profile()
+                          parent:group_b_0_
+                   configuration:BookmarkEditor::SHOW_TREE
+                         handler:nil];
+  }
+
+  virtual void SetUp() {
+    CocoaTest::SetUp();
+    controller_ = CreateController();
+    EXPECT_TRUE([controller_ window]);
+  }
+
+  virtual void TearDown() {
+    [controller_ close];
+    CocoaTest::TearDown();
+  }
+};
+
+TEST_F(BookmarkEditorBaseControllerTest, VerifyBookmarkTestModel) {
+  BookmarkModel& model(*(helper_.profile()->GetBookmarkModel()));
+  const BookmarkNode& root(*model.GetBookmarkBarNode());
+  EXPECT_EQ(4, root.GetChildCount());
+  // a
+  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());
+  // b
+  child = root.GetChild(1);
+  EXPECT_EQ(5, child->GetChildCount());
+  subchild = child->GetChild(0);
+  EXPECT_EQ(1, subchild->GetChildCount());
+  const BookmarkNode* subsubchild = subchild->GetChild(0);
+  EXPECT_EQ(0, subsubchild->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(2, subchild->GetChildCount());
+  subsubchild = subchild->GetChild(0);
+  EXPECT_EQ(0, subsubchild->GetChildCount());
+  subsubchild = subchild->GetChild(1);
+  EXPECT_EQ(0, subsubchild->GetChildCount());
+  subchild = child->GetChild(4);
+  EXPECT_EQ(0, subchild->GetChildCount());
+  // c
+  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());
+  // d
+  child = root.GetChild(3);
+  EXPECT_EQ(0, child->GetChildCount());
+}
+
+TEST_F(BookmarkEditorBaseControllerTest, FolderChildForParent) {
+  const BookmarkNode* child =
+    [BookmarkEditorBaseController folderChildForParent:group_b_
+                                       withFolderIndex:1];
+  EXPECT_EQ(child, group_b_3_);
+}
+
+TEST_F(BookmarkEditorBaseControllerTest, IndexOfFolderChild) {
+  int index = [BookmarkEditorBaseController indexOfFolderChild:group_b_3_];
+  EXPECT_EQ(index, 1);
+}
+
+TEST_F(BookmarkEditorBaseControllerTest, NodeSelection) {
+  [controller_ selectNodeInBrowser:group_b_3_];
+  const BookmarkNode* node = [controller_ selectedNode];
+  EXPECT_EQ(node, group_b_3_);
+}
diff --git a/chrome/browser/cocoa/bookmark_editor_controller.h b/chrome/browser/cocoa/bookmark_editor_controller.h
index 055b7fd..ff779b8 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller.h
+++ b/chrome/browser/cocoa/bookmark_editor_controller.h
@@ -5,37 +5,19 @@
 #ifndef CHROME_BROWSER_COCOA_BOOKMARK_EDITOR_CONTROLLER_H_
 #define CHROME_BROWSER_COCOA_BOOKMARK_EDITOR_CONTROLLER_H_
 
-#import <Cocoa/Cocoa.h>
-
-#import "base/cocoa_protocols_mac.h"
-#include "base/scoped_ptr.h"
-#include "base/scoped_nsobject.h"
-#include "chrome/browser/bookmarks/bookmark_editor.h"
-
-@class BookmarkTreeBrowserCell;
+#import "chrome/browser/cocoa/bookmark_editor_base_controller.h"
 
 // A controller for the bookmark editor, opened with Edit... from the
 // context menu of a bookmark button.
-@interface BookmarkEditorController : NSWindowController<NSMatrixDelegate,
-                                                         NSTextFieldDelegate> {
+@interface BookmarkEditorController : BookmarkEditorBaseController {
  @private
-  IBOutlet NSTextField* nameField_;
-  IBOutlet NSTextField* urlField_;
-  IBOutlet NSBrowser* folderBrowser_;
-  IBOutlet NSButton* okButton_;
-  IBOutlet NSButton* newFolderButton_;
-
-  NSWindow* parentWindow_;
-  Profile* profile_;  // weak
-  const BookmarkNode* parentNode_;  // weak; owned by the model
   const BookmarkNode* node_;  // weak; owned by the model
-  BookmarkEditor::Configuration configuration_;
-  scoped_ptr<BookmarkEditor::Handler> handler_;  // we take ownership
-
-  scoped_nsobject<NSString> initialName_;
   scoped_nsobject<NSString> initialUrl_;
+  NSString* displayURL_;  // Bound to a text field in the dialog.
 }
 
+@property (copy) NSString* displayURL;
+
 - (id)initWithParentWindow:(NSWindow*)parentWindow
                    profile:(Profile*)profile
                     parent:(const BookmarkNode*)parent
@@ -43,23 +25,6 @@
              configuration:(BookmarkEditor::Configuration)configuration
                    handler:(BookmarkEditor::Handler*)handler;
 
-// Run the bookmark editor as a modal sheet.  Does not block.
-- (void)runAsModalSheet;
-
-// Create a new folder at the end of the selected parent folder, give it
-// an untitled name, and put it into editing mode.
-- (IBAction)newFolder:(id)sender;
-
-// Actions for the buttons at the bottom of the window.
-- (IBAction)cancel:(id)sender;
-- (IBAction)ok:(id)sender;
-@end
-
-@interface BookmarkEditorController(TestingAPI)
-@property (assign) NSString* displayName;
-@property (assign) NSString* displayURL;
-@property (readonly) BOOL okButtonEnabled;
-- (void)selectTestNodeInBrowser:(const BookmarkNode*)node;
 @end
 
 #endif  /* CHROME_BROWSER_COCOA_BOOKMARK_EDITOR_CONTROLLER_H_ */
diff --git a/chrome/browser/cocoa/bookmark_editor_controller.mm b/chrome/browser/cocoa/bookmark_editor_controller.mm
index 2915094..8164d4b 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller.mm
@@ -2,319 +2,69 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#import "chrome/browser/cocoa/bookmark_editor_controller.h"
 #include "app/l10n_util.h"
-#include "base/logging.h"
-#include "base/mac_util.h"
 #include "base/sys_string_conversions.h"
-#include "chrome/browser/bookmarks/bookmark_editor.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
-#include "chrome/browser/profile.h"
-#import "chrome/browser/cocoa/bookmark_tree_browser_cell.h"
-#import "chrome/browser/cocoa/bookmark_editor_controller.h"
-#include "grit/generated_resources.h"
 
 @interface BookmarkEditorController (Private)
 
 // Grab the url from the text field and convert.
 - (GURL)GURLFromUrlField;
 
-// Given a cell in the folder browser, make that cell editable so that the
-// bookmark folder name can be modified by the user.
-- (void)editFolderNameInCell:(BookmarkTreeBrowserCell*)cell;
-
-// The action called by the bookmark folder name cell being edited by
-// the user when editing has been completed (such as by pressing <return>).
-- (void)cellEditingCompleted:(id)sender;
-
-// Update the folder name from the current edit in the given cell
-// and return the focus to the folder tree browser.
-- (void)saveFolderNameForCell:(BookmarkTreeBrowserCell*)cell;
-
-// A custom action handler called by the bookmark folder browser when the
-// user has double-clicked on a folder name.
-- (void)browserDoubleClicked:(id)sender;
-
-// 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.  This assumes that leaf nodes (pure
-// bookmarks) are not being presented.
-- (const BookmarkNode*)selectedNode;
-
-// Select/highlight the given node within the browser tree view.  If the
-// node is nil then select the bookmark bar.
-- (void)selectNodeInBrowser:(const BookmarkNode*)node;
-
 @end
 
-// static; implemented for each platform.
-void BookmarkEditor::Show(gfx::NativeWindow parent_hwnd,
-                          Profile* profile,
-                          const BookmarkNode* parent,
-                          const EditDetails& details,
-                          Configuration configuration,
-                          Handler* handler) {
-  if (details.type == EditDetails::NEW_FOLDER) {
-    // TODO(sky): implement this.
-    NOTIMPLEMENTED();
-    return;
-  }
-  BookmarkEditorController* controller = [[BookmarkEditorController alloc]
-      initWithParentWindow:parent_hwnd
-                   profile:profile
-                    parent:parent
-                      node:details.existing_node
-             configuration:configuration
-                   handler:handler];
-  [controller runAsModalSheet];
-}
-
-#pragma mark Bookmark TreeNode Helpers
-
-namespace {
+@implementation BookmarkEditorController
 
-// 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;
-}
+@synthesize displayURL = displayURL_;
 
-// 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;
++ (NSSet*)keyPathsForValuesAffectingOkEnabled {
+  return [NSSet setWithObject:@"displayURL"];
 }
 
-} // namespace
-
-@implementation BookmarkEditorController
-
 - (id)initWithParentWindow:(NSWindow*)parentWindow
                    profile:(Profile*)profile
                     parent:(const BookmarkNode*)parent
                       node:(const BookmarkNode*)node
              configuration:(BookmarkEditor::Configuration)configuration
                    handler:(BookmarkEditor::Handler*)handler {
-  NSString* nibpath = [mac_util::MainAppBundle()
-                        pathForResource:@"BookmarkEditor"
-                        ofType:@"nib"];
-  if ((self = [super initWithWindowNibPath:nibpath owner:self])) {
-    parentWindow_ = parentWindow;
-    profile_ = profile;
-    parentNode_ = parent;
+  if ((self = [super initWithParentWindow:parentWindow
+                                  nibName:@"BookmarkEditor"
+                                  profile:profile
+                                   parent:parent
+                            configuration:configuration
+                                  handler:handler])) {
     // "Add Page..." has no "node" so this may be NULL.
     node_ = node;
-    configuration_ = configuration;
-    handler_.reset(handler);
   }
   return self;
 }
 
+- (void)dealloc {
+  [displayURL_ release];
+  [super dealloc];
+}
+
 - (void)awakeFromNib {
   // Set text fields to match our bookmark.  If the node is NULL we
   // arrived here from an "Add Page..." item in a context menu.
   if (node_) {
-    initialName_.reset([base::SysWideToNSString(node_->GetTitle()) retain]);
+    [self setInitialName:base::SysWideToNSString(node_->GetTitle())];
     std::string url_string = node_->GetURL().possibly_invalid_spec();
     initialUrl_.reset([[NSString stringWithUTF8String:url_string.c_str()]
                         retain]);
   } else {
-    initialName_.reset([@"" retain]);
     initialUrl_.reset([@"" retain]);
   }
-  [nameField_ setStringValue:initialName_];
-  [urlField_ setStringValue:initialUrl_];
-
-  // Get a ping when the URL or name text fields change;
-  // trigger an initial ping to set things up.
-  [nameField_ setDelegate:self];
-  [urlField_ setDelegate:self];
-  [self controlTextDidChange:nil];
-
-  if (configuration_ != BookmarkEditor::SHOW_TREE) {
-    // Remember the NSBrowser's height; we will shrink our frame by that
-    // much.
-    NSRect frame = [[self window] frame];
-    CGFloat browserHeight = [folderBrowser_ frame].size.height;
-    frame.size.height -= browserHeight;
-    frame.origin.y += browserHeight;
-    // Remove the NSBrowser and "new folder" button.
-    [folderBrowser_ removeFromSuperview];
-    [newFolderButton_ removeFromSuperview];
-    // Finally, commit the size change.
-    [[self window] setFrame:frame display:YES];
-  }
-
-  [folderBrowser_ setCellClass:[BookmarkTreeBrowserCell class]];
-  [folderBrowser_ setDoubleAction:@selector(browserDoubleClicked:)];
-}
-
-- (void)windowDidLoad {
-  if (configuration_ == BookmarkEditor::SHOW_TREE) {
-    // Find and select the |parent| bookmark node in the folder tree browser.
-    [self selectNodeInBrowser:parentNode_];
-  }
-}
-
-- (void)windowWillClose:(NSNotification *)notification {
-  // If a folder name cell is being edited then force it to end editing
-  // so that any changes are recorded.
-  [[self window] makeFirstResponder:nil];
-
-  // This is probably unnecessary but it feels cleaner since the
-  // delegate of a text field can be automatically registered for
-  // notifications.
-  [nameField_ setDelegate:nil];
-  [urlField_ setDelegate:nil];
-  [self autorelease];
-}
-
-/* TODO(jrg):
-// Implementing this informal protocol allows us to open the sheet
-// somewhere other than at the top of the window.  NOTE: this means
-// that I, the controller, am also the window's delegate.
-- (NSRect)window:(NSWindow*)window willPositionSheet:(NSWindow*)sheet
-        usingRect:(NSRect)rect {
-  // adjust rect.origin.y to be the bottom of the toolbar
-  return rect;
-}
-*/
-
-// TODO(jrg): consider NSModalSession.
-- (void)runAsModalSheet {
-  [NSApp beginSheet:[self window]
-     modalForWindow:parentWindow_
-      modalDelegate:self
-     didEndSelector:@selector(didEndSheet:returnCode:contextInfo:)
-        contextInfo:nil];
-}
-
-- (void)selectNodeInBrowser:(const BookmarkNode*)node {
-  DCHECK(configuration_ == BookmarkEditor::SHOW_TREE);
-  std::deque<NSInteger> rowsToSelect;
-  const BookmarkNode* nodeParent = nil;
-  if (node) {
-    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();
-    }
-  } else {
-    BookmarkModel* model = profile_->GetBookmarkModel();
-    nodeParent = model->GetBookmarkBarNode();
-    rowsToSelect.push_front(0);
-  }
-  for (std::deque<NSInteger>::size_type column = 0;
-       column < rowsToSelect.size();
-       ++column) {
-    [folderBrowser_ selectRow:rowsToSelect[column] inColumn:column];
-  }
-  [self controlTextDidChange:nil];
-}
-
-- (const BookmarkNode*)selectedNode {
-  BookmarkModel* model = profile_->GetBookmarkModel();
-  const BookmarkNode* selectedNode = NULL;
-  // Determine a new parent node only if the browser is showing.
-  if (configuration_ == BookmarkEditor::SHOW_TREE) {
-    selectedNode = model->root_node();
-    NSInteger column = 0;
-    NSInteger selectedRow = [folderBrowser_ selectedRowInColumn:column];
-    while (selectedRow >= 0) {
-      selectedNode = GetFolderChildForParent(selectedNode,
-                                             selectedRow);
-      ++column;
-      selectedRow = [folderBrowser_ selectedRowInColumn:column];
-    }
-  } else {
-    // If the tree is not showing then we use the original parent.
-    selectedNode = parentNode_;
-  }
-  return selectedNode;
-}
-
-#pragma mark New Folder Handler & Folder Cell Editing
-
-- (void)editFolderNameInCell:(BookmarkTreeBrowserCell*)cell {
-  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
-  [cell setEditable:YES];
-  [cell setTarget:self];
-  [cell setAction:@selector(cellEditingCompleted:)];
-  [cell setSendsActionOnEndEditing:YES];
-  NSMatrix* matrix = [cell matrix];
-  // Set the delegate so that we get called when editing wants to complete.
-  [matrix setDelegate:self];
-  [matrix selectText:self];
-}
-
-- (void)cellEditingCompleted:(id)sender {
-  DCHECK([sender isKindOfClass:[NSMatrix class]]);
-  BookmarkTreeBrowserCell* cell = [sender selectedCell];
-  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
-  [self saveFolderNameForCell:cell];
-}
-
-- (void)saveFolderNameForCell:(BookmarkTreeBrowserCell*)cell {
-  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
-  // It's possible that the cell can get reused so clean things up
-  // to prevent inadvertant notifications.
-  [cell setTarget:nil];
-  [cell setAction:nil];
-  [cell setEditable:NO];
-  [cell setSendsActionOnEndEditing:NO];
-  // Force a responder change here to force the editing of the cell's text
-  // to complete otherwise the call to -[cell title] could return stale text.
-  [[folderBrowser_ window] makeFirstResponder:folderBrowser_];
-  const BookmarkNode* bookmarkNode = [cell bookmarkNode];
-  BookmarkModel* model = profile_->GetBookmarkModel();
-  NSString* newTitle = [cell title];
-  model->SetTitle(bookmarkNode, base::SysNSStringToWide(newTitle));
-}
-
-- (void)browserDoubleClicked:(id)sender {
-  BookmarkTreeBrowserCell* cell = [folderBrowser_ selectedCell];
-  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
-  [self editFolderNameInCell:cell];
-}
-
-- (IBAction)newFolder:(id)sender {
-  BookmarkModel* model = profile_->GetBookmarkModel();
-  const BookmarkNode* newParentNode = [self selectedNode];
-  int newIndex = newParentNode->GetChildCount();
-  std::wstring newFolderString =
-      l10n_util::GetString(IDS_BOOMARK_EDITOR_NEW_FOLDER_NAME);
-  const BookmarkNode* newFolder = model->AddGroup(newParentNode, newIndex,
-                                                  newFolderString);
-  [self selectNodeInBrowser:newFolder];
-  BookmarkTreeBrowserCell* cell = [folderBrowser_ selectedCell];
-  [self editFolderNameInCell:cell];
+  [self setDisplayURL:initialUrl_];
+  [super awakeFromNib];
 }
 
 #pragma mark Bookmark Editing
 
 // If possible, return a valid GURL from the URL text field.
 - (GURL)GURLFromUrlField {
-  NSString* url = [urlField_ stringValue];
+  NSString* url = [self displayURL];
   GURL newURL = GURL([url UTF8String]);
   if (!newURL.is_valid()) {
     // Mimic observed friendliness from Windows
@@ -323,25 +73,28 @@ int IndexOfFolderChild(const BookmarkNode* child_node) {
   return newURL;
 }
 
-// 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];
-  [okButton_ setEnabled:(newURL.is_valid()) ? YES : NO];
+// Enable the OK button if there is a valid URL.
+- (BOOL)okEnabled {
+  BOOL okEnabled = NO;
+  if ([[self displayURL] length]) {
+    GURL newURL = [self GURLFromUrlField];
+    okEnabled = (newURL.is_valid()) ? YES : NO;
+  }
+  return okEnabled;
 }
 
-// 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.
+// The the bookmark's URL is 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* name = [[self displayName] stringByTrimmingCharactersInSet:
+                    [NSCharacterSet newlineCharacterSet]];
   std::wstring newTitle = base::SysNSStringToWide(name);
+  const BookmarkNode* newParentNode = [self selectedNode];
+  BookmarkModel* model = [self bookmarkModel];
+  int newIndex = newParentNode->GetChildCount();
   GURL newURL = [self GURLFromUrlField];
   if (!newURL.is_valid()) {
     // Shouldn't be reached -- OK button disabled if not valid!
@@ -350,114 +103,21 @@ int IndexOfFolderChild(const BookmarkNode* child_node) {
   }
 
   // Determine where the new/replacement bookmark is to go.
-  const BookmarkNode* newParentNode = [self selectedNode];
-  BookmarkModel* model = profile_->GetBookmarkModel();
-  int newIndex = newParentNode->GetChildCount();
-  if (node_ && parentNode_) {
+  const BookmarkNode* parentNode = [self parentNode];
+  if (node_ && parentNode) {
     // Replace the old bookmark with the updated bookmark.
-    int oldIndex = parentNode_->IndexOfChild(node_);
+    int oldIndex = parentNode->IndexOfChild(node_);
     if (oldIndex >= 0)
-      model->Remove(parentNode_, oldIndex);
-    if (parentNode_ == newParentNode)
+      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]];
-}
-
-- (IBAction)cancel:(id)sender {
-  [NSApp endSheet:[self window]];
-}
-
-- (void)didEndSheet:(NSWindow*)sheet
-         returnCode:(int)returnCode
-        contextInfo:(void*)contextInfo {
-  [sheet close];
-}
-
-#pragma mark For Unit Test Use Only
-
-- (NSString*)displayName {
-  return [nameField_ stringValue];
-}
-
-- (NSString*)displayURL {
-  return [urlField_ stringValue];
-}
-
-- (void)setDisplayName:(NSString*)name {
-  [nameField_ setStringValue:name];
-  [self controlTextDidChange:nil];
-}
-
-- (void)setDisplayURL:(NSString*)name {
-  [urlField_ setStringValue:name];
-  [self controlTextDidChange:nil];
-}
-
-- (BOOL)okButtonEnabled {
-  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 = [folderBrowser_ selectedRowInColumn:i];
-    node = GetFolderChildForParent(node, 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);
-  DCHECK([cell isKindOfClass:[BookmarkTreeBrowserCell class]]);
-  const BookmarkNode* parentNode = [self parentNodeForColumn:column];
-  const BookmarkNode* childNode = GetFolderChildForParent(parentNode, row);
-  DCHECK(childNode);
-  BookmarkTreeBrowserCell* browserCell =
-      static_cast<BookmarkTreeBrowserCell*>(cell);
-  [browserCell setTitle:base::SysWideToNSString(childNode->GetTitle())];
-  [browserCell setBookmarkNode:childNode];
-  [browserCell setMatrix:[folderBrowser_ matrixInColumn:column]];
+  [self NotifyHandlerCreatedNode:node];
+  [super ok:sender];
 }
 
 @end  // BookmarkEditorController
diff --git a/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm b/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
index e8d06d5..db340b1 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller_unittest.mm
@@ -183,9 +183,9 @@ class BookmarkEditorControllerTreeTest : public CocoaTest {
     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_, 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_, 2, L"b-1", 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");
@@ -365,4 +365,3 @@ TEST_F(BookmarkEditorControllerTreeNoParentTest, AddFolderWithNoGroupSelected) {
   const BookmarkNode* folderChild = bookmarkBar->GetChild(4);
   EXPECT_EQ(folderChild->GetTitle(), L"New folder");
 }
-