[Mac] "Refactor" DraggableButton into a mixin and impl.

[Mac] "Refactor" DraggableButton into a mixin and impl.

This is because DraggableButton inherits from NSButton when a future class will
need it to inherit from NSPopupButton. Because ObjC lacks multiple inheritance,
and we want to avoid runtime trickery, this is the result.

BUG=none
TEST=Dragging download shelf and bookmark bar items works as before. Partially covered by unit tests.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96433

[Robert Sesek, 2011-08-10 21:15:51]

Enjoy :-D.

[Mark Mentovai, 2011-08-11 00:51:17]

This isn’t nearly as bad as you made it out to be, although I can appreciate
that the work was frustrating.

LGTM with this stuff fixed.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/bookmar...
File chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm (right):

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/bookmar...
chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm:7: #include <complex>
<cmath> (see elsewhere for rationale)

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/bookmar...
chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm:232: return
kDraggableButtonImplUseBase;
In a sense, this is actually cleaner than what it replaces.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
File chrome/browser/ui/cocoa/draggable_button_mixin.h (right):

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:4: 
#include guards and #pragma once.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:57: // Resets the draggable
state of the button after dragging is finished.  This is
I thought you were a one-space-after-period kind of guy.

Yeah, I notice this stuff.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:61: // Decides whether we
should treat the click as a cue to start dragging, or
We, and on line 70.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:65: yDelta:(float)yDelta
Seems like you could have lined up the colons here.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:80: @end
Not too bad so far.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:105: @property NSTimeInterval
durationMouseWasDown;
atomic? 107 too.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:125: -
(DraggableButtonResult)mouseDown:(NSEvent*)event;
Hmm. I guess this is technically unportable, but it will work on the
architectures we care about (and probably any we don’t care about). If clang
doesn’t complain, I’ll allow it.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
File chrome/browser/ui/cocoa/draggable_button_mixin.mm (right):

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.mm:7: #include <complex>
<complex> is for std::abs on complex<> values. You want std::abs<> on
floating-point types. Use <cmath>.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.mm:70: if (actionHasFired_)
Here’s where the turd pile begins, but it’s actually not as bad as you made it
out to be.

[Robert Sesek, 2011-08-11 16:06:00]

Addressed allays.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
File chrome/browser/ui/cocoa/draggable_button_mixin.h (right):

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:57: // Resets the draggable
state of the button after dragging is finished.  This is
On 2011/08/11 00:51:17, Mark Mentovai wrote:
> I thought you were a one-space-after-period kind of guy.
> 
> Yeah, I notice this stuff.

I am. I just didn't feel like fixing all the old comments.

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:65: yDelta:(float)yDelta
On 2011/08/11 00:51:17, Mark Mentovai wrote:
> Seems like you could have lined up the colons here.

Eh. Methods that fly off to the right like that are just annoying. (Looking at
you nextEventMatchingMask:)

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:125: -
(DraggableButtonResult)mouseDown:(NSEvent*)event;
On 2011/08/11 00:51:17, Mark Mentovai wrote:
> Hmm. I guess this is technically unportable, but it will work on the
> architectures we care about (and probably any we don’t care about). If clang
> doesn’t complain, I’ll allow it.

Why wouldn't this be "portable"? This inherits from NSObject, not NSResponder.
Clang don't complain.

[Mark Mentovai, 2011-08-11 16:11:45]

LGTM

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
File chrome/browser/ui/cocoa/draggable_button_mixin.h (right):

http://codereview.chromium.org/7462018/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button_mixin.h:125: -
(DraggableButtonResult)mouseDown:(NSEvent*)event;
rsesek wrote:
> Why wouldn't this be "portable"? This inherits from NSObject, not NSResponder.

Oh yeah.