Add support for observing tracking areas so that when tabs are moved undernea...

Add support for observing tracking areas so that when tabs are moved underneath
our cursor, we highlight them correctly.

BUG=27458, 13208, 21448
TEST=Create a pile of tabs. 
     Select a middle one. 
     Put your cursor in some other tab. 
     Hit cmd-w a couple of times
     Watch to make sure highlights occur correctly in both the tabs and the
     close buttons.


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

[dmac, 2009-11-12 18:55:53]

Polishing away under the direct supervision of Mr. Cole.

[rohitrao (ping after 24h), 2009-11-12 19:00:44]

Does this fix http://code.google.com/p/chromium/issues/detail?id=13208 and
http://code.google.com/p/chromium/issues/detail?id=21448 ?

[dmac, 2009-11-12 19:19:13]

On 2009/11/12 19:00:44, rohitrao wrote:
> Does this fix http://code.google.com/p/chromium/issues/detail?id=13208 and
> http://code.google.com/p/chromium/issues/detail?id=21448 ?

It should. I have updated the description appropriately

[pink (ping after 24hrs), 2009-11-12 19:25:56]

I don't quite follow why the event synthesis is needed if you already have the
button being updated when the tracking rects are fixed up in the view. Why do
you need both?

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

http://codereview.chromium.org/386021/diff/1/2#newcode110
Line 110: - (void)mouseMoved:(NSEvent*)event;
if this is already in NSView, we don't need to declare it.

http://codereview.chromium.org/386021/diff/1/2#newcode900
Line 900: // Stop observing the tabs tracking areas.
Should we do this before we remove the tab from the superview? I doubt it
matters, just asking.

http://codereview.chromium.org/386021/diff/1/2#newcode1193
Line 1193: - (void)tabUpdateTracking:(NSNotification*)notification {
function-level comment.

http://codereview.chromium.org/386021/diff/1/2#newcode1195
Line 1195: // Update our hover states
period at end.

http://codereview.chromium.org/386021/diff/1/2#newcode1282
Line 1282: for (NSView* view in [tabView_ subviews]) {
worth commenting that you're removing here in case the view gets removed and not
put back? Unfortunate that we have to do this but I can't think of a better way.

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

http://codereview.chromium.org/386021/diff/1/3#newcode119
Line 119: - (void)updateTrackingAreas {
function-level comment?

[dmac, 2009-11-12 19:49:33]

New version up.

We need to track the close box separately because mousemoved events are not a
reliable way of tracking when the mouse enters and exits a region. You will not
necessarily get a mousemoved event for every mouse exited/entered event because
they coalesce. I tried getting rid of the close box tracking area, but it was
easy to move quickly and get the buttons in bad states.

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

http://codereview.chromium.org/386021/diff/1/2#newcode110
Line 110: - (void)mouseMoved:(NSEvent*)event;
On 2009/11/12 19:25:56, pink wrote:
> if this is already in NSView, we don't need to declare it.

Actually we do because TabStripController doesn't inherit from NSView or
NSResponder.

http://codereview.chromium.org/386021/diff/1/2#newcode900
Line 900: // Stop observing the tabs tracking areas.
On 2009/11/12 19:25:56, pink wrote:
> Should we do this before we remove the tab from the superview? I doubt it
> matters, just asking.

Shouldn't matter, but if it makes you sleep better at night ;-) Fixed.

http://codereview.chromium.org/386021/diff/1/2#newcode1193
Line 1193: - (void)tabUpdateTracking:(NSNotification*)notification {
On 2009/11/12 19:25:56, pink wrote:
> function-level comment.

Done.

http://codereview.chromium.org/386021/diff/1/2#newcode1195
Line 1195: // Update our hover states
On 2009/11/12 19:25:56, pink wrote:
> period at end.

Done.

http://codereview.chromium.org/386021/diff/1/2#newcode1282
Line 1282: for (NSView* view in [tabView_ subviews]) {
On 2009/11/12 19:25:56, pink wrote:
> worth commenting that you're removing here in case the view gets removed and
not
> put back? Unfortunate that we have to do this but I can't think of a better
way.

Done.

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

http://codereview.chromium.org/386021/diff/1/3#newcode119
Line 119: - (void)updateTrackingAreas {
On 2009/11/12 19:25:56, pink wrote:
> function-level comment?

Done.

[pink (ping after 24hrs), 2009-11-12 19:59:37]

LGTM with some comments

http://codereview.chromium.org/386021/diff/3003/3004
File chrome/browser/cocoa/tab_strip_controller.mm (right):

http://codereview.chromium.org/386021/diff/3003/3004#newcode1199
Line 1199: // Update our hover states.
implies that there are hover states in the tab strip, which there aren't. You
can probably just omit this comment now given the function comment.

http://codereview.chromium.org/386021/diff/3003/3005
File chrome/browser/cocoa/tab_view.mm (right):

http://codereview.chromium.org/386021/diff/3003/3005#newcode106
Line 106: NSString *name = nil;
NSString* name

http://codereview.chromium.org/386021/diff/3003/3005#newcode107
Line 107: if (NSPointInRect(hoverPoint_, [closeButton_ frame])) {
worth abstracting this into a helper? looks almost the same as the code below.