[Mac] Fix the new tab button hover bounds to match what we test for in hitTest.

BUG=52302
TEST=New tab button should never highlight when the mouse is close to but still outside the button.
Review URL: http://codereview.chromium.org/3153016

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

6 files changed, 61 insertions, 27 deletions

diff --git a/chrome/browser/cocoa/new_tab_button.h b/chrome/browser/cocoa/new_tab_button.h
index 6feee8a..7315ec0 100644
--- a/chrome/browser/cocoa/new_tab_button.h
+++ b/chrome/browser/cocoa/new_tab_button.h
@@ -19,6 +19,10 @@
  @private
   scoped_nsobject<NSBezierPath> imagePath_;
 }
+
+// Returns YES if the given point is over the button.  |point| is in the
+// superview's coordinate system.
+- (BOOL)pointIsOverButton:(NSPoint)point;
 @end
 
 #endif  // CHROME_BROWSER_COCOA_NEW_TAB_BUTTON
diff --git a/chrome/browser/cocoa/new_tab_button.mm b/chrome/browser/cocoa/new_tab_button.mm
index c5e41df..bc92485 100644
--- a/chrome/browser/cocoa/new_tab_button.mm
+++ b/chrome/browser/cocoa/new_tab_button.mm
@@ -25,12 +25,16 @@
   return imagePath_;
 }
 
+- (BOOL)pointIsOverButton:(NSPoint)point {
+  NSPoint localPoint = [self convertPoint:point fromView:[self superview]];
+  NSBezierPath* buttonPath = [self pathForButton];
+  return [buttonPath containsPoint:localPoint];
+}
+
 // Override to only accept clicks within the bounds of the defined path, not
 // the entire bounding box. |aPoint| is in the superview's coordinate system.
 - (NSView*)hitTest:(NSPoint)aPoint {
-  NSBezierPath* buttonPath = [self pathForButton];
-  NSPoint localPoint = [self convertPoint:aPoint fromView:[self superview]];
-  if ([buttonPath containsPoint:localPoint])
+  if ([self pointIsOverButton:aPoint])
     return [super hitTest:aPoint];
   return nil;
 }
diff --git a/chrome/browser/cocoa/tab_strip_controller.h b/chrome/browser/cocoa/tab_strip_controller.h
index 036cfe2..054f2cb 100644
--- a/chrome/browser/cocoa/tab_strip_controller.h
+++ b/chrome/browser/cocoa/tab_strip_controller.h
@@ -14,6 +14,7 @@
 #import "chrome/browser/cocoa/url_drop_target.h"
 #import "third_party/GTM/AppKit/GTMWindowSheetController.h"
 
+@class NewTabButton;
 @class TabContentsController;
 @class TabView;
 @class TabStripView;
@@ -45,7 +46,7 @@ class ToolbarModel;
   scoped_nsobject<TabStripView> tabStripView_;
   NSView* switchView_;  // weak
   scoped_nsobject<NSView> dragBlockingView_;  // avoid bad window server drags
-  NSButton* newTabButton_;  // weak, obtained from the nib.
+  NewTabButton* newTabButton_;  // weak, obtained from the nib.
 
   // Tracks the newTabButton_ for rollovers.
   scoped_nsobject<NSTrackingArea> newTabTrackingArea_;
@@ -53,6 +54,10 @@ class ToolbarModel;
   Browser* browser_;  // weak
   TabStripModel* tabStripModel_;  // weak
 
+  // YES if the new tab button is currently displaying the hover image (if the
+  // mouse is currently over the button).
+  BOOL newTabButtonShowingHoverImage_;
+
   // Access to the TabContentsControllers (which own the parent view
   // for the toolbar and associated tab contents) given an index. Call
   // |indexFromModelIndex:| to convert a |tabStripModel_| index to a
diff --git a/chrome/browser/cocoa/tab_strip_controller.mm b/chrome/browser/cocoa/tab_strip_controller.mm
index cae8307..fbe48bf 100644
--- a/chrome/browser/cocoa/tab_strip_controller.mm
+++ b/chrome/browser/cocoa/tab_strip_controller.mm
@@ -22,6 +22,7 @@
 #include "chrome/browser/profile.h"
 #import "chrome/browser/cocoa/browser_window_controller.h"
 #import "chrome/browser/cocoa/constrained_window_mac.h"
