diff options
Diffstat (limited to 'chrome')
6 files changed, 40 insertions, 24 deletions
diff --git a/chrome/browser/extensions/theme_installed_infobar_delegate.cc b/chrome/browser/extensions/theme_installed_infobar_delegate.cc index fa3b884..9254d1f 100644 --- a/chrome/browser/extensions/theme_installed_infobar_delegate.cc +++ b/chrome/browser/extensions/theme_installed_infobar_delegate.cc @@ -55,16 +55,15 @@ bool ThemeInstalledInfoBarDelegate::Cancel() { extension_service_->GetExtensionById(previous_theme_id_, true); if (previous_theme) { theme_service_->SetTheme(previous_theme); - return true; + return false; // The theme change will close us. } } - if (previous_using_native_theme_) { + if (previous_using_native_theme_) theme_service_->SetNativeTheme(); - } else { + else theme_service_->UseDefaultTheme(); - } - return true; + return false; // The theme change will close us. } gfx::Image* ThemeInstalledInfoBarDelegate::GetIcon() const { diff --git a/chrome/browser/infobars/infobar.cc b/chrome/browser/infobars/infobar.cc index f909d23..9fb30f5 100644 --- a/chrome/browser/infobars/infobar.cc +++ b/chrome/browser/infobars/infobar.cc @@ -102,8 +102,16 @@ void InfoBar::AnimationProgressed(const ui::Animation* animation) { } void InfoBar::RemoveSelf() { - DCHECK(owner_); - owner_->RemoveInfoBar(delegate_); + // |owner_| should never be NULL here. If it is, then someone violated what + // they were supposed to do -- e.g. a ConfirmInfoBarDelegate subclass returned + // true from Accept() or Cancel() even though the infobar was already closing. + // In the worst case, if we also switched tabs during that process, then + // |this| has already been destroyed. But if that's the case, then we're + // going to deref a garbage |this| pointer here whether we check |owner_| or + // not, and in other cases (where we're still closing and |this| is valid), + // checking |owner_| here will avoid a NULL deref. + if (owner_) + owner_->RemoveInfoBar(delegate_); } void InfoBar::SetBarTargetHeight(int height) { diff --git a/chrome/browser/tab_contents/confirm_infobar_delegate.h b/chrome/browser/tab_contents/confirm_infobar_delegate.h index c39f1e1..593fe3d 100644 --- a/chrome/browser/tab_contents/confirm_infobar_delegate.h +++ b/chrome/browser/tab_contents/confirm_infobar_delegate.h @@ -34,25 +34,25 @@ class ConfirmInfoBarDelegate : public InfoBarDelegate { // Return whether or not the specified button needs elevation. virtual bool NeedElevation(InfoBarButton button) const; - // Called when the OK button is pressed. If the function returns true, the - // InfoBarDelegate should be removed from the associated TabContentsWrapper. + // Called when the OK button is pressed. If this function returns true, the + // infobar is then immediately closed. Subclasses MUST NOT return true if in + // handling this call something triggers the infobar to begin closing. virtual bool Accept(); - // Called when the Cancel button is pressed. If the function returns true, - // the InfoBarDelegate should be removed from the associated - // TabContentsWrapper. + // Called when the Cancel button is pressed. If this function returns true, + // the infobar is then immediately closed. Subclasses MUST NOT return true if + // in handling this call something triggers the infobar to begin closing. virtual bool Cancel(); // Returns the text of the link to be displayed, if any. Otherwise returns // and empty string. virtual string16 GetLinkText() const; - // Called when the Link is clicked. The |disposition| specifies how the - // resulting document should be loaded (based on the event flags present when - // the link was clicked). This function returns true if the InfoBar should be - // closed now or false if it should remain until the user explicitly closes - // it. - // Will only be called if GetLinkText() returns non-empty string. + // Called when the Link (if any) is clicked. The |disposition| specifies how + // the resulting document should be loaded (based on the event flags present + // when the link was clicked). If this function returns true, the infobar is + // then immediately closed. Subclasses MUST NOT return true if in handling + // this call something triggers the infobar to begin closing. virtual bool LinkClicked(WindowOpenDisposition disposition); protected: diff --git a/chrome/browser/tab_contents/link_infobar_delegate.h b/chrome/browser/tab_contents/link_infobar_delegate.h index 0a20f13..e4d7d33 100644 --- a/chrome/browser/tab_contents/link_infobar_delegate.h +++ b/chrome/browser/tab_contents/link_infobar_delegate.h @@ -26,9 +26,9 @@ class LinkInfoBarDelegate : public InfoBarDelegate { // Called when the Link is clicked. The |disposition| specifies how the // resulting document should be loaded (based on the event flags present when - // the link was clicked). This function returns true if the InfoBar should be - // closed now or false if it should remain until the user explicitly closes - // it. + // the link was clicked). If this function returns true, the infobar is then + // immediately closed. Subclasses MUST NOT return true if in handling this + // call something triggers the infobar to begin closing. virtual bool LinkClicked(WindowOpenDisposition disposition); protected: diff --git a/chrome/browser/ui/cocoa/infobars/infobar_controller.mm b/chrome/browser/ui/cocoa/infobars/infobar_controller.mm index d7e8177..c81a363 100644 --- a/chrome/browser/ui/cocoa/infobars/infobar_controller.mm +++ b/chrome/browser/ui/cocoa/infobars/infobar_controller.mm @@ -126,8 +126,16 @@ const float kAnimateCloseDuration = 0.12; } - (void)removeSelf { - DCHECK(owner_); - owner_->RemoveInfoBar(delegate_); + // |owner_| should never be NULL here. If it is, then someone violated what + // they were supposed to do -- e.g. a ConfirmInfoBarDelegate subclass returned + // true from Accept() or Cancel() even though the infobar was already closing. + // In the worst case, if we also switched tabs during that process, then + // |this| has already been destroyed. But if that's the case, then we're + // going to deref a garbage |this| pointer here whether we check |owner_| or + // not, and in other cases (where we're still closing and |this| is valid), + // checking |owner_| here will avoid a NULL deref. + if (owner_) + owner_->RemoveInfoBar(delegate_); } - (AnimatableView*)animatableView { diff --git a/chrome/browser/ui/views/infobars/infobar_view.cc b/chrome/browser/ui/views/infobars/infobar_view.cc index 2d911e2..6d8209c 100644 --- a/chrome/browser/ui/views/infobars/infobar_view.cc +++ b/chrome/browser/ui/views/infobars/infobar_view.cc @@ -263,7 +263,8 @@ void InfoBarView::PaintChildren(gfx::Canvas* canvas) { void InfoBarView::ButtonPressed(views::Button* sender, const views::Event& event) { - DCHECK(owned()); // Subclasses should never call us while we're closing. + if (!owned()) + return; // We're closing; don't call anything, it might access the owner. if (sender == close_button_) { delegate()->InfoBarDismissed(); RemoveSelf(); |