Mac: Allow dragging bookmarks to trash

This patch allows users to delete bookmarks by dragging them to the trash icon on the Mac OS dock.

Note: Original patch http://codereview.chromium.org/3342023 proposed by fnepal@gmail.com.

BUG=54727
TEST=Dragged bookmarks to trash. Verified that the bookmark was deleted. Verified that the poof animation was draw centered over the mouse cursor. Verified that the same thing worked for bookmarks inside folders.

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

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

8 files changed, 235 insertions, 17 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index 020e8cf..9c57ee0 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -294,6 +294,9 @@ willAnimateFromState:(bookmarks::VisualState)oldState
 // presenting a new menu.)
 - (void)closeFolderAndStopTrackingMenus;
 
+// Checks if operations such as edit or delete are allowed.
+- (BOOL)canEditBookmark:(const BookmarkNode*)node;
+
 // Actions for manipulating bookmarks.
 // Open a normal bookmark or folder from a button, ...
 - (IBAction)openBookmark:(id)sender;
@@ -346,7 +349,7 @@ willAnimateFromState:(bookmarks::VisualState)oldState
 - (NSMenu*)offTheSideMenu;
 - (NSButton*)offTheSideButton;
 - (BOOL)offTheSideButtonIsHidden;
-- (NSButton*)otherBookmarksButton;
+- (BookmarkButton*)otherBookmarksButton;
 - (BookmarkBarFolderController*)folderController;
 - (id)folderTarget;
 - (int)displayedButtonCount;
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index 7bd2988..f3a59d6 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -506,6 +506,15 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   [self closeAllBookmarkFolders];
 }
 
+- (BOOL)canEditBookmark:(const BookmarkNode*)node {
+  // Don't allow edit/delete of the bar node, or of "Other Bookmarks"
+  if ((node == nil) ||
+      (node == bookmarkModel_->other_node()) ||
+      (node == bookmarkModel_->GetBookmarkBarNode()))
+    return NO;
+  return YES;
+}
+
 #pragma mark Actions
 
 - (IBAction)openBookmark:(id)sender {
@@ -905,10 +914,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
       (action == @selector(deleteBookmark:)) ||
       (action == @selector(cutBookmark:)) ||
       (action == @selector(copyBookmark:))) {
-    // Don't allow edit/delete of the bar node, or of "Other Bookmarks"
-    if ((node == nil) ||
-        (node == bookmarkModel_->other_node()) ||
-        (node == bookmarkModel_->GetBookmarkBarNode())) {
+    if (![self canEditBookmark:node]) {
       return NO;
     }
   }
@@ -1422,7 +1428,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   return [offTheSideButton_ isHidden];
 }
 
-- (NSButton*)otherBookmarksButton {
+- (BookmarkButton*)otherBookmarksButton {
   return otherBookmarksButton_.get();
 }
 
@@ -2004,6 +2010,21 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   return [[self view] window];
 }
 
+- (BOOL)canDragBookmarkButtonToTrash:(BookmarkButton*)button {
+  return [self canEditBookmark:[button bookmarkNode]];
+}
+
+- (void)didDragBookmarkToTrash:(BookmarkButton*)button {
+  // TODO(mrossetti): Refactor BookmarkBarFolder common code.
+  // http://crbug.com/35966
+  const BookmarkNode* node = [button bookmarkNode];
+  if (node) {
+    const BookmarkNode* parent = node->GetParent();
+    bookmarkModel_->Remove(parent,
+                           parent->IndexOfChild(node));
+  }
+}
+
 #pragma mark BookmarkButtonControllerProtocol
 
 // Close all bookmark folders.  "Folder" here is the fake menu for
@@ -2378,10 +2399,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
       // If we are deleting a button whose folder is currently open, close it!
       [self closeAllBookmarkFolders];
     }
