[Mac] Show group separators in the keyword editor

BUG=23570,21640
TEST=Chromium-->Preferences-->Manage. (1) See "Default" group. (2) Add an engine and it appears under an "Other" group header. (3) Make that new engine default and it moves under the "Default" group and the "Other" header disappears.

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

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

3 files changed, 167 insertions, 20 deletions

diff --git a/chrome/browser/cocoa/keyword_editor_cocoa_controller.h b/chrome/browser/cocoa/keyword_editor_cocoa_controller.h
index 6aac616..105e76c 100644
--- a/chrome/browser/cocoa/keyword_editor_cocoa_controller.h
+++ b/chrome/browser/cocoa/keyword_editor_cocoa_controller.h
@@ -64,6 +64,8 @@ class KeywordEditorModelObserver : public TemplateURLModelObserver,
   IBOutlet NSButton* removeButton_;
   IBOutlet NSButton* makeDefaultButton_;
 
+  scoped_nsobject<NSTextFieldCell> groupCell_;
+
   Profile* profile_;  // weak
   scoped_ptr<KeywordEditorController> controller_;
   scoped_ptr<KeywordEditorModelObserver> observer_;
@@ -96,4 +98,8 @@ class KeywordEditorModelObserver : public TemplateURLModelObserver,
 // or nil if there is none.
 + (KeywordEditorCocoaController*)sharedInstanceForProfile:(Profile*)profile;
 
+// Converts a row index in our table view (which has group header rows) into
+// one in the |controller_|'s model, which does not have them.
+- (int)indexInModelForRow:(NSUInteger)row;
+
 @end
diff --git a/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm b/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm
index 4f20b37..37ffdf3 100644
--- a/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm
+++ b/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm
@@ -22,6 +22,7 @@
 @interface KeywordEditorCocoaController (Private)
 - (void)adjustEditingButtons;
 - (void)editKeyword:(id)sender;
+- (int)indexInModelForRow:(NSUInteger)row;
 @end
 
 // KeywordEditorModelObserver -------------------------------------------------
@@ -176,6 +177,7 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
     observer_.reset(new KeywordEditorModelObserver(self));
     controller_->table_model()->SetObserver(observer_.get());
     controller_->url_model()->AddObserver(observer_.get());
+    groupCell_.reset([[NSTextFieldCell alloc] init]);
   }
   return self;
 }
@@ -224,8 +226,7 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
   }
 }
 
