[Mac] Prevent stale reload button tooltips from remaining after state change.

Apparently -[NSView setToolTip:] don't handle removing any existing
display.  -removeAllToolTips or -setToolTip:nil both appear to do the
right thing.

This handles both transitions because you can go from reload mode to
stop mode due to in-page activity.

Unfortunately, after transition the new tooltip will never appear.
AFAICT, you can't force a tooltip up, we'd probably have to fake out
the appkit machinery, which seems excessive to me.

BUG=61556
TEST=See bug.

Review URL: http://codereview.chromium.org/4451003

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

3 files changed, 67 insertions, 26 deletions

diff --git a/chrome/browser/cocoa/reload_button.h b/chrome/browser/cocoa/reload_button.h
index edb6e9c..6432943 100644
--- a/chrome/browser/cocoa/reload_button.h
+++ b/chrome/browser/cocoa/reload_button.h
@@ -27,6 +27,11 @@
 // Returns YES if the mouse is currently inside the bounds.
 - (BOOL)isMouseInside;
 
+// Update the tag, and the image and tooltip to match.  If |anInt|
+// matches the current tag, no action is taken.  |anInt| must be
+// either |IDC_STOP| or |IDC_RELOAD|.
+- (void)updateTag:(NSInteger)anInt;
+
 // Update the button to be a reload button or stop button depending on
 // |isLoading|.  If |force|, always sets the indicated mode.  If
 // |!force|, and the mouse is over the button, defer the transition
diff --git a/chrome/browser/cocoa/reload_button.mm b/chrome/browser/cocoa/reload_button.mm
index 9975db7..56cb3d7 100644
--- a/chrome/browser/cocoa/reload_button.mm
+++ b/chrome/browser/cocoa/reload_button.mm
@@ -60,21 +60,36 @@ NSTimeInterval kPendingReloadTimeout = 1.35;
   [self setIgnoresMultiClick:YES];
 }
 
