[Mac] Fix beta-blocking bugs in the cookies manager

* Auto-expand the Cookies folder after expanding a domain-level folder
* XIB change: disable multiple selection (and removal) of cookies to prevent a crash
* Auto-select the next cookie after deletion

BUG=31678,32002,32022

TEST=Open cookies manager. Expand a domain and Cookies folder should auto-expand.
TEST=Open cookies manager. Should not be able to perform multiple selection or deletion.
TEST=Open cookies manager. Expand domain with multiple cookies. Delete the first cookie. Second cookie should now be selected.

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

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

3 files changed, 102 insertions, 4 deletions

diff --git a/chrome/app/nibs/Cookies.xib b/chrome/app/nibs/Cookies.xib
index 9f37c85..5904961c 100644
--- a/chrome/app/nibs/Cookies.xib
+++ b/chrome/app/nibs/Cookies.xib
@@ -172,7 +172,7 @@
 												</object>
 											</object>
 											<double key="NSRowHeight">17</double>
-											<int key="NSTvFlags">-633339904</int>
+											<int key="NSTvFlags">-767557632</int>
 											<reference key="NSDelegate"/>
 											<reference key="NSDataSource"/>
 											<int key="NSColumnAutoresizingStyle">4</int>
diff --git a/chrome/browser/cocoa/cookies_window_controller.mm b/chrome/browser/cocoa/cookies_window_controller.mm
index 41ba73f..25d4120 100644
--- a/chrome/browser/cocoa/cookies_window_controller.mm
+++ b/chrome/browser/cocoa/cookies_window_controller.mm
@@ -184,11 +184,19 @@ CocoaCookieTreeNode* CookiesTreeModelObserverBridge::FindCocoaNode(
 }
 
 - (IBAction)deleteCookie:(id)sender {
-  scoped_nsobject<NSArray> selection(
-      [[treeController_ selectedObjects] retain]);
-  for (CocoaCookieTreeNode* node in selection.get()) {
+  NSIndexPath* selectionPath = [treeController_ selectionIndexPath];
+  // N.B.: I suspect that |-selectedObjects| does not retain/autorelease the
+  // return value, which may result in the pointer going to garbage before it
+  // even goes out of scope. Retaining it immediately will fix this.
+  NSArray* selection = [treeController_ selectedObjects];
+  if (selectionPath) {
+    DCHECK_EQ([selection count], 1U);
+    CocoaCookieTreeNode* node = [selection lastObject];
     CookieTreeNode* cookie = static_cast<CookieTreeNode*>([node treeNode]);
     treeModel_->DeleteCookieNode(cookie);
+    // If there is a next cookie, this will select it because items will slide
+    // up. If there is no next cookie, this is a no-op.
+    [treeController_ setSelectionIndexPath:selectionPath];
   }
 }
 
@@ -229,6 +237,24 @@ CocoaCookieTreeNode* CookiesTreeModelObserverBridge::FindCocoaNode(
   [(ImageAndTextCell*)cell setImage:icon];
 }
 
+- (void)outlineViewItemDidExpand:(NSNotification*)notif {
+  NSTreeNode* item = [[notif userInfo] objectForKey:@"NSObject"];
+  CocoaCookieTreeNode* node = [item representedObject];
+  NSArray* children = [node children];
+  if ([children count] == 1U) {
+    // The node that will expand has one child. Do the user a favor and expand
+    // that node (saving her a click) if it is non-leaf.
+    CocoaCookieTreeNode* child = [children lastObject];
+    if (![child isLeaf]) {
+      NSOutlineView* outlineView = [notif object];
+      // Tell the OutlineView to expand the NSTreeNode, not the model object.
+      children = [item childNodes];
+      DCHECK_EQ([children count], 1U);
+      [outlineView expandItem:[children lastObject]];
+    }
+  }
+}
+
 #pragma mark Unit Testing
 
 - (CookiesTreeModelObserverBridge*)modelObserver {
diff --git a/chrome/browser/cocoa/cookies_window_controller_unittest.mm b/chrome/browser/cocoa/cookies_window_controller_unittest.mm
index 38b278d..0c8cb58 100644
--- a/chrome/browser/cocoa/cookies_window_controller_unittest.mm
+++ b/chrome/browser/cocoa/cookies_window_controller_unittest.mm
@@ -17,6 +17,7 @@
 #include "net/url_request/url_request_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
+#import "third_party/ocmock/OCMock/OCMock.h"
 
 // Used to test FindCocoaNode. This only sets the title and node, without
 // initializing any other members.
@@ -335,4 +336,75 @@ TEST_F(CookiesWindowControllerTest, TreeNodeChanged) {
   EXPECT_TRUE([@"Silly Change" isEqualToString:[cocoa_node title]]);
 }
 
+TEST_F(CookiesWindowControllerTest, TestDeleteCookie) {
+  const GURL url = GURL("http://foo.com");
+  TestingProfile* profile = browser_helper_.profile();
+  net::CookieMonster* cm = profile->GetCookieMonster();
+  cm->SetCookie(url, "A=B");
+  cm->SetCookie(url, "C=D");
+
+  // This will clean itself up when we call |-closeSheet:|. If we reset the
+  // scoper, we'd get a double-free.
+  CookiesWindowController* controller =
+      [[CookiesWindowController alloc] initWithProfile:profile];
+  [controller attachSheetTo:test_window()];
+  NSTreeController* treeController = [controller treeController];
+
+  // Pretend to select cookie A.
+  NSUInteger path[3] = {0, 0, 0};
+  NSIndexPath* indexPath = [NSIndexPath indexPathWithIndexes:path length:3];
+  [treeController setSelectionIndexPath:indexPath];
+
+  // Press the "Delete" button.
+  [controller deleteCookie:nil];
+
+  // Root --> foo.com --> Cookies.
+  NSArray* cookies = [[[[[[controller cocoaTreeModel] childs] objectAtIndex:0]
+      childs] objectAtIndex:0] childs];
+  EXPECT_EQ(1U, [cookies count]);
+  EXPECT_TRUE([@"C" isEqualToString:[[cookies lastObject] title]]);
+  EXPECT_TRUE([indexPath isEqual:[treeController selectionIndexPath]]);
+
+  [controller closeSheet:nil];
+}
+
+TEST_F(CookiesWindowControllerTest, TestDidExpandItem) {
+  const GURL url = GURL("http://foo.com");
+  TestingProfile* profile = browser_helper_.profile();
+  net::CookieMonster* cm = profile->GetCookieMonster();
+  cm->SetCookie(url, "A=B");
+  cm->SetCookie(url, "C=D");
+
+  controller_.reset(
+      [[CookiesWindowController alloc] initWithProfile:profile]);
+
+  // Root --> foo.com.
+  CocoaCookieTreeNode* foo =
+      [[[controller_ cocoaTreeModel] childs] objectAtIndex:0];
+
+  // Create the objects we are going to be testing with.
+  id outlineView = [OCMockObject mockForClass:[NSOutlineView class]];
+  id treeNode = [OCMockObject mockForClass:[NSTreeNode class]];
+  NSTreeNode* childTreeNode =
+      [NSTreeNode treeNodeWithRepresentedObject:[[foo childs] lastObject]];
+  NSArray* fakeChildren = [NSArray arrayWithObject:childTreeNode];
+
+  // Set up the mock object.
+  [[[treeNode stub] andReturn:foo] representedObject];
+  [[[treeNode stub] andReturn:fakeChildren] childNodes];
+
+  // Create a fake "ItemDidExpand" notification.
+  NSDictionary* userInfo = [NSDictionary dictionaryWithObject:treeNode
+                                                       forKey:@"NSObject"];
+  NSNotification* notif =
+      [NSNotification notificationWithName:@"ItemDidExpandNotification"
+                                    object:outlineView
+                                  userInfo:userInfo];
+
+  // Make sure we work correctly.
+  [[outlineView expect] expandItem:childTreeNode];
+  [controller_ outlineViewItemDidExpand:notif];
+  [outlineView verify];
+}
+
 }  // namespace