-// The last page info window that was moved will determine the location of the
-// next new one.
+// Remeber the position of the keyword editor.
 - (void)windowDidMove:(NSNotification*)notif {
   if (g_browser_process && g_browser_process->local_state()) {
     NSWindow* window = [self window];
@@ -264,9 +265,10 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
 
 - (void)editKeyword:(id)sender {
   const NSInteger clickedRow = [tableView_ clickedRow];
-  if (clickedRow == -1)
+  if (clickedRow < 0 || [self tableView:tableView_ isGroupRow:clickedRow])
     return;
-  const TemplateURL* url = controller_->GetTemplateURL(clickedRow);
+  const TemplateURL* url = controller_->GetTemplateURL(
+      [self indexInModelForRow:clickedRow]);
   // The controller will release itself when the window closes.
   EditSearchEngineCocoaController* editor =
       [[EditSearchEngineCocoaController alloc] initWithProfile:profile_
@@ -284,7 +286,7 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
   DCHECK_GT([selection count], 0U);
   NSUInteger index = [selection lastIndex];
   while (index != NSNotFound) {
-    controller_->RemoveTemplateURL(index);
+    controller_->RemoveTemplateURL([self indexInModelForRow:index]);
     index = [selection indexLessThanIndex:index];
   }
 }
@@ -292,33 +294,50 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
 - (IBAction)makeDefault:(id)sender {
   NSIndexSet* selection = [tableView_ selectedRowIndexes];
   DCHECK_EQ([selection count], 1U);
-  controller_->MakeDefaultTemplateURL([selection firstIndex]);
+  int row = [self indexInModelForRow:[selection firstIndex]];
+  controller_->MakeDefaultTemplateURL(row);
 }
 
 // Table View Data Source -----------------------------------------------------
 
-- (NSInteger)numberOfRowsInTableView:(NSTableView*)aTableView {
-  return controller_->table_model()->RowCount();
+- (NSInteger)numberOfRowsInTableView:(NSTableView*)table {
+  int rowCount = controller_->table_model()->RowCount();
+  int numGroups = controller_->table_model()->GetGroups().size();
+  if ([self tableView:table isGroupRow:rowCount + numGroups - 1]) {
+    // Don't show a group header with no rows underneath it.
+    --numGroups;
+  }
+  return rowCount + numGroups;
 }
 
 - (id)tableView:(NSTableView*)tv
     objectValueForTableColumn:(NSTableColumn*)tableColumn
                           row:(NSInteger)row {
-  if (!tableColumn)
-    return nil;
-  NSString* identifier = [tableColumn identifier];
+  if ([self tableView:tv isGroupRow:row]) {
+    DCHECK(!tableColumn);
+    TableModel::Groups groups = controller_->table_model()->GetGroups();
+    if (row == 0) {
+      return base::SysWideToNSString(groups[0].title);
+    } else {
+      return base::SysWideToNSString(groups[1].title);
+    }
+  }
 
+  NSString* identifier = [tableColumn identifier];
   if ([identifier isEqualToString:@"name"]) {
     // The name column is an NSButtonCell so we can have text and image in the
     // same cell. As such, the "object value" for a button cell is either on
     // or off, so we always return off so we don't act like a button.
     return [NSNumber numberWithInt:NSOffState];
-  } else if ([identifier isEqualToString:@"keyword"]) {
+  }
+  if ([identifier isEqualToString:@"keyword"]) {
     // The keyword object value is a normal string.
+    int index = [self indexInModelForRow:row];
     int columnID = IDS_SEARCH_ENGINES_EDITOR_KEYWORD_COLUMN;
-    std::wstring text = controller_->table_model()->GetText(row, columnID);
+    std::wstring text = controller_->table_model()->GetText(index, columnID);
     return base::SysWideToNSString(text);
   }
+
   // And we shouldn't have any other columns...
   NOTREACHED();
   return nil;
@@ -331,20 +350,39 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
   [self adjustEditingButtons];
 }
 
+// Disallow selection of the group header rows.
+- (BOOL)tableView:(NSTableView*)table shouldSelectRow:(NSInteger)row {
+  return ![self tableView:table isGroupRow:row];
+}
+
+- (BOOL)tableView:(NSTableView*)table isGroupRow:(NSInteger)row {
+  int otherGroupRow =
+      controller_->table_model()->last_search_engine_index() + 1;
+  return (row == 0 || row == otherGroupRow);
+}
+
 - (NSCell*)tableView:(NSTableView*)tableView
     dataCellForTableColumn:(NSTableColumn*)tableColumn
                        row:(NSInteger)row {
   static const CGFloat kCellFontSize = 12.0;
+
+  // Check to see if we are a grouped row.
+  if ([self tableView:tableView isGroupRow:row]) {
+    DCHECK(!tableColumn);  // This would violate the group row contract.
+    return groupCell_.get();
+  }
+
   NSCell* cell = [tableColumn dataCellForRow:row];
+  int offsetRow = [self indexInModelForRow:row];
 
   // Set the favicon and title for the search engine in the name column.
   if ([[tableColumn identifier] isEqualToString:@"name"]) {
     DCHECK([cell isKindOfClass:[NSButtonCell class]]);
     NSButtonCell* buttonCell = static_cast<NSButtonCell*>(cell);
-    std::wstring title = controller_->table_model()->GetText(row,
+    std::wstring title = controller_->table_model()->GetText(offsetRow,
         IDS_SEARCH_ENGINES_EDITOR_DESCRIPTION_COLUMN);
     [buttonCell setTitle:base::SysWideToNSString(title)];
-    [buttonCell setImage:observer_->GetImageForRow(row)];
+    [buttonCell setImage:observer_->GetImageForRow(offsetRow)];
     [buttonCell setRefusesFirstResponder:YES];  // Don't push in like a button.
     [buttonCell setHighlightsBy:NSNoCellMask];
   }
@@ -352,7 +390,8 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
   // The default search engine should be in bold font.
   const TemplateURL* defaultEngine =
       controller_->url_model()->GetDefaultSearchProvider();
-  if (controller_->table_model()->IndexOfTemplateURL(defaultEngine) == row) {
+  int rowIndex = controller_->table_model()->IndexOfTemplateURL(defaultEngine);
+  if (rowIndex == offsetRow) {
     [cell setFont:[NSFont boldSystemFontOfSize:kCellFontSize]];
   } else {
     [cell setFont:[NSFont systemFontOfSize:kCellFontSize]];
@@ -365,13 +404,15 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
 // This function appropriately sets the enabled states on the table's editing
 // buttons.
 - (void)adjustEditingButtons {
-  // Delete button.
   NSIndexSet* selection = [tableView_ selectedRowIndexes];
   BOOL canRemove = ([selection count] > 0);
   NSUInteger index = [selection firstIndex];
+
+  // Delete button.
   while (canRemove && index != NSNotFound) {
+    int modelIndex = [self indexInModelForRow:index];
     const TemplateURL& url =
-        controller_->table_model()->GetTemplateURL(index);
+        controller_->table_model()->GetTemplateURL(modelIndex);
     if (!controller_->CanRemove(&url))
       canRemove = NO;
     index = [selection indexGreaterThanIndex:index];
@@ -382,10 +423,25 @@ typedef std::map<Profile*,KeywordEditorCocoaController*> ProfileControllerMap;
   if ([selection count] != 1) {
     [makeDefaultButton_ setEnabled:NO];
   } else {
+    int row = [self indexInModelForRow:[selection firstIndex]];
     const TemplateURL& url =
-        controller_->table_model()->GetTemplateURL([selection firstIndex]);
+        controller_->table_model()->GetTemplateURL(row);
     [makeDefaultButton_ setEnabled:controller_->CanMakeDefault(&url)];
   }
 }
 
+// This converts a row index in our table view to an index in the model by
+// computing the group offsets.
+- (int)indexInModelForRow:(NSUInteger)row {
+  DCHECK_GT(row, 0U);
+  unsigned otherGroupId =
+      controller_->table_model()->last_search_engine_index() + 1;
+  DCHECK_NE(row, otherGroupId);
+  if (row >= otherGroupId) {
+    return row - 2;  // Other group.
+  } else {
+    return row - 1;  // Default group.
+  }
+}
+
 @end
diff --git a/chrome/browser/cocoa/keyword_editor_cocoa_controller_unittest.mm b/chrome/browser/cocoa/keyword_editor_cocoa_controller_unittest.mm
index e551175..aad19ed 100644
--- a/chrome/browser/cocoa/keyword_editor_cocoa_controller_unittest.mm
+++ b/chrome/browser/cocoa/keyword_editor_cocoa_controller_unittest.mm
@@ -35,6 +35,10 @@
   return observer_.get();
 }
 
+- (NSTableView*)tableView {
+  return tableView_;
+}
+
 @end
 
 // TODO(rsesek): Figure out a good way to test this class (crbug.com/21640).
@@ -49,7 +53,8 @@ class KeywordEditorCocoaControllerTest : public CocoaTest {
         static_cast<TestingProfile*>(browser_helper_.profile());
     profile->CreateTemplateURLModel();
 
-    controller_ = [[FakeKeywordEditorController alloc] initWithProfile:profile];
+    controller_ =
+        [[FakeKeywordEditorController alloc] initWithProfile:profile];
   }
 
   virtual void TearDown() {
@@ -132,4 +137,84 @@ TEST_F(KeywordEditorCocoaControllerTest, ShowKeywordEditor) {
   [newSharedInstance close];
 }
 
+TEST_F(KeywordEditorCocoaControllerTest, IndexInModelForRowMixed) {
+  [controller_ window];  // Force |-awakeFromNib|.
+  TemplateURLModel* template_model = [controller_ controller]->url_model();
+
+  // Add a default engine.
+  TemplateURL* t_url = new TemplateURL();
+  t_url->SetURL(L"http://test1/{searchTerms}", 0, 0);
+  t_url->set_keyword(L"test1");
+  t_url->set_short_name(L"Test1");
+  t_url->set_show_in_default_list(true);
+  template_model->Add(t_url);
+
+  // Add a non-default engine.
+  t_url = new TemplateURL();
+  t_url->SetURL(L"http://test2/{searchTerms}", 0, 0);
+  t_url->set_keyword(L"test2");
+  t_url->set_short_name(L"Test2");
+  t_url->set_show_in_default_list(false);
+  template_model->Add(t_url);
+
+  // Two headers with a single row underneath each.
+  NSTableView* table = [controller_ tableView];
+  [table reloadData];
+  ASSERT_EQ(4, [[controller_ tableView] numberOfRows]);
+
+  // Index 0 is the group header, index 1 should be the first engine.
+  ASSERT_EQ(0, [controller_ indexInModelForRow:1]);
+
+  // Index 2 should be the group header, so index 3 should be the non-default
+  // engine.
+  ASSERT_EQ(1, [controller_ indexInModelForRow:3]);
+
+  ASSERT_TRUE([controller_ tableView:table isGroupRow:0]);
+  ASSERT_FALSE([controller_ tableView:table isGroupRow:1]);
+  ASSERT_TRUE([controller_ tableView:table isGroupRow:2]);
+  ASSERT_FALSE([controller_ tableView:table isGroupRow:3]);
+
+  ASSERT_FALSE([controller_ tableView:table shouldSelectRow:0]);
+  ASSERT_TRUE([controller_ tableView:table shouldSelectRow:1]);
+  ASSERT_FALSE([controller_ tableView:table shouldSelectRow:2]);
+  ASSERT_TRUE([controller_ tableView:table shouldSelectRow:3]);
+}
+
+TEST_F(KeywordEditorCocoaControllerTest, IndexInModelForDefault) {
+  [controller_ window];  // Force |-awakeFromNib|.
+  TemplateURLModel* template_model = [controller_ controller]->url_model();
+
+  // Add 2 default engines.
+  TemplateURL* t_url = new TemplateURL();
+  t_url->SetURL(L"http://test1/{searchTerms}", 0, 0);
+  t_url->set_keyword(L"test1");
+  t_url->set_short_name(L"Test1");
+  t_url->set_show_in_default_list(true);
+  template_model->Add(t_url);
+
+  t_url = new TemplateURL();
+  t_url->SetURL(L"http://test2/{searchTerms}", 0, 0);
+  t_url->set_keyword(L"test2");
+  t_url->set_short_name(L"Test2");
+  t_url->set_show_in_default_list(true);
+  template_model->Add(t_url);
+
+  // One header and two rows.
+  NSTableView* table = [controller_ tableView];
+  [table reloadData];
+  ASSERT_EQ(3, [[controller_ tableView] numberOfRows]);
+
+  // Index 0 is the group header, index 1 should be the first engine.
+  ASSERT_EQ(0, [controller_ indexInModelForRow:1]);
+  ASSERT_EQ(1, [controller_ indexInModelForRow:2]);
+
+  ASSERT_TRUE([controller_ tableView:table isGroupRow:0]);
+  ASSERT_FALSE([controller_ tableView:table isGroupRow:1]);
+  ASSERT_FALSE([controller_ tableView:table isGroupRow:2]);
+
+  ASSERT_FALSE([controller_ tableView:table shouldSelectRow:0]);
+  ASSERT_TRUE([controller_ tableView:table shouldSelectRow:1]);
+  ASSERT_TRUE([controller_ tableView:table shouldSelectRow:2]);
+}
+
 }  // namespace