[Mac] Implement highlight-on-hover for the Wrench menu buttons.

chrome/browser/cocoa/menu_tracked_button.mm

Index: chrome/browser/cocoa/menu_tracked_button.mm
diff --git a/chrome/browser/cocoa/menu_tracked_button.mm b/chrome/browser/cocoa/menu_tracked_button.mm
index 654e850243b4d0ac0e91cde08518e6eb66a84fa8..fd5b88d95277e140991e2d21ceddf6ffad46fa9f 100644
--- a/chrome/browser/cocoa/menu_tracked_button.mm
+++ b/chrome/browser/cocoa/menu_tracked_button.mm
@@ -4,29 +4,91 @@
 
 #import "chrome/browser/cocoa/menu_tracked_button.h"
 
+@interface MenuTrackedButton (Private)
+- (void)doHighlight:(BOOL)highlight;
+- (void)checkMouseInRect;
+- (NSRect)insetBounds;
+@end
+
 @implementation MenuTrackedButton
 
+- (void)updateTrackingAreas {
+  [super updateTrackingAreas];
+  [self removeTrackingRect:trackingTag_];
+  trackingTag_ = [self addTrackingRect:NSInsetRect([self bounds], 1, 1)
+                                 owner:self
+                              userData:NULL
+                          assumeInside:NO];
+}
+
+- (void)viewDidMoveToWindow {
+  [self updateTrackingAreas];
+  [self doHighlight:NO];
+}
+
 - (void)mouseEntered:(NSEvent*)theEvent {
-  didEnter_ = YES;
+  if (!tracking_) {
+    didEnter_ = YES;
+  }
+  [self doHighlight:YES];
   [super mouseEntered:theEvent];
 }
 
 - (void)mouseExited:(NSEvent*)theEvent {
   didEnter_ = NO;
   tracking_ = NO;
+  [self doHighlight:NO];
   [super mouseExited:theEvent];
 }
 
 - (void)mouseDragged:(NSEvent*)theEvent {
   tracking_ = !didEnter_;
+
+  NSPoint point = [self convertPoint:[theEvent locationInWindow] fromView:nil];
+  BOOL highlight = NSPointInRect(point, [self insetBounds]);
+  [self doHighlight:highlight];
+
+  // If tracking in non-sticky mode, poll the mouse cursor to see if it is still
+  // over the button and thus needs to be highlighted.
+  if (tracking_) {
+    [self performSelector:@selector(checkMouseInRect)
+               withObject:nil
+               afterDelay:0.05  // This is the smallest delay that works.

[pink (ping after 24hrs), 2010/08/17 17:41:01]

do we really need to be checking 20 times a second to see if the mouse is still
in the button? That seems really wasteful. Can't we do 0.5s ? If not, then your
comment is backwards, no? It's the "largest" delay that works, because nothing
larger will work.

[Robert Sesek, 2010/08/17 18:18:04]

It's absolutely necessary to check this frequently.  The delay is how long the
button could be stuck in hover state if the user mouses out.  1/20th of a second
is the mean time for a user to visually perceive change according to Wikipedia. 
The delay is noticeable, but barely.  If it was changed to 1/2 a second, the
delay would be jarringly noticeable.  I'll fix the comment.

+                  inModes:[NSArray arrayWithObject:NSEventTrackingRunLoopMode]];
+  }
   [super mouseDragged:theEvent];
 }
 
 - (void)mouseUp:(NSEvent*)theEvent {
+  [self doHighlight:NO];
   if (!tracking_) {
     return [super mouseUp:theEvent];
   }
   [self performClick:self];
 }
 
+- (void)doHighlight:(BOOL)highlight {
+  [[self cell] setHighlighted:highlight];
+  [self setNeedsDisplay];
+}
+
+// Checks if the user's current mouse location is over this button.  If it is,
+// she is merely hovering here.  If it is not, then disable the highlight.  If

[pink (ping after 24hrs), 2010/08/17 17:41:01]

s/she/they ? we are usually gender-neutral in comments.

[Robert Sesek, 2010/08/17 18:18:04]

'They' is plural and would break subject-verb agreement.  'It' implies
non-human.  Using she is grammatically and politically acceptable.  I could also
reword the comment to repeat the dereferenced subject.

+// the menu is opened in non-sticky mode, the button does not receive enter/exit
+// mouse events and thus polling is necessary.
+- (void)checkMouseInRect {
+  NSPoint point = [NSEvent mouseLocation];
+  point = [[self window] convertScreenToBase:point];

[pink (ping after 24hrs), 2010/08/17 17:41:01]

I don't think you want convertScreenToBase. Check with Avi. I think you want to
[convertPoint:point fromView:nil]

[Robert Sesek, 2010/08/17 18:18:04]

I was under the impression that the two were equivalent when |fromView| is nil.

[pink (ping after 24hrs), 2010/08/17 18:27:53]

we've had a lot of bugs crop up from this, i think avi figured out the
difference at one point, he should be able to explain it.

+  point = [self convertPointFromBase:point];

[Scott Hess - ex-Googler, 2010/08/17 19:55:07]

I find pink's comment mis-leading.  I do think you need -[NSWindow
convertScreenToBase:] to get the point from screen coordinates to the window's
coordinates, but then you want -[NSView convertPoint:fromView:nil] to get it to
your target view's coordinates.

I think pink was refering to this thread:
https://groups.google.com/a/chromium.org/group/chromium-dev/browse_thread/thr...

+  if (!NSPointInRect(point, [self insetBounds])) {
+    [self doHighlight:NO];
+  }
+}
+
+// Returns the bounds of the receiver slightly inset to avoid highlighting both
+// buttons in a pair that overlap.
+- (NSRect)insetBounds {
+  return NSInsetRect([self bounds], 2, 1);
+}
+
 @end