(Mac) Make mashing the close tab button work.

(Mac) Make mashing the close tab button work.

To do this, I had to commit several crimes against humanity. In
particular, Cocoa doesn't generate the required extra hit tests during
animations, so we have to. Sometimes, it gets really messed up and ends
up hitting the "drag blocking view". Moreover, we have to account for
the possibility of the mouse down hitting a moving tab, and going up on
the close button, etc.

BUG=17720
TEST=Mash the close tabs button under a wide variety of situations. \
     Also make sure that the handling of the tabs (dragging, etc.)  \
     hasn't accidentally been messed up.

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

[pink (ping after 24hrs), 2009-08-28 02:07:41]

Overall it doesn't look as bad as you say. LGTM with these fixed.

http://codereview.chromium.org/174461/diff/1/5
File chrome/browser/cocoa/tab_strip_controller.mm (right):

http://codereview.chromium.org/174461/diff/1/5#newcode42
Line 42: // false picks up clicks during rapid tab closure, so we have to
account for
correct the grammar.

http://codereview.chromium.org/174461/diff/1/5#newcode62
Line 62: - (void)mouseDown:(NSEvent*)event {
Comment as to what this is doing and why.

http://codereview.chromium.org/174461/diff/1/5#newcode65
Line 65: NSPoint hit_location =
Obj-C code, so use Obj-C naming, eg, |hitLocation|.

http://codereview.chromium.org/174461/diff/1/5#newcode69
Line 69: if (hit_view != self) {
hitView

http://codereview.chromium.org/174461/diff/1/6
File chrome/browser/cocoa/tab_view.mm (right):

http://codereview.chromium.org/174461/diff/1/6#newcode111
Line 111: // don't close.
don't you mean the reverse? If it has moved less than the threshold then we do
want to close?

http://codereview.chromium.org/174461/diff/1/6#newcode115
Line 115: NSPoint down_location = [theEvent locationInWindow];
downLocation

http://codereview.chromium.org/174461/diff/1/6#newcode175
Line 175: NSPoint up_location = [theEvent locationInWindow];
upLocation

http://codereview.chromium.org/174461/diff/1/6#newcode200
Line 200: NSPoint hit_location =
is this used anywhere? Looks like it can be removed.

[viettrungluu-gmail, 2009-08-28 04:30:16]

Thanks for the review. I've made all the requested changes (I think). Probably I
exaggerated the badness of the code -- working around Cocoa's
animation-unfriendly hit testing is merely unpleasant, not horrible.

http://codereview.chromium.org/174461/diff/1/5
File chrome/browser/cocoa/tab_strip_controller.mm (right):

http://codereview.chromium.org/174461/diff/1/5#newcode42
Line 42: // false picks up clicks during rapid tab closure, so we have to
account for
On 2009/08/28 02:07:41, pink wrote:
> correct the grammar.

Done. That was a grammatical train wreck.

http://codereview.chromium.org/174461/diff/1/5#newcode62
Line 62: - (void)mouseDown:(NSEvent*)event {
On 2009/08/28 02:07:41, pink wrote:
> Comment as to what this is doing and why.

Done.

http://codereview.chromium.org/174461/diff/1/5#newcode65
Line 65: NSPoint hit_location =
On 2009/08/28 02:07:41, pink wrote:
> Obj-C code, so use Obj-C naming, eg, |hitLocation|.

Done.

http://codereview.chromium.org/174461/diff/1/5#newcode69
Line 69: if (hit_view != self) {
On 2009/08/28 02:07:41, pink wrote:
> hitView

Done.

http://codereview.chromium.org/174461/diff/1/6
File chrome/browser/cocoa/tab_view.mm (right):

http://codereview.chromium.org/174461/diff/1/6#newcode111
Line 111: // don't close.
On 2009/08/28 02:07:41, pink wrote:
> don't you mean the reverse? If it has moved less than the threshold then we do
> want to close?

You're right. I guess I had a 50% chance of being right. ;-)

http://codereview.chromium.org/174461/diff/1/6#newcode115
Line 115: NSPoint down_location = [theEvent locationInWindow];
On 2009/08/28 02:07:41, pink wrote:
> downLocation

Done. Don't know what I was thinking that day. (Probably, I was just done with
some C++ code....)

http://codereview.chromium.org/174461/diff/1/6#newcode175
Line 175: NSPoint up_location = [theEvent locationInWindow];
On 2009/08/28 02:07:41, pink wrote:
> upLocation

Done.

http://codereview.chromium.org/174461/diff/1/6#newcode200
Line 200: NSPoint hit_location =
On 2009/08/28 02:07:41, pink wrote:
> is this used anywhere? Looks like it can be removed.

Oops, incomplete deletion. Gone.