Make infobars ignore button clicks when closing.

chrome/browser/ui/cocoa/infobars/infobar_controller.mm

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import <Cocoa/Cocoa.h>
 
 #include "base/logging.h"  // for NOTREACHED()
 #include "base/mac/mac_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/infobars/infobar_tab_helper.h"
@@ skipping 16 matching lines @@
 namespace {
 // Durations set to match the default SlideAnimation duration.
 const float kAnimateOpenDuration = 0.12;
 const float kAnimateCloseDuration = 0.12;
 }
 
 @interface InfoBarController (PrivateMethods)
 // Sets |label_| based on |labelPlaceholder_|, sets |labelPlaceholder_| to nil.
 - (void)initializeLabel;
 
-// Asks the container controller to remove the infobar for this delegate.  This
-// call will trigger a notification that starts the infobar animating closed.
-- (void)removeSelf;
+// Called when someone clicks on the close button.  Dismisses the
+// infobar without taking any action.
+- (IBAction)dismiss:(id)sender;
 
 // Performs final cleanup after an animation is finished or stopped, including
 // notifying the InfoBarDelegate that the infobar was closed and removing the
 // infobar from its container, if necessary.
 - (void)cleanUpAfterAnimation:(BOOL)finished;
 
 // Returns the point, in gradient view coordinates, at which the apex of the
 // infobar tip should be drawn.
 - (NSPoint)pointForTipApex;
 @end
@@ skipping 48 matching lines @@
 
 // Called when someone clicks on the embedded link.
 - (BOOL) textView:(NSTextView*)textView
     clickedOnLink:(id)link
           atIndex:(NSUInteger)charIndex {
   if ([self respondsToSelector:@selector(linkClicked)])
     [self performSelector:@selector(linkClicked)];
   return YES;
 }
 
+- (bool)owned {
+  return !!owner_;
+}
+
 // Called when someone clicks on the ok button.
 - (void)ok:(id)sender {
   // Subclasses must override this method if they do not hide the ok button.
   NOTREACHED();
 }
 
 // Called when someone clicks on the cancel button.
 - (void)cancel:(id)sender {
   // Subclasses must override this method if they do not hide the cancel button.
   NOTREACHED();
 }
 
