Fix up reload button to not flicker, and simplify implementation.

chrome/browser/ui/cocoa/toolbar/reload_button.mm

Index: chrome/browser/ui/cocoa/toolbar/reload_button.mm
diff --git a/chrome/browser/ui/cocoa/toolbar/reload_button.mm b/chrome/browser/ui/cocoa/toolbar/reload_button.mm
index 10cd47ff4cb33a7819c6b33a9c012ba42d192823..7960ec79a1f835eee52812cc8e186b464b0436a9 100644
--- a/chrome/browser/ui/cocoa/toolbar/reload_button.mm
+++ b/chrome/browser/ui/cocoa/toolbar/reload_button.mm
@@ -5,7 +5,6 @@
 #import "chrome/browser/ui/cocoa/toolbar/reload_button.h"
 
 #include "chrome/app/chrome_command_ids.h"
-#import "chrome/browser/ui/cocoa/image_button_cell.h"
 #import "chrome/browser/ui/cocoa/view_id_util.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
@@ -20,155 +19,163 @@ NSTimeInterval kPendingReloadTimeout = 1.35;
 
 }  // namespace
 
+@interface ReloadButton ()
+- (void)invalidatePendingReloadTimer;
+- (void)forceReloadState:(NSTimer *)timer;
+@end
+
 @implementation ReloadButton
 
 + (Class)cellClass {
   return [ImageButtonCell class];
 }
 