-    NSRect poofFrame = [oldButton bounds];
-    NSPoint poofPoint = NSMakePoint(NSMidX(poofFrame), NSMidY(poofFrame));
-    poofPoint = [oldButton convertPoint:poofPoint toView:nil];
-    poofPoint = [[oldButton window] convertBaseToScreen:poofPoint];
+    NSPoint poofPoint = [oldButton screenLocationForRemoveAnimation];
     NSRect oldFrame = [oldButton frame];
     [oldButton setDelegate:nil];
     [oldButton removeFromSuperview];
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index 7c29c00..2a8887f 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -2076,4 +2076,37 @@ TEST_F(BookmarkBarControllerDragDropTest, PulseButton) {
   EXPECT_FALSE([button isContinuousPulsing]);
 }
 
+TEST_F(BookmarkBarControllerDragDropTest, DragBookmarkDataToTrash) {
+  BookmarkModel& model(*helper_.profile()->GetBookmarkModel());
+  const BookmarkNode* root = model.GetBookmarkBarNode();
+  const std::string model_string("1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] "
+                                  "2f3b ] 3b 4b ");
+  model_test_utils::AddNodesFromModelString(model, root, model_string);
+
+  // Validate initial model.
+  std::string actual = model_test_utils::ModelStringFromNode(root);
+  EXPECT_EQ(model_string, actual);
+
+  int oldChildCount = root->GetChildCount();
+
+  // Drag a button to the trash.
+  BookmarkButton* buttonToDelete = [bar_ buttonWithTitleEqualTo:@"3b"];
+  ASSERT_TRUE(buttonToDelete);
+  EXPECT_TRUE([bar_ canDragBookmarkButtonToTrash:buttonToDelete]);
+  [bar_ didDragBookmarkToTrash:buttonToDelete];
+
+  // There should be one less button in the bar.
+  int newChildCount = root->GetChildCount();
+  EXPECT_EQ(oldChildCount - 1, newChildCount);
+  // Verify the model.
+  const std::string expected("1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] "
+                             "2f3b ] 4b ");
+  actual = model_test_utils::ModelStringFromNode(root);
+  EXPECT_EQ(expected, actual);
+
+  // Verify that the other bookmark folder can't be deleted.
+  BookmarkButton *otherButton = [bar_ otherBookmarksButton];
+  EXPECT_FALSE([bar_ canDragBookmarkButtonToTrash:otherButton]);
+}
+
 }  // namespace
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
index c915bc9..0e9e751 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
@@ -1042,6 +1042,21 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   return [parentController_ browserWindow];
 }
 
+- (BOOL)canDragBookmarkButtonToTrash:(BookmarkButton*)button {
+  return [barController_ canEditBookmark:[button bookmarkNode]];
+}
+
+- (void)didDragBookmarkToTrash:(BookmarkButton*)button {
+  // TODO(mrossetti): Refactor BookmarkBarFolder common code.
+  // http://crbug.com/35966
+  const BookmarkNode* node = [button bookmarkNode];
+  if (node) {
+    const BookmarkNode* parent = node->GetParent();
+    [self bookmarkModel]->Remove(parent,
+                                 parent->IndexOfChild(node));
+  }
+}
+
 #pragma mark BookmarkButtonControllerProtocol
 
 // Recursively close all bookmark folders.
