Make infobars ignore button clicks when closing.

chrome/browser/ui/views/infobars/infobar_view.cc

 // 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.
 
 #include "chrome/browser/ui/views/infobars/infobar_view.h"
 
 #include <algorithm>
 
 #include "base/memory/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
@@ skipping 50 matching lines @@
 
 InfoBarView::InfoBarView(TabContentsWrapper* owner, InfoBarDelegate* delegate)
     : InfoBar(owner, delegate),
       icon_(NULL),
       close_button_(NULL) {
   set_parent_owned(false);  // InfoBar deletes itself at the appropriate time.
   set_background(new InfoBarBackground(delegate->GetInfoBarType()));
 }
 
 InfoBarView::~InfoBarView() {
+  // We should have closed any open menus in PlatformSpecificHide(), then
+  // subclasses' RunMenu() functions should have prevented opening any new ones
+  // once we became unowned.
+  DCHECK(!menu_runner_.get());
 }
 
 // static
 views::Label* InfoBarView::CreateLabel(const string16& text) {
   views::Label* label = new views::Label(UTF16ToWideHack(text),
       ResourceBundle::GetSharedInstance().GetFont(ResourceBundle::MediumFont));
   label->SetColor(SK_ColorBLACK);
   label->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   return label;
 }
@@ skipping 171 matching lines @@
   // canvas_skia->clipPath(fill_path_);
   DCHECK_EQ(total_height(), height())
       << "Infobar piecewise heights do not match overall height";
   canvas->ClipRectInt(0, arrow_height(), width(), bar_height());
   views::View::PaintChildren(canvas);
   canvas->Restore();
 }
 
 void InfoBarView::ButtonPressed(views::Button* sender,
                                 const views::Event& event) {
+  DCHECK(owned());  // Subclasses should never call us while we're closing.
   if (sender == close_button_) {
-    // If we're not owned, we're already closing, so don't call
-    // InfoBarDismissed(), since this can lead to us double-recording
-    // dismissals.
-    if (delegate() && owned())
-      delegate()->InfoBarDismissed();
+    delegate()->InfoBarDismissed();
     RemoveSelf();
   }
 }
 
 int InfoBarView::ContentMinimumWidth() const {
   return 0;
 }
 
 int InfoBarView::StartX() const {
   // Ensure we don't return a value greater than EndX(), so children can safely
   // set something's width to "EndX() - StartX()" without risking that being
   // negative.
   return std::min(EndX(),
       ((icon_ != NULL) ? icon_->bounds().right() : 0) + kHorizontalPadding);
 }
 
 int InfoBarView::EndX() const {
   const int kCloseButtonSpacing = 12;
   return close_button_->x() - kCloseButtonSpacing;
 }
 
 const InfoBarContainer::Delegate* InfoBarView::container_delegate() const {
   const InfoBarContainer* infobar_container = container();
   return infobar_container ? infobar_container->delegate() : NULL;
 }
 
 void InfoBarView::RunMenuAt(ui::MenuModel* menu_model,
                             views::MenuButton* button,
                             views::MenuItemView::AnchorPosition anchor) {
+  DCHECK(owned());  // We'd better not open any menus while we're closing.
   views::MenuModelAdapter adapter(menu_model);
   gfx::Point screen_point;
   views::View::ConvertPointToScreen(button, &screen_point);
   menu_runner_.reset(new views::MenuRunner(adapter.CreateMenu()));
-  // Ignore the result as we know we can only get here after the menu has
-  // closed.
+  // Ignore the result since we don't need to handle a deleted menu specially.
   ignore_result(menu_runner_->RunMenuAt(
       GetWidget(), button, gfx::Rect(screen_point, button->size()), anchor,
       views::MenuRunner::HAS_MNEMONICS));
-  // TODO(pkasting): this may be deleted after rewrite.
 }
 
 void InfoBarView::PlatformSpecificShow(bool animate) {
   views::Widget* widget = GetWidget();
   views::FocusManager* focus_manager = GetFocusManager();
 #if defined(OS_WIN)
   // If we gain focus, we want to restore it to the previously-focused element
   // when we're hidden.  So when we're in a Widget, create a focus tracker so
   // that if we gain focus we'll know what the previously-focused element was.
   if (widget) {
     focus_tracker_.reset(
         new views::ExternalFocusTracker(this, focus_manager));
   }
 #endif
   if (focus_manager)
     focus_manager->AddFocusChangeListener(this);
   if (widget) {
     widget->NotifyAccessibilityEvent(
         this, ui::AccessibilityTypes::EVENT_ALERT, true);
   }
 }
 
 void InfoBarView::PlatformSpecificHide(bool animate) {
-  // We're being removed. Cancel any menus we may have open.  Because we are
-  // deleted after a delay and after our delegate is deleted we have to
-  // explicitly cancel the menu rather than relying on the destructor to cancel
-  // the menu.
+  // Cancel any menus we may have open.  It doesn't make sense to leave them
+  // open while we're hidden, and if we're going to become unowned, we can't
+  // allow the user to choose any options and potentially call functions that
+  // try to access the owner.
   menu_runner_.reset();
 
   // It's possible to be called twice (once with |animate| true and once with it
   // false); in this case the second RemoveFocusChangeListener() call will
   // silently no-op.
   views::FocusManager* focus_manager = GetFocusManager();
   if (focus_manager)
     focus_manager->RemoveFocusChangeListener(this);
 
 #if defined(OS_WIN) && !defined(USE_AURA)
@@ skipping 30 matching lines @@
 
 void InfoBarView::FocusWillChange(View* focused_before, View* focused_now) {
   // This will trigger some screen readers to read the entire contents of this
   // infobar.
   if (focused_before && focused_now && !Contains(focused_before) &&
       Contains(focused_now) && GetWidget()) {
     GetWidget()->NotifyAccessibilityEvent(
         this, ui::AccessibilityTypes::EVENT_ALERT, true);
   }
 }