-- (void)dealloc {
-  if (trackingArea_) {
-    [self removeTrackingArea:trackingArea_];
-    trackingArea_.reset();
+- (id)initWithFrame:(NSRect)frameRect {
+  if ((self = [super initWithFrame:frameRect])) {
+    // Since this is not a custom view, -awakeFromNib won't be called twice.
+    [self awakeFromNib];
   }
-  [super dealloc];
+  return self;
 }
 
-- (void)updateTrackingAreas {
-  // If the mouse is hovering when the tracking area is updated, the
-  // control could end up locked into inappropriate behavior for
-  // awhile, so unwind state.
-  if (isMouseInside_)
-    [self mouseExited:nil];
-
-  if (trackingArea_) {
-    [self removeTrackingArea:trackingArea_];
-    trackingArea_.reset();
-  }
-  trackingArea_.reset([[NSTrackingArea alloc]
-                        initWithRect:[self bounds]
-                             options:(NSTrackingMouseEnteredAndExited |
-                                      NSTrackingActiveInActiveApp)
-                               owner:self
-                            userInfo:nil]);
-  [self addTrackingArea:trackingArea_];
+- (void)viewWillMoveToWindow:(NSWindow *)newWindow {
+  // If this view is moved to a new window, reset its state.
+  [self setIsLoading:NO force:YES];
+  [super viewWillMoveToWindow:newWindow];
 }
 
 - (void)awakeFromNib {
-  [self updateTrackingAreas];
-
   // Don't allow multi-clicks, because the user probably wouldn't ever
   // want to stop+reload or reload+stop.
   [self setIgnoresMultiClick:YES];
 }
 
+- (void)invalidatePendingReloadTimer {
+  [pendingReloadTimer_ invalidate];
+  pendingReloadTimer_ = nil;
+}
+
 - (void)updateTag:(NSInteger)anInt {
   if ([self tag] == anInt)
     return;
 
   // Forcibly remove any stale tooltip which is being displayed.
   [self removeAllToolTips];
-
+  id cell = [self cell];
   [self setTag:anInt];
   if (anInt == IDC_RELOAD) {
-    [[self cell] setImageID:IDR_RELOAD
-             forButtonState:image_button_cell::kDefaultState];
-    [[self cell] setImageID:IDR_RELOAD_H
-             forButtonState:image_button_cell::kHoverState];
-    [[self cell] setImageID:IDR_RELOAD_P
-             forButtonState:image_button_cell::kPressedState];
+    [cell setImageID:IDR_RELOAD
+      forButtonState:image_button_cell::kDefaultState];
+    [cell setImageID:IDR_RELOAD_H
+      forButtonState:image_button_cell::kHoverState];
+    [cell setImageID:IDR_RELOAD_P
+      forButtonState:image_button_cell::kPressedState];
     // The stop button has a disabled image but the reload button doesn't. To
     // unset it we have to explicilty change the image ID to 0.
-    [[self cell] setImageID:0
-             forButtonState:image_button_cell::kDisabledState];
+    [cell setImageID:0
+      forButtonState:image_button_cell::kDisabledState];
     [self setToolTip:l10n_util::GetNSStringWithFixup(IDS_TOOLTIP_RELOAD)];
   } else if (anInt == IDC_STOP) {
-    [[self cell] setImageID:IDR_STOP
-             forButtonState:image_button_cell::kDefaultState];
-    [[self cell] setImageID:IDR_STOP_H
-             forButtonState:image_button_cell::kHoverState];
-    [[self cell] setImageID:IDR_STOP_P
-             forButtonState:image_button_cell::kPressedState];
-    [[self cell] setImageID:IDR_STOP_D
-             forButtonState:image_button_cell::kDisabledState];
+    [cell setImageID:IDR_STOP
+      forButtonState:image_button_cell::kDefaultState];
+    [cell setImageID:IDR_STOP_H
+      forButtonState:image_button_cell::kHoverState];
+    [cell setImageID:IDR_STOP_P
+      forButtonState:image_button_cell::kPressedState];
+    [cell setImageID:IDR_STOP_D
+      forButtonState:image_button_cell::kDisabledState];
     [self setToolTip:l10n_util::GetNSStringWithFixup(IDS_TOOLTIP_STOP)];
   } else {
     NOTREACHED();
   }
 }
 
+- (id)accessibilityAttributeValue:(NSString *)attribute {
+  if ([attribute isEqualToString:NSAccessibilityEnabledAttribute] &&
+      pendingReloadTimer_) {
+    return [NSNumber numberWithBool:NO];
+  } else {
+    return [super accessibilityAttributeValue:attribute];
+  }
+}
+
 - (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 invalidatePendingReloadTimer];
     [self updateTag:IDC_STOP];
-    [self setEnabled:YES];
-  } else if (force || ![self isMouseInside]) {
-    pendingReloadTimer_.reset();
+  } else if (force) {
+    [self invalidatePendingReloadTimer];
     [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
-    // sure the cell's sense of mouse-inside matches the local sense, to prevent
-    // drawing artifacts.
+  } else if ([self tag] == IDC_STOP &&
+             !pendingReloadTimer_ &&
+             [[self cell] isMouseInside]) {
     id cell = [self cell];
-    if ([cell respondsToSelector:@selector(setIsMouseInside:)])
-      [cell setIsMouseInside:[self isMouseInside]];
-    [self setEnabled:YES];
-  } else if ([self tag] == IDC_STOP && !pendingReloadTimer_) {
-    [self setEnabled:NO];
-    pendingReloadTimer_.reset(
-        [[NSTimer scheduledTimerWithTimeInterval:kPendingReloadTimeout
-                                          target:self
-                                        selector:@selector(forceReloadState)
-                                        userInfo:nil
-                                         repeats:NO] retain]);
+    [cell setImageID:IDR_STOP_D
+      forButtonState:image_button_cell::kDefaultState];
+    [cell setImageID:IDR_STOP_D
+      forButtonState:image_button_cell::kDisabledState];
+    [cell setImageID:IDR_STOP_D
+      forButtonState:image_button_cell::kHoverState];
+    [cell setImageID:IDR_STOP_D
+      forButtonState:image_button_cell::kPressedState];
+    pendingReloadTimer_ =
+        [NSTimer timerWithTimeInterval:kPendingReloadTimeout
+                                target:self
+                              selector:@selector(forceReloadState:)
+                              userInfo:nil
+                               repeats:NO];
+    // Must add the timer to |NSRunLoopCommonModes| because
+    // it should run in |NSEventTrackingRunLoopMode| as well as
+    // |NSDefaultRunLoopMode|.
+    [[NSRunLoop currentRunLoop] addTimer:pendingReloadTimer_
+                                 forMode:NSRunLoopCommonModes];
+  } else {
+    [self invalidatePendingReloadTimer];
+    [self updateTag:IDC_RELOAD];
   }
+  [self setEnabled:pendingReloadTimer_ == nil];
 }
 
-- (void)forceReloadState {
+- (void)forceReloadState:(NSTimer *)timer {
+  DCHECK_EQ(timer, pendingReloadTimer_);
   [self setIsLoading:NO force:YES];
+  // Verify that |pendingReloadTimer_| is nil so it is not left dangling.
+  DCHECK(!pendingReloadTimer_);
 }
 
 - (BOOL)sendAction:(SEL)theAction to:(id)theTarget {
   if ([self tag] == IDC_STOP) {
-    // When the timer is started, the button is disabled, so this
-    // should not be possible.
-    DCHECK(!pendingReloadTimer_.get());
-
-    // When the stop is processed, immediately change to reload mode,
-    // even though the IPC still has to bounce off the renderer and
-    // back before the regular |-setIsLoaded:force:| will be called.
-    // [This is how views and gtk do it.]
-    const BOOL ret = [super sendAction:theAction to:theTarget];
-    if (ret)
-      [self forceReloadState];
-    return ret;
+    if (pendingReloadTimer_) {
+      // If |pendingReloadTimer_| then the control is currently being
+      // drawn in a disabled state, so just return. The control is NOT actually
+      // disabled, otherwise mousetracking (courtesy of the NSButtonCell)
+      // would not work.
+      return YES;
+    } else {
+      // When the stop is processed, immediately change to reload mode,
+      // even though the IPC still has to bounce off the renderer and
+      // back before the regular |-setIsLoaded:force:| will be called.
+      // [This is how views and gtk do it.]
+      BOOL ret = [super sendAction:theAction to:theTarget];
+      if (ret)
+        [self forceReloadState:pendingReloadTimer_];

[Scott Hess - ex-Googler, 2011/08/04 23:49:44]

This seems contrived (pendingReloadTimer_ is nil, which matches in
-forceReloadState:, but still).  WDYT about just calling -setIsLoading:NO
force:YES directly?  Though I suppose that makes it a bit more brittle.

+      return ret;
+    }
   }
 
   return [super sendAction:theAction to:theTarget];
 }
 
-- (void)mouseEntered:(NSEvent*)theEvent {
-  isMouseInside_ = YES;
+- (ViewID)viewID {
+  return VIEW_ID_RELOAD_BUTTON;
 }
 
-- (void)mouseExited:(NSEvent*)theEvent {
-  isMouseInside_ = NO;
-
-  // Reload mode was requested during the hover.
+- (void)mouseInsideStateDidChange:(BOOL)isInside {
   if (pendingReloadTimer_)
-    [self forceReloadState];
-}
-
-- (BOOL)isMouseInside {
-  return isMouseInside_;
-}
-
-- (ViewID)viewID {
-  return VIEW_ID_RELOAD_BUTTON;
+    [pendingReloadTimer_ fire];

[Scott Hess - ex-Googler, 2011/08/04 23:49:44]

No need for the if(), and this can't possibly happen enough to be noticeable.

 }
 
 @end  // ReloadButton
@@ -179,8 +186,4 @@ NSTimeInterval kPendingReloadTimeout = 1.35;
   kPendingReloadTimeout = seconds;
 }
 
-- (NSTrackingArea*)trackingArea {
-  return trackingArea_;
-}
-
 @end