@@ -1349,10 +1364,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
 - (void)removeButton:(NSInteger)buttonIndex animate:(BOOL)animate {
   // TODO(mrossetti): Get disappearing animation to work. http://crbug.com/42360
   BookmarkButton* oldButton = [buttons_ objectAtIndex:buttonIndex];
-  NSRect poofFrame = [oldButton bounds];
-  NSPoint poofPoint = NSMakePoint(NSMidX(poofFrame), NSMidY(poofFrame));
-  poofPoint = [oldButton convertPoint:poofPoint toView:nil];
-  poofPoint = [[oldButton window] convertBaseToScreen:poofPoint];
+  NSPoint poofPoint = [oldButton screenLocationForRemoveAnimation];
 
   // If a hover-open is pending, cancel it.
   if (oldButton == buttonThatMouseIsIn_) {
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm
index 3d46a62..6d09ae7 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm
@@ -1289,7 +1289,6 @@ TEST_F(BookmarkBarFolderControllerMenuTest, DragBookmarkData) {
                           initWithParentButton:button
                               parentController:nil
                                  barController:bar_]);
-  [folderController window];
   BookmarkButton* targetButton =
       [folderController buttonWithTitleEqualTo:@"2f1b"];
   ASSERT_TRUE(targetButton);
@@ -1324,6 +1323,45 @@ TEST_F(BookmarkBarFolderControllerMenuTest, DragBookmarkData) {
   EXPECT_EQ(expectedA, actual);
 }
 
+TEST_F(BookmarkBarFolderControllerMenuTest, DragBookmarkDataToTrash) {
+  BookmarkModel& model(*helper_.profile()->GetBookmarkModel());
+  const BookmarkNode* root = model.GetBookmarkBarNode();
+  const std::string model_string("1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] "
+                                 "2f3b ] 3b 4b ");
+  model_test_utils::AddNodesFromModelString(model, root, model_string);
+
+  // Validate initial model.
+  std::string actual = model_test_utils::ModelStringFromNode(root);
+  EXPECT_EQ(model_string, actual);
+
+  const BookmarkNode* folderNode = root->GetChild(1);
+  int oldFolderChildCount = folderNode->GetChildCount();
+
+  // Pop open a folder.
+  BookmarkButton* button = [bar_ buttonWithTitleEqualTo:@"2f"];
+  scoped_nsobject<BookmarkBarFolderControllerDragData> folderController;
+  folderController.reset([[BookmarkBarFolderControllerDragData alloc]
+                          initWithParentButton:button
+                              parentController:nil
+                                 barController:bar_]);
+
+  // Drag a button to the trash.
+  BookmarkButton* buttonToDelete =
+      [folderController buttonWithTitleEqualTo:@"2f1b"];
+  ASSERT_TRUE(buttonToDelete);
+  EXPECT_TRUE([folderController canDragBookmarkButtonToTrash:buttonToDelete]);
+  [folderController didDragBookmarkToTrash:buttonToDelete];
+
+  // There should be one less button in the folder.
+  int newFolderChildCount = folderNode->GetChildCount();
+  EXPECT_EQ(oldFolderChildCount - 1, newFolderChildCount);
+  // Verify the model.
+  const std::string expected("1b 2f:[ 2f2f:[ 2f2f1b 2f2f2b 2f2f3b ] "
+                             "2f3b ] 3b 4b ");
+  actual = model_test_utils::ModelStringFromNode(root);
+  EXPECT_EQ(expected, actual);
+}
+
 TEST_F(BookmarkBarFolderControllerMenuTest, AddURLs) {
   BookmarkModel& model(*helper_.profile()->GetBookmarkModel());
   const BookmarkNode* root = model.GetBookmarkBarNode();
diff --git a/chrome/browser/cocoa/bookmark_button.h b/chrome/browser/cocoa/bookmark_button.h
index 3459809..56d9bc1 100644
--- a/chrome/browser/cocoa/bookmark_button.h
+++ b/chrome/browser/cocoa/bookmark_button.h
@@ -40,6 +40,13 @@ class ThemeProvider;
 // Returns the top-level window for this button.
 - (NSWindow*)browserWindow;
 
+// Returns YES if the bookmark button can be dragged to the trash, NO otherwise.
+- (BOOL)canDragBookmarkButtonToTrash:(BookmarkButton*)button;
+
+// This is called after the user has dropped the bookmark button on the trash.
+// The delegate can use this event to delete the bookmark.
+- (void)didDragBookmarkToTrash:(BookmarkButton*)button;
+
 @end
 
 
@@ -180,6 +187,10 @@ class ThemeProvider;
   // different window), so there is no way to retrieve the same BWC object after
   // a drag.
   BrowserWindowController* visibilityDelegate_;  // weak
+
+  NSPoint dragMouseOffset_;
+  NSPoint dragEndScreenLocation_;
+  BOOL dragPending_;
 }
 
 @property(assign, nonatomic) NSObject<BookmarkButtonDelegate>* delegate;
@@ -205,6 +216,10 @@ class ThemeProvider;
 // Return continuous pulse state.
 - (BOOL)isContinuousPulsing;
 
+// Return the location in screen coordinates where the remove animation should
+// be displayed.
+- (NSPoint)screenLocationForRemoveAnimation;
+
 @end  // @interface BookmarkButton
 
 
diff --git a/chrome/browser/cocoa/bookmark_button.mm b/chrome/browser/cocoa/bookmark_button.mm
index 1a72bda..6c773d6 100644
--- a/chrome/browser/cocoa/bookmark_button.mm
+++ b/chrome/browser/cocoa/bookmark_button.mm
@@ -73,6 +73,29 @@ NSString* const kBookmarkPulseFlagKey = @"BookmarkPulseFlagKey";
   return [[self cell] isContinuousPulsing];
 }
 
