Make download items drag sources on OS X.

Extract button dragging out of BookmarkButton into DraggableButton. Make BookmarkButton a subclass of DraggableButton. Create new class DownloadItemButton and make it a subclass of DraggableButton.

xib change: Make download item a DownloadItemButton instead of an NSButton.

BUG=15776
TEST=Download something, wait for it to complete, then drag it from the download shelf to somewhere. It should now work. Bookmarks should still be draggable in the bookmarks bar.

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

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

11 files changed, 262 insertions, 128 deletions

diff --git a/chrome/browser/cocoa/bookmark_button.h b/chrome/browser/cocoa/bookmark_button.h
index 631a7ba..3034166 100644
--- a/chrome/browser/cocoa/bookmark_button.h
+++ b/chrome/browser/cocoa/bookmark_button.h
@@ -1,26 +1,19 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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>
 
+#import "chrome/browser/cocoa/draggable_button.h"
+
 @protocol BookmarkButtonDelegate;
 
 // Class for bookmark bar buttons that can be drag sources.
-@interface BookmarkButton : NSButton {
+@interface BookmarkButton : DraggableButton {
  @private
-  BOOL draggable_;     // Is this a draggable type of button?
-  BOOL mayDragStart_;  // Set to YES on mouse down, NO on up or drag.
-  BOOL beingDragged_;
   id<BookmarkButtonDelegate> delegate_;
-
-  // Initial mouse-down to prevent a hair-trigger drag.
-  NSPoint initialMouseDownLocation_;
 }
 
-// Enable or disable dragability for special buttons like "Other Bookmarks".
-@property BOOL draggable;
-
 @property(assign, nonatomic) id<BookmarkButtonDelegate> delegate;
 
 @end  // @interface BookmarkButton
diff --git a/chrome/browser/cocoa/bookmark_button.mm b/chrome/browser/cocoa/bookmark_button.mm
index 55c95f6..b0b6ecd 100644
--- a/chrome/browser/cocoa/bookmark_button.mm
+++ b/chrome/browser/cocoa/bookmark_button.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -6,19 +6,9 @@
 #import "base/scoped_nsobject.h"
 #import "chrome/browser/cocoa/bookmark_button.h"
 #import "chrome/browser/cocoa/bookmark_button_cell.h"
-#import "third_party/GTM/AppKit/GTMTheme.h"
-
-namespace {
-
-// Code taken from <http://codereview.chromium.org/180036/diff/3001/3004>.
-// TODO(viettrungluu): Do we want common, standard code for drag hysteresis?
-const CGFloat kWebDragStartHysteresisX = 5.0;
-const CGFloat kWebDragStartHysteresisY = 5.0;
 
 // The opacity of the bookmark button drag image.
-const CGFloat kDragImageOpacity = 0.7;
-
-}
+static const CGFloat kDragImageOpacity = 0.7;
 
 @interface BookmarkButton(Private)
 
@@ -29,25 +19,15 @@ const CGFloat kDragImageOpacity = 0.7;
 
 @implementation BookmarkButton
 
-@synthesize draggable = draggable_;
 @synthesize delegate = delegate_;
 
-- (id)initWithFrame:(NSRect)frame {
-  if ((self = [super initWithFrame:frame])) {
-    draggable_ = YES;
-  }
-  return self;
-}
-
 // By default, NSButton ignores middle-clicks.
 - (void)otherMouseUp:(NSEvent*)event {
   [self performClick:self];
 }
 
+// Overridden from DraggableButton.
 - (void)beginDrag:(NSEvent*)event {
-  // Starting drag. Never start another drag until another mouse down.
-  mayDragStart_ = NO;
-
   if (delegate_) {
     // Ask our delegate to fill the pasteboard for us.
     NSPasteboard* pboard = [NSPasteboard pasteboardWithName:NSDragPboard];
@@ -64,7 +44,7 @@ const CGFloat kDragImageOpacity = 0.7;
     [self dragImage:[self dragImage] at:NSMakePoint(0, yAt) offset:dragOffset
               event:event pasteboard:pboard source:self slideBack:YES];
 
-    // And we're done.
+    // And we're done.	
     [self autorelease];
   } else {
     // Avoid blowing up, but we really shouldn't get here.
@@ -75,8 +55,7 @@ const CGFloat kDragImageOpacity = 0.7;
 - (void)draggedImage:(NSImage*)anImage
              endedAt:(NSPoint)aPoint
            operation:(NSDragOperation)operation {
-  beingDragged_ = NO;
-  [[self cell] setHighlighted:NO];
+  [super endDrag];
 }
 
 - (NSDragOperation)draggingSourceOperationMaskForLocal:(BOOL)isLocal {
@@ -84,61 +63,6 @@ const CGFloat kDragImageOpacity = 0.7;
                  : NSDragOperationCopy;
 }
 
-- (void)mouseUp:(NSEvent*)theEvent {
-  // Make sure that we can't start a drag until we see a mouse down again.
-  mayDragStart_ = NO;
-
-  // This conditional is never true (DnD loops in Cocoa eat the mouse
-  // up) but I added it in case future versions of Cocoa do unexpected
-  // things.
-  if (beingDragged_) {
-    NOTREACHED();
-    return [super mouseUp:theEvent];
-  }
-
-  // There are non-drag cases where a mouseUp: may happen
-  // (e.g. mouse-down, cmd-tab to another application, move mouse,
-  // mouse-up).  So we check.
-  NSPoint viewLocal = [self convertPoint:[theEvent locationInWindow]
-                                fromView:[[self window] contentView]];
-  if (NSPointInRect(viewLocal, [self bounds])) {
-    [self performClick:self];
-  } else {
-    [[self cell] setHighlighted:NO];
-  }
-}
-
-// Mimic "begin a click" operation visually.  Do NOT follow through
-// with normal button event handling.
-- (void)mouseDown:(NSEvent*)theEvent {
-  mayDragStart_ = YES;
-  [[self cell] setHighlighted:YES];
-  initialMouseDownLocation_ = [theEvent locationInWindow];
-}
-
-// Return YES if we have crossed a threshold of movement after
-// mouse-down when we should begin a drag.  Else NO.
-- (BOOL)hasCrossedDragThreshold:(NSEvent*)theEvent {
-  NSPoint currentLocation = [theEvent locationInWindow];
-  if ((abs(currentLocation.x - initialMouseDownLocation_.x) >
-       kWebDragStartHysteresisX) ||
-      (abs(currentLocation.y - initialMouseDownLocation_.y) >
-       kWebDragStartHysteresisY)) {
-    return YES;
-  } else {
-    return NO;
-  }
-}
-
-- (void)mouseDragged:(NSEvent*)theEvent {
-  if (beingDragged_) {
-    [super mouseDragged:theEvent];
-  } else if (draggable_ && mayDragStart_ &&
-             [self hasCrossedDragThreshold:theEvent]) {
-    [self beginDrag:theEvent];
-  }
-}
-
 @end
 
 @implementation BookmarkButton(Private)
diff --git a/chrome/browser/cocoa/bookmark_button_unittest.mm b/chrome/browser/cocoa/bookmark_button_unittest.mm
index fcdf528..5ec87bd 100644
--- a/chrome/browser/cocoa/bookmark_button_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_button_unittest.mm
@@ -12,38 +12,8 @@ class BookmarkButtonTest : public CocoaTest {
  public:
 };
 
-NSEvent* Event(const NSPoint point, const NSEventType type) {
-  static NSUInteger eventNumber = 0;  // thx shess
-  return [NSEvent mouseEventWithType:type
-                            location:point
-                       modifierFlags:0
-                           timestamp:0
-                        windowNumber:183  // picked out of thin air.
-                             context:nil
-                         eventNumber:eventNumber++
-                          clickCount:1
-                            pressure:0.0];
-}
-
-// Make sure the basic case of "click" still works.
-TEST_F(BookmarkButtonTest, DownUp) {
-  scoped_nsobject<NSMutableArray> array;
-  array.reset([[NSMutableArray alloc] init]);
-  [array addObject:@"foo"];
-  [array addObject:@"bar"];
-
+// Make sure nothing leaks
+TEST_F(BookmarkButtonTest, Create) {
   scoped_nsobject<BookmarkButton> button;
   button.reset([[BookmarkButton alloc] initWithFrame:NSMakeRect(0,0,500,500)]);
-
-  [button setTarget:array.get()];
-  [button setAction:@selector(removeAllObjects)];
-  EXPECT_FALSE([[button cell] isHighlighted]);
-
-  NSEvent* downEvent(Event(NSMakePoint(10,10), NSLeftMouseDown));
-  NSEvent* upEvent(Event(NSMakePoint(10,10), NSLeftMouseDown));
-  [button mouseDown:downEvent];
-  EXPECT_TRUE([[button cell] isHighlighted]);
-  [button mouseUp:upEvent];
-  EXPECT_FALSE([[button cell] isHighlighted]);
-  EXPECT_FALSE([array count]);  // confirms target/action fired
 }
diff --git a/chrome/browser/cocoa/download_item_button.h b/chrome/browser/cocoa/download_item_button.h
new file mode 100644
index 0000000..5ce1a01
--- /dev/null
+++ b/chrome/browser/cocoa/download_item_button.h
@@ -0,0 +1,21 @@
+// Copyright (c) 2010 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/file_path.h"
+#import "chrome/browser/cocoa/draggable_button.h"
+
+// A button that is a drag source for a file.
+@interface DownloadItemButton : DraggableButton {
+ @private
+  FilePath downloadPath_;
+}
+
+@property(assign, nonatomic) FilePath download;
+
+// Overridden from DraggableButton.
+- (void)beginDrag:(NSEvent*)event;
+
+@end
diff --git a/chrome/browser/cocoa/download_item_button.mm b/chrome/browser/cocoa/download_item_button.mm
new file mode 100644
index 0000000..41a62c8
--- /dev/null
+++ b/chrome/browser/cocoa/download_item_button.mm
@@ -0,0 +1,21 @@
+// Copyright (c) 2010 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/download_item_button.h"
+
+#include "base/sys_string_conversions.h"
+
+@implementation DownloadItemButton
+
+@synthesize download = downloadPath_;
+
+// Overridden from DraggableButton.
+- (void)beginDrag:(NSEvent*)event {
+  if (!downloadPath_.empty()) {
+    NSString* filename = base::SysUTF8ToNSString(downloadPath_.value());
+    [self dragFile:filename fromRect:[self bounds] slideBack:YES event:event];
+  }
+}
+
+@end
diff --git a/chrome/browser/cocoa/download_item_button_unittest.mm b/chrome/browser/cocoa/download_item_button_unittest.mm
new file mode 100644
index 0000000..d5a1b54
--- /dev/null
+++ b/chrome/browser/cocoa/download_item_button_unittest.mm
@@ -0,0 +1,21 @@
+// Copyright (c) 2010 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.
+
+#include "base/scoped_nsobject.h"
+#import "chrome/browser/cocoa/download_item_button.h"
+#import "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+// Make sure nothing leaks.
+TEST(DownloadItemButtonTest, Create) {
+  scoped_nsobject<DownloadItemButton> button;
+  button.reset([[DownloadItemButton alloc]
+      initWithFrame:NSMakeRect(0,0,500,500)]);
+
+  // Test setter
+  FilePath path("foo");
+  [button.get() setDownload:path];
+  EXPECT_EQ(path.value(), [button.get() download].value());
+}
diff --git a/chrome/browser/cocoa/download_item_controller.h b/chrome/browser/cocoa/download_item_controller.h
index 287d61c..a4c7f66 100644
--- a/chrome/browser/cocoa/download_item_controller.h
+++ b/chrome/browser/cocoa/download_item_controller.h
@@ -11,6 +11,7 @@ class BaseDownloadItemModel;
 @class ChromeUILocalizer;
 @class DownloadItemCell;
 class DownloadItem;
+@class DownloadItemButton;
 class DownloadItemMac;
 class DownloadShelfContextMenuMac;
 @class DownloadShelfController;
@@ -20,7 +21,7 @@ class DownloadShelfContextMenuMac;
 
 @interface DownloadItemController : NSViewController {
  @private
-  IBOutlet NSButton* progressView_;
+  IBOutlet DownloadItemButton* progressView_;
   IBOutlet DownloadItemCell* cell_;
 
   IBOutlet NSMenu* activeDownloadMenu_;
diff --git a/chrome/browser/cocoa/download_item_controller.mm b/chrome/browser/cocoa/download_item_controller.mm
index 9e1651b..b2ff7ff 100644
--- a/chrome/browser/cocoa/download_item_controller.mm
+++ b/chrome/browser/cocoa/download_item_controller.mm
@@ -9,6 +9,7 @@
 #include "app/resource_bundle.h"
 #include "base/mac_util.h"
 #include "base/sys_string_conversions.h"
+#import "chrome/browser/cocoa/download_item_button.h"
 #import "chrome/browser/cocoa/download_item_cell.h"
 #include "chrome/browser/cocoa/download_item_mac.h"
 #import "chrome/browser/cocoa/download_shelf_controller.h"
@@ -161,11 +162,13 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu {
     return;
   }
 
-  // Set the correct popup menu.
-  if (downloadModel->download()->state() == DownloadItem::COMPLETE)
+  // Set correct popup menu. Also, set draggable download on completion.
+  if (downloadModel->download()->state() == DownloadItem::COMPLETE) {
     currentMenu_ = completeDownloadMenu_;
-  else
+    [progressView_ setDownload:downloadModel->download()->full_path()];
+  } else {
     currentMenu_ = activeDownloadMenu_;
+  }
 
   [progressView_ setMenu:currentMenu_];  // for context menu
   [cell_ setStateFromDownload:downloadModel];
diff --git a/chrome/browser/cocoa/draggable_button.h b/chrome/browser/cocoa/draggable_button.h
new file mode 100644
index 0000000..ed9a94f
--- /dev/null
+++ b/chrome/browser/cocoa/draggable_button.h
@@ -0,0 +1,30 @@
+// Copyright (c) 2010 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>
+
+// Class for buttons that can be drag sources. If the mouse is clicked and moved
+// more than a given distance, this class will call |-beginDrag:| instead of
+// |-performClick:|. Subclasses should override these two methods.
+@interface DraggableButton : NSButton {
+ @private
+  BOOL draggable_;     // Is this a draggable type of button?
+  BOOL mayDragStart_;  // Set to YES on mouse down, NO on up or drag.
+  BOOL beingDragged_;
+
+  // Initial mouse-down to prevent a hair-trigger drag.
+  NSPoint initialMouseDownLocation_;
+}
+
+// Enable or disable dragability for special buttons like "Other Bookmarks".
+@property BOOL draggable;
+
+// Called when a drag starts. Subclasses must override this.
+- (void)beginDrag:(NSEvent*)dragEvent;
+
+// Subclasses should call this method to notify DraggableButton when a drag is
+// over.
+- (void)endDrag;
+
+@end  // @interface DraggableButton
diff --git a/chrome/browser/cocoa/draggable_button.mm b/chrome/browser/cocoa/draggable_button.mm
new file mode 100644
index 0000000..ff8270d
--- /dev/null
+++ b/chrome/browser/cocoa/draggable_button.mm
@@ -0,0 +1,105 @@
+// Copyright (c) 2010 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/draggable_button.h"
+
+#include "base/logging.h"
+#import "base/scoped_nsobject.h"
+
+namespace {
+
+// Code taken from <http://codereview.chromium.org/180036/diff/3001/3004>.
+// TODO(viettrungluu): Do we want common, standard code for drag hysteresis?
+const CGFloat kWebDragStartHysteresisX = 5.0;
+const CGFloat kWebDragStartHysteresisY = 5.0;
+
+}
+
+@implementation DraggableButton
+
+@synthesize draggable = draggable_;
+
+- (id)initWithFrame:(NSRect)frame {
+  if ((self = [super initWithFrame:frame])) {
+    draggable_ = YES;
+  }
+  return self;
+}
+
+- (id)initWithCoder:(NSCoder*)coder {
+  if ((self = [super initWithCoder:coder])) {
+    draggable_ = YES;
+  }
+  return self;
+}
+
+- (void)mouseUp:(NSEvent*)theEvent {
+  // Make sure that we can't start a drag until we see a mouse down again.
+  mayDragStart_ = NO;
+
+  // This conditional is never true (DnD loops in Cocoa eat the mouse
+  // up) but I added it in case future versions of Cocoa do unexpected
+  // things.
+  if (beingDragged_) {
+    NOTREACHED();
+    return [super mouseUp:theEvent];
+  }
+
+  // There are non-drag cases where a mouseUp: may happen
+  // (e.g. mouse-down, cmd-tab to another application, move mouse,
+  // mouse-up).  So we check.
+  NSPoint viewLocal = [self convertPoint:[theEvent locationInWindow]
+                                fromView:[[self window] contentView]];
+  if (NSPointInRect(viewLocal, [self bounds])) {
+    [self performClick:self];
+  } else {
+    [self endDrag];
+  }
+}
+
+// Mimic "begin a click" operation visually.  Do NOT follow through
+// with normal button event handling.
+- (void)mouseDown:(NSEvent*)theEvent {
+  mayDragStart_ = YES;
+  [[self cell] setHighlighted:YES];
+  initialMouseDownLocation_ = [theEvent locationInWindow];
+}
+
+// Return YES if we have crossed a threshold of movement after
+// mouse-down when we should begin a drag.  Else NO.
+- (BOOL)hasCrossedDragThreshold:(NSEvent*)theEvent {
+  NSPoint currentLocation = [theEvent locationInWindow];
+
+  if ((abs(currentLocation.x - initialMouseDownLocation_.x) >
+       kWebDragStartHysteresisX) ||
+      (abs(currentLocation.y - initialMouseDownLocation_.y) >
+       kWebDragStartHysteresisY)) {
+    return YES;
+  } else {
+    return NO;
+  }
+}
+
+- (void)mouseDragged:(NSEvent*)theEvent {
+  if (beingDragged_) {
+    [super mouseDragged:theEvent];
+  } else if (draggable_ && mayDragStart_ &&
+             [self hasCrossedDragThreshold:theEvent]) {
+    // Starting drag. Never start another drag until another mouse down.
+    mayDragStart_ = NO;
+    [self beginDrag:theEvent];
+  }
+}
+
+- (void)beginDrag:(NSEvent*)dragEvent {
+  // Must be overridden by subclasses.
+  NOTREACHED();
+}
+
+- (void)endDrag {
+  beingDragged_ = NO;
+  [[self cell] setHighlighted:NO];
+}
+
+@end
diff --git a/chrome/browser/cocoa/draggable_button_unittest.mm b/chrome/browser/cocoa/draggable_button_unittest.mm
new file mode 100644
index 0000000..9b892cc
--- /dev/null
+++ b/chrome/browser/cocoa/draggable_button_unittest.mm
@@ -0,0 +1,45 @@
+// Copyright (c) 2010 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.
+
+#include "base/scoped_nsobject.h"
+#import "chrome/browser/cocoa/bookmark_button.h"
+#import "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+NSEvent* Event(const NSPoint point, const NSEventType type) {
+  static NSUInteger eventNumber = 0;  // thx shess
+  return [NSEvent mouseEventWithType:type
+                            location:point
+                       modifierFlags:0
+                           timestamp:0
+                        windowNumber:183  // picked out of thin air.
+                             context:nil
+                         eventNumber:eventNumber++
+                          clickCount:1
+                            pressure:0.0];
+}
+
+// Make sure the basic case of "click" still works.
+TEST(DraggableButtonTest, DownUp) {
+  scoped_nsobject<NSMutableArray> array;
+  array.reset([[NSMutableArray alloc] init]);
+  [array addObject:@"foo"];
+  [array addObject:@"bar"];
+
+  scoped_nsobject<DraggableButton> button;
+  button.reset([[DraggableButton alloc] initWithFrame:NSMakeRect(0,0,500,500)]);
+
+  [button setTarget:array.get()];
+  [button setAction:@selector(removeAllObjects)];
+  EXPECT_FALSE([[button cell] isHighlighted]);
+
+  NSEvent* downEvent(Event(NSMakePoint(10,10), NSLeftMouseDown));
+  NSEvent* upEvent(Event(NSMakePoint(10,10), NSLeftMouseDown));
+  [button mouseDown:downEvent];
+  EXPECT_TRUE([[button cell] isHighlighted]);
+  [button mouseUp:upEvent];
+  EXPECT_FALSE([[button cell] isHighlighted]);
+  EXPECT_FALSE([array count]);  // confirms target/action fired
+}