+#import "chrome/browser/cocoa/new_tab_button.h"
 #import "chrome/browser/cocoa/tab_strip_view.h"
 #import "chrome/browser/cocoa/tab_contents_controller.h"
 #import "chrome/browser/cocoa/tab_controller.h"
@@ -133,6 +134,7 @@ private:
 - (void)droppingURLsAt:(NSPoint)point
             givesIndex:(NSInteger*)index
            disposition:(WindowOpenDisposition*)disposition;
+- (void)setNewTabButtonHoverState:(BOOL)showHover;
 @end
 
 // A simple view class that prevents the Window Server from dragging the area
@@ -308,6 +310,7 @@ private:
     [newTabButton_ setImage:nsimage_cache::ImageNamed(kNewTabImage)];
     [newTabButton_
         setAlternateImage:nsimage_cache::ImageNamed(kNewTabPressedImage)];
+    newTabButtonShowingHoverImage_ = NO;
     newTabTrackingArea_.reset(
         [[NSTrackingArea alloc] initWithRect:[newTabButton_ bounds]
                                      options:(NSTrackingMouseEnteredAndExited |
@@ -882,13 +885,9 @@ private:
       NSWindow* window = [tabStripView_ window];
       NSPoint currentMouse = [window mouseLocationOutsideOfEventStream];
       currentMouse = [tabStripView_ convertPoint:currentMouse fromView:nil];
-      NSString* imageName = nil;
-      if (NSPointInRect(currentMouse, newTabNewFrame)) {
-        imageName = kNewTabHoverImage;
-      } else {
-        imageName = kNewTabImage;
-      }
-      [newTabButton_ setImage:nsimage_cache::ImageNamed(imageName)];
+
+      BOOL shouldShowHover = [newTabButton_ pointIsOverButton:currentMouse];
+      [self setNewTabButtonHoverState:shouldShowHover];
 
       // Move the new tab button into place. We want to animate the new tab
       // button if it's moving to the left (closing a tab), but not when it's
@@ -1457,20 +1456,25 @@ private:
 
 - (void)mouseMoved:(NSEvent*)event {
   // Use hit test to figure out what view we are hovering over.
-  TabView* targetView =
-      (TabView*)[tabStripView_ hitTest:[event locationInWindow]];
-  if (![targetView isKindOfClass:[TabView class]]) {
-    if ([[targetView superview] isKindOfClass:[TabView class]]) {
-      targetView = (TabView*)[targetView superview];
+  NSView* targetView = [tabStripView_ hitTest:[event locationInWindow]];
+
+  // Set the new tab button hover state iff the mouse is over the button.
+  BOOL shouldShowHoverImage = [targetView isKindOfClass:[NewTabButton class]];
+  [self setNewTabButtonHoverState:shouldShowHoverImage];
+
+  TabView* tabView = (TabView*)targetView;
+  if (![tabView isKindOfClass:[TabView class]]) {
+    if ([[tabView superview] isKindOfClass:[TabView class]]) {
+      tabView = (TabView*)[targetView superview];
     } else {
-      targetView = nil;
+      tabView = nil;
     }
   }
 
-  if (hoveredTab_ != targetView) {
+  if (hoveredTab_ != tabView) {
     [hoveredTab_ mouseExited:nil];  // We don't pass event because moved events
-    [targetView mouseEntered:nil];  // don't have valid tracking areas
-    hoveredTab_ = targetView;
+    [tabView mouseEntered:nil];  // don't have valid tracking areas
+    hoveredTab_ = tabView;
   } else {
     [hoveredTab_ mouseMoved:event];
   }
@@ -1482,8 +1486,6 @@ private:
     mouseInside_ = YES;
     [self setTabTrackingAreasEnabled:YES];
     [self mouseMoved:event];
-  } else if ([area isEqual:newTabTrackingArea_]) {
-    [newTabButton_ setImage:nsimage_cache::ImageNamed(kNewTabHoverImage)];
   }
 }
 
@@ -1500,7 +1502,11 @@ private:
     hoveredTab_ = nil;
     [self layoutTabs];
   } else if ([area isEqual:newTabTrackingArea_]) {
-    [newTabButton_ setImage:nsimage_cache::ImageNamed(kNewTabImage)];
+    // If the mouse is moved quickly enough, it is possible for the mouse to
+    // leave the tabstrip without sending any mouseMoved: messages at all.
+    // Since this would result in the new tab button incorrectly staying in the
+    // hover state, disable the hover image on every mouse exit.
+    [self setNewTabButtonHoverState:NO];
   }
 }
 
@@ -1525,6 +1531,18 @@ private:
   }
 }
 
+// Sets the new tab button's image based on the current hover state.  Does
+// nothing if the hover state is already correct.
+- (void)setNewTabButtonHoverState:(BOOL)shouldShowHover {
+  if (shouldShowHover && !newTabButtonShowingHoverImage_) {
+    newTabButtonShowingHoverImage_ = YES;
+    [newTabButton_ setImage:nsimage_cache::ImageNamed(kNewTabHoverImage)];
+  } else if (!shouldShowHover && newTabButtonShowingHoverImage_) {
+    newTabButtonShowingHoverImage_ = NO;
+    [newTabButton_ setImage:nsimage_cache::ImageNamed(kNewTabImage)];
+  }
+}
+
 // Adds the given subview to (the end of) the list of permanent subviews
 // (specified from bottom up). These subviews will always be below the
 // transitory subviews (tabs). |-regenerateSubviewList| must be called to
diff --git a/chrome/browser/cocoa/tab_strip_controller_unittest.mm b/chrome/browser/cocoa/tab_strip_controller_unittest.mm
index 032912d..eee7580 100644
--- a/chrome/browser/cocoa/tab_strip_controller_unittest.mm
+++ b/chrome/browser/cocoa/tab_strip_controller_unittest.mm
@@ -7,6 +7,7 @@
 #import "chrome/browser/browser_window.h"
 #include "chrome/browser/cocoa/browser_test_helper.h"
 #import "chrome/browser/cocoa/cocoa_test_helper.h"
+#import "chrome/browser/cocoa/new_tab_button.h"
 #import "chrome/browser/cocoa/tab_strip_controller.h"
 #import "chrome/browser/cocoa/tab_strip_view.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
@@ -96,8 +97,8 @@ class TabStripControllerTest : public CocoaTest {
         [[TabStripView alloc] initWithFrame:strip_frame]);
     [parent addSubview:tab_strip.get()];
     NSRect button_frame = NSMakeRect(0, 0, 15, 15);
-    scoped_nsobject<NSButton> new_tab_button(
-        [[NSButton alloc] initWithFrame:button_frame]);
+    scoped_nsobject<NewTabButton> new_tab_button(
+        [[NewTabButton alloc] initWithFrame:button_frame]);
     [tab_strip addSubview:new_tab_button.get()];
     [tab_strip setNewTabButton:new_tab_button.get()];
 
diff --git a/chrome/browser/cocoa/tab_strip_view.h b/chrome/browser/cocoa/tab_strip_view.h
index 824d577..55214da 100644
--- a/chrome/browser/cocoa/tab_strip_view.h
+++ b/chrome/browser/cocoa/tab_strip_view.h
@@ -11,6 +11,8 @@
 #include "base/scoped_nsobject.h"
 #import "chrome/browser/cocoa/url_drop_target.h"
 
+@class NewTabButton;
+
 // A view class that handles rendering the tab strip and drops of URLS with
 // a positioning locator for drop feedback.
 
@@ -22,7 +24,7 @@
   scoped_nsobject<URLDropTargetHandler> dropHandler_;
 
   // Weak; the following come from the nib.
-  NSButton* newTabButton_;
+  NewTabButton* newTabButton_;
 
   // Whether the drop-indicator arrow is shown, and if it is, the coordinate of
   // its tip.
@@ -30,7 +32,7 @@
   NSPoint dropArrowPosition_;
 }
 
-@property(assign, nonatomic) IBOutlet NSButton* newTabButton;
+@property(assign, nonatomic) IBOutlet NewTabButton* newTabButton;
 @property(assign, nonatomic) BOOL dropArrowShown;
 @property(assign, nonatomic) NSPoint dropArrowPosition;