+- (NSPoint)screenLocationForRemoveAnimation {
+  NSPoint point;
+
+  if (dragPending_) {
+    // Use the position of the mouse in the drag image as the location.
+    point = dragEndScreenLocation_;
+    point.x += dragMouseOffset_.x;
+    if ([self isFlipped]) {
+      point.y += [self bounds].size.height - dragMouseOffset_.y;
+    } else {
+      point.y += dragMouseOffset_.y;
+    }
+  } else {
+    // Use the middle of this button as the location.
+    NSRect bounds = [self bounds];
+    point = NSMakePoint(NSMidX(bounds), NSMidY(bounds));
+    point = [self convertPoint:point toView:nil];
+    point = [[self window] convertBaseToScreen:point];
+  }
+
+  return point;
+}
+
 // By default, NSButton ignores middle-clicks.
 // But we want them.
 - (void)otherMouseUp:(NSEvent*)event {
@@ -114,12 +137,16 @@ NSString* const kBookmarkPulseFlagKey = @"BookmarkPulseFlagKey";
         isWithinFolder ? "BookmarkBarFolder_DragStart" :
             "BookmarkBar_DragStart"));
 
+    dragMouseOffset_ = [self convertPointFromBase:[event locationInWindow]];
+    dragPending_ = YES;
+
     CGFloat yAt = [self bounds].size.height;
     NSSize dragOffset = NSMakeSize(0.0, 0.0);
     [self dragImage:[self dragImage] at:NSMakePoint(0, yAt) offset:dragOffset
               event:event pasteboard:pboard source:self slideBack:YES];
 
     // And we're done.
+    dragPending_ = NO;
     [self autorelease];
   } else {
     // Avoid blowing up, but we really shouldn't get here.
@@ -138,8 +165,23 @@ NSString* const kBookmarkPulseFlagKey = @"BookmarkPulseFlagKey";
 }
 
 - (NSDragOperation)draggingSourceOperationMaskForLocal:(BOOL)isLocal {
-  return isLocal ? NSDragOperationCopy | NSDragOperationMove
-                 : NSDragOperationCopy;
+  NSDragOperation operation = NSDragOperationCopy;
+  if (isLocal) {
+    operation |= NSDragOperationMove;
+  }
+  if ([delegate_ canDragBookmarkButtonToTrash:self]) {
+    operation |= NSDragOperationDelete;
+  }
+  return operation;
+}
+
+- (void)draggedImage:(NSImage *)anImage
+             endedAt:(NSPoint)aPoint
+           operation:(NSDragOperation)operation {
+  if (operation & NSDragOperationDelete) {
+    dragEndScreenLocation_ = aPoint;
+    [delegate_ didDragBookmarkToTrash:self];
+  }
 }
 
 // mouseEntered: and mouseExited: are called from our