-// Called when someone clicks on the close button.
-- (void)dismiss:(id)sender {
-  if (delegate_)
-    delegate_->InfoBarDismissed();
-
-  [self removeSelf];
+- (void)removeSelf {
+  DCHECK(owner_);
+  owner_->infobar_tab_helper()->RemoveInfoBar(delegate_);
 }
 
 - (AnimatableView*)animatableView {
   return static_cast<AnimatableView*>([self view]);
 }
 
 - (void)open {
   // Simply reset the frame size to its opened size, forcing a relayout.
   CGFloat finalHeight = [[self view] frame].size.height;
   [[self animatableView] setHeight:finalHeight];
@@ skipping 22 matching lines @@
 
   // Start animating closed.  We will receive a notification when the animation
   // is done, at which point we can remove our view from the hierarchy and
   // notify the delegate that the infobar was closed.
   [[self animatableView] animateToNewHeight:0 duration:kAnimateCloseDuration];
 
   // The above call may trigger an animationDidStop: notification for any
   // currently-running animations, so do not set |infoBarClosing_| until after
   // starting the animation.
   infoBarClosing_ = YES;
-
-  // The owner called this method to close the infobar, so there will
-  // be no need to forward future remove events to the owner.
-  owner_ = NULL;
 }
 
 - (void)addAdditionalControls {
   // Default implementation does nothing.
 }
 
 - (void)infobarWillClose {
-  // Default implementation does nothing.
+  owner_ = NULL;
 }
 
 - (void)removeButtons {
   // Extend the label all the way across.
   NSRect labelFrame = [label_.get() frame];
   labelFrame.size.width = NSMaxX([cancelButton_ frame]) - NSMinX(labelFrame);
   [okButton_ removeFromSuperview];
   okButton_ = nil;
   [cancelButton_ removeFromSuperview];
   cancelButton_ = nil;
@@ skipping 11 matching lines @@
   // programmatically.
   label_.reset([[HyperlinkTextView alloc]
       initWithFrame:[labelPlaceholder_ frame]]);
   [label_.get() setAutoresizingMask:[labelPlaceholder_ autoresizingMask]];
   [[labelPlaceholder_ superview]
       replaceSubview:labelPlaceholder_ with:label_.get()];
   labelPlaceholder_ = nil;  // Now released.
   [label_.get() setDelegate:self];
 }
 
-- (void)removeSelf {
-  // TODO(rohitrao): This method can be called even if the infobar has already
-  // been removed and |delegate_| is NULL.  Is there a way to rewrite the code
-  // so that inner event loops don't cause us to try and remove the infobar
-  // twice?  http://crbug.com/54253
-  if (owner_)
-    owner_->infobar_tab_helper()->RemoveInfoBar(delegate_);
-  owner_ = NULL;
+// Called when someone clicks on the close button.
+- (void)dismiss:(id)sender {
+  if (![self owned])
+    return;  // We're closing; don't call anything, it might access the owner.
+  delegate_->InfoBarDismissed();
+  [self removeSelf];
 }
 
 - (void)cleanUpAfterAnimation:(BOOL)finished {
   // Don't need to do any cleanup if the bar was animating open.
   if (!infoBarClosing_)
     return;
 
   // Notify the delegate that the infobar was closed.  The delegate will delete
   // itself as a result of InfoBarClosed(), so we null out its pointer.
   if (delegate_) {
@@ skipping 62 matching lines @@
                  atOffset:offset
                      font:font
              messageColor:[NSColor blackColor]
                 linkColor:[NSColor blueColor]];
 }
 
 // Called when someone clicks on the link in the infobar.  This method
 // is called by the InfobarTextField on its delegate (the
 // LinkInfoBarController).
 - (void)linkClicked {
+  if (![self owned])
+    return;  // We're closing; don't call anything, it might access the owner.
   WindowOpenDisposition disposition =
       event_utils::WindowOpenDispositionFromNSEvent([NSApp currentEvent]);
-  if (delegate_ &&
-      delegate_->AsLinkInfoBarDelegate()->LinkClicked(disposition)) {
-    // Call |-removeSelf| on the outermost runloop to force a delay. As shown in
-    // <http://crbug.com/87201>, the second click event can be delivered after
-    // the animation finishes (and this gets released and deallocated), which
-    // leads to zombie messaging. Unfortunately, the order between the animation
-    // finishing and the click event being delivered is nondeterministic, so
-    // this hack is the best that can be done.
-    [self performSelector:@selector(removeSelf)
-               withObject:nil
-               afterDelay:0.0];
-  }
+  if (delegate_->AsLinkInfoBarDelegate()->LinkClicked(disposition))
+    [self removeSelf];
 }
 
 @end
 
 
 /////////////////////////////////////////////////////////////////////////
 // ConfirmInfoBarController implementation
 
 @implementation ConfirmInfoBarController
 
 // Called when someone clicks on the "OK" button.
 - (IBAction)ok:(id)sender {
-  if (delegate_ && delegate_->AsConfirmInfoBarDelegate()->Accept())
+  if (![self owned])
+    return;  // We're closing; don't call anything, it might access the owner.
+  if (delegate_->AsConfirmInfoBarDelegate()->Accept())
     [self removeSelf];
 }
 
 // Called when someone clicks on the "Cancel" button.
 - (IBAction)cancel:(id)sender {
-  if (delegate_ && delegate_->AsConfirmInfoBarDelegate()->Cancel())
+  if (![self owned])
+    return;  // We're closing; don't call anything, it might access the owner.
+  if (delegate_->AsConfirmInfoBarDelegate()->Cancel())
     [self removeSelf];
 }
 
 // Confirm infobars can have OK and/or cancel buttons, depending on
 // the return value of GetButtons().  We create each button if
 // required and position them to the left of the close button.
 - (void)addAdditionalControls {
   ConfirmInfoBarDelegate* delegate = delegate_->AsConfirmInfoBarDelegate();
   DCHECK(delegate);
   int visibleButtons = delegate->GetButtons();
@@ skipping 78 matching lines @@
                  atOffset:[message length]
                      font:font
              messageColor:[NSColor blackColor]
                 linkColor:[NSColor blueColor]];
 }
 
 // Called when someone clicks on the link in the infobar.  This method
 // is called by the InfobarTextField on its delegate (the
 // LinkInfoBarController).
 - (void)linkClicked {
+  if (![self owned])
+    return;  // We're closing; don't call anything, it might access the owner.
   WindowOpenDisposition disposition =
       event_utils::WindowOpenDispositionFromNSEvent([NSApp currentEvent]);
-  if (delegate_ &&
-      delegate_->AsConfirmInfoBarDelegate()->LinkClicked(disposition))
+  if (delegate_->AsConfirmInfoBarDelegate()->LinkClicked(disposition))
     [self removeSelf];
 }
 
 @end
 
 
 //////////////////////////////////////////////////////////////////////////
 // CreateInfoBar() implementations
 
 InfoBar* LinkInfoBarDelegate::CreateInfoBar(TabContentsWrapper* owner) {
   LinkInfoBarController* controller =
       [[LinkInfoBarController alloc] initWithDelegate:this owner:owner];
   return new InfoBar(controller, this);
 }
 
 InfoBar* ConfirmInfoBarDelegate::CreateInfoBar(TabContentsWrapper* owner) {
   ConfirmInfoBarController* controller =
       [[ConfirmInfoBarController alloc] initWithDelegate:this owner:owner];
   return new InfoBar(controller, this);
 }