+- (void)updateTag:(NSInteger)anInt {
+  if ([self tag] == anInt)
+    return;
+
+  // Forcibly remove any stale tooltip which is being displayed.
+  [self removeAllToolTips];
+
+  [self setTag:anInt];
+  if (anInt == IDC_RELOAD) {
+    [self setImage:nsimage_cache::ImageNamed(kReloadImageName)];
+    [self setToolTip:l10n_util::GetNSStringWithFixup(IDS_TOOLTIP_RELOAD)];
+  } else if (anInt == IDC_STOP) {
+    [self setImage:nsimage_cache::ImageNamed(kStopImageName)];
+    [self setToolTip:l10n_util::GetNSStringWithFixup(IDS_TOOLTIP_STOP)];
+  } else {
+    NOTREACHED();
+  }
+}
+
 - (void)setIsLoading:(BOOL)isLoading force:(BOOL)force {
   // Can always transition to stop mode.  Only transition to reload
   // mode if forced or if the mouse isn't hovering.  Otherwise, note
   // that reload mode is desired and disable the button.
   if (isLoading) {
     pendingReloadTimer_.reset();
-    [self setImage:nsimage_cache::ImageNamed(kStopImageName)];
-    [self setTag:IDC_STOP];
-    [self setToolTip:l10n_util::GetNSStringWithFixup(IDS_TOOLTIP_STOP)];
+    [self updateTag:IDC_STOP];
     [self setEnabled:YES];
   } else if (force || ![self isMouseInside]) {
     pendingReloadTimer_.reset();
-    [self setImage:nsimage_cache::ImageNamed(kReloadImageName)];
-    [self setTag:IDC_RELOAD];
-    [self setToolTip:l10n_util::GetNSStringWithFixup(IDS_TOOLTIP_RELOAD)];
+    [self updateTag:IDC_RELOAD];
 
     // This button's cell may not have received a mouseExited event, and
     // therefore it could still think that the mouse is inside the button.  Make
diff --git a/chrome/browser/cocoa/reload_button_unittest.mm b/chrome/browser/cocoa/reload_button_unittest.mm
index 19fde82..617503f 100644
--- a/chrome/browser/cocoa/reload_button_unittest.mm
+++ b/chrome/browser/cocoa/reload_button_unittest.mm
@@ -10,7 +10,7 @@
 #include "chrome/app/chrome_command_ids.h"
 #import "chrome/browser/cocoa/cocoa_test_helper.h"
 #import "chrome/browser/cocoa/test_event_utils.h"
-#include "testing/gtest/include/gtest/gtest.h"
+#import "testing/gtest_mac.h"
 #include "testing/platform_test.h"
 #import "third_party/ocmock/OCMock/OCMock.h"
 
@@ -72,74 +72,95 @@ TEST_F(ReloadButtonTest, IgnoredMultiClick) {
   [button_ setTarget:nil];
 }
 
+TEST_F(ReloadButtonTest, UpdateTag) {
+  [button_ setTag:IDC_STOP];
+
+  [button_ updateTag:IDC_RELOAD];
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
+  NSImage* reloadImage = [button_ image];
+  NSString* const reloadToolTip = [button_ toolTip];
+
+  [button_ updateTag:IDC_STOP];
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
+  NSImage* stopImage = [button_ image];
+  NSString* const stopToolTip = [button_ toolTip];
+  EXPECT_NSNE(reloadImage, stopImage);
+  EXPECT_NSNE(reloadToolTip, stopToolTip);
+
+  [button_ updateTag:IDC_RELOAD];
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
+  EXPECT_NSEQ(reloadImage, [button_ image]);
+  EXPECT_NSEQ(reloadToolTip, [button_ toolTip]);
+}
+
 // Test that when forcing the mode, it takes effect immediately,
 // regardless of whether the mouse is hovering.
 TEST_F(ReloadButtonTest, SetIsLoadingForce) {
   EXPECT_FALSE([button_ isMouseInside]);
-  EXPECT_EQ([button_ tag], IDC_RELOAD);
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
 
   // Changes to stop immediately.
   [button_ setIsLoading:YES force:YES];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
 
   // Changes to reload immediately.
   [button_ setIsLoading:NO force:YES];
-  EXPECT_EQ([button_ tag], IDC_RELOAD);
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
 
   // Changes to stop immediately when the mouse is hovered, and
   // doesn't change when the mouse exits.
   [button_ mouseEntered:nil];
   EXPECT_TRUE([button_ isMouseInside]);
   [button_ setIsLoading:YES force:YES];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
   [button_ mouseExited:nil];
   EXPECT_FALSE([button_ isMouseInside]);
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
 
   // Changes to reload immediately when the mouse is hovered, and
   // doesn't change when the mouse exits.
   [button_ mouseEntered:nil];
   EXPECT_TRUE([button_ isMouseInside]);
   [button_ setIsLoading:NO force:YES];
-  EXPECT_EQ([button_ tag], IDC_RELOAD);
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
   [button_ mouseExited:nil];
   EXPECT_FALSE([button_ isMouseInside]);
-  EXPECT_EQ([button_ tag], IDC_RELOAD);
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
 }
 
 // Test that without force, stop mode is set immediately, but reload
 // is affected by the hover status.
 TEST_F(ReloadButtonTest, SetIsLoadingNoForceUnHover) {
   EXPECT_FALSE([button_ isMouseInside]);
-  EXPECT_EQ([button_ tag], IDC_RELOAD);
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
 
   // Changes to stop immediately when the mouse is not hovering.
   [button_ setIsLoading:YES force:NO];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
 
   // Changes to reload immediately when the mouse is not hovering.
   [button_ setIsLoading:NO force:NO];
-  EXPECT_EQ([button_ tag], IDC_RELOAD);
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
 
   // Changes to stop immediately when the mouse is hovered, and
   // doesn't change when the mouse exits.
   [button_ mouseEntered:nil];
   EXPECT_TRUE([button_ isMouseInside]);
   [button_ setIsLoading:YES force:NO];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
   [button_ mouseExited:nil];
   EXPECT_FALSE([button_ isMouseInside]);
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
 
   // Does not change to reload immediately when the mouse is hovered,
   // changes when the mouse exits.
   [button_ mouseEntered:nil];
   EXPECT_TRUE([button_ isMouseInside]);
   [button_ setIsLoading:NO force:NO];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
   [button_ mouseExited:nil];
   EXPECT_FALSE([button_ isMouseInside]);
-  EXPECT_EQ([button_ tag], IDC_RELOAD);
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
 }
 
 // Test that without force, stop mode is set immediately, and reload
@@ -201,7 +222,7 @@ TEST_F(ReloadButtonTest, StopAfterReloadSet) {
 
   // Get to stop mode.
   [button_ setIsLoading:YES force:YES];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
   EXPECT_TRUE([button_ isEnabled]);
 
   // Expect the action once.
@@ -213,12 +234,12 @@ TEST_F(ReloadButtonTest, StopAfterReloadSet) {
       test_event_utils::MouseClickInView(button_, 1);
   [NSApp postEvent:click.second atStart:YES];
   [button_ mouseDown:click.first];
-  EXPECT_EQ([button_ tag], IDC_RELOAD);
+  EXPECT_EQ(IDC_RELOAD, [button_ tag]);
   EXPECT_TRUE([button_ isEnabled]);
 
   // Get back to stop mode.
   [button_ setIsLoading:YES force:YES];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
   EXPECT_TRUE([button_ isEnabled]);
 
   // If hover prevented reload mode immediately taking effect, clicks should do
@@ -226,11 +247,11 @@ TEST_F(ReloadButtonTest, StopAfterReloadSet) {
   [button_ mouseEntered:nil];
   EXPECT_TRUE([button_ isMouseInside]);
   [button_ setIsLoading:NO force:NO];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
   EXPECT_FALSE([button_ isEnabled]);
   [NSApp postEvent:click.second atStart:YES];
   [button_ mouseDown:click.first];
-  EXPECT_EQ([button_ tag], IDC_STOP);
+  EXPECT_EQ(IDC_STOP, [button_ tag]);
 
   [button_ setTarget:nil];
 }