diff --git a/chrome/browser/cocoa/bookmark_button_unittest.mm b/chrome/browser/cocoa/bookmark_button_unittest.mm
index 419be3d..59cb39e 100644
--- a/chrome/browser/cocoa/bookmark_button_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_button_unittest.mm
@@ -18,6 +18,8 @@
  @public
   int entered_;
   int exited_;
+  BOOL canDragToTrash_;
+  int didDragToTrashCount_;
 }
 @end
 
@@ -42,6 +44,15 @@
 - (NSWindow*)browserWindow {
   return nil;
 }
+
+- (BOOL)canDragBookmarkButtonToTrash:(BookmarkButton*)button {
+  return canDragToTrash_;
+}
+
+- (void)didDragBookmarkToTrash:(BookmarkButton*)button {
+  didDragToTrashCount_++;
+}
+
 @end
 
 namespace {
@@ -114,4 +125,50 @@ TEST_F(BookmarkButtonTest, MouseEnterExitRedirect) {
   EXPECT_EQ(2, delegate.get()->exited_);
 }
 
+TEST_F(BookmarkButtonTest, DragToTrash) {
+  BrowserTestHelper helper_;
+
+  scoped_nsobject<BookmarkButton> button;
+  scoped_nsobject<BookmarkButtonCell> cell;
+  scoped_nsobject<FakeButtonDelegate>
+      delegate([[FakeButtonDelegate alloc] init]);
+  button.reset([[BookmarkButton alloc] initWithFrame:NSMakeRect(0,0,500,500)]);
+  cell.reset([[BookmarkButtonCell alloc] initTextCell:@"hi mom"]);
+  [button setCell:cell];
+  [button setDelegate:delegate];
+
+  // Add a deletable bookmark to the button.
+  BookmarkModel* model = helper_.profile()->GetBookmarkModel();
+  const BookmarkNode* barNode = model->GetBookmarkBarNode();
+  const BookmarkNode* node = model->AddURL(barNode, 0, ASCIIToUTF16("hi mom"),
+                                           GURL("http://www.google.com"));
+  [cell setBookmarkNode:node];
+
+  // Verify that if canDragBookmarkButtonToTrash is NO then the button can't
+  // be dragged to the trash.
+  delegate.get()->canDragToTrash_ = NO;
+  NSDragOperation operation = [button draggingSourceOperationMaskForLocal:NO];
+  EXPECT_EQ(0u, operation & NSDragOperationDelete);
+  operation = [button draggingSourceOperationMaskForLocal:YES];
+  EXPECT_EQ(0u, operation & NSDragOperationDelete);
+
+  // Verify that if canDragBookmarkButtonToTrash is YES then the button can
+  // be dragged to the trash.
+  delegate.get()->canDragToTrash_ = YES;
+  operation = [button draggingSourceOperationMaskForLocal:NO];
+  EXPECT_EQ(NSDragOperationDelete, operation & NSDragOperationDelete);
+  operation = [button draggingSourceOperationMaskForLocal:YES];
+  EXPECT_EQ(NSDragOperationDelete, operation & NSDragOperationDelete);
+
+  // Verify that canDragBookmarkButtonToTrash is called when expected.
+  delegate.get()->canDragToTrash_ = YES;
+  EXPECT_EQ(0, delegate.get()->didDragToTrashCount_);
+  [button draggedImage:nil endedAt:NSZeroPoint operation:NSDragOperationCopy];
+  EXPECT_EQ(0, delegate.get()->didDragToTrashCount_);
+  [button draggedImage:nil endedAt:NSZeroPoint operation:NSDragOperationMove];
+  EXPECT_EQ(0, delegate.get()->didDragToTrashCount_);
+  [button draggedImage:nil endedAt:NSZeroPoint operation:NSDragOperationDelete];
+  EXPECT_EQ(1, delegate.get()->didDragToTrashCount_);
+}
+
 }