diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-30 23:29:18 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-30 23:29:18 +0000 |
commit | b0349dfbcee4b07bb2d466f4c8d422a26868111d (patch) | |
tree | afd8def3a010d119124eb58e832304067124c997 /chrome/browser/infobars | |
parent | caf4c05195faad46eaa650c2194b0dc4c3ba80be (diff) | |
download | chromium_src-b0349dfbcee4b07bb2d466f4c8d422a26868111d.zip chromium_src-b0349dfbcee4b07bb2d466f4c8d422a26868111d.tar.gz chromium_src-b0349dfbcee4b07bb2d466f4c8d422a26868111d.tar.bz2 |
Fix crashes with infobar closure.
I forgot that not all InfoBarView subclasses (e.g. LinkInfoBar) need to override ButtonPressed(). This means the base class has to manually check for an owner.
Also, the theme infobar was erroneously returning true from Cancel() even though it was already closing. This led to RemoveSelf() being called twice. I fixed this and also put a defensive check in RemoveSelf() with a long comment about why it's there.
BUG=98691
TEST=none
Review URL: http://codereview.chromium.org/8093008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103576 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/infobars')
-rw-r--r-- | chrome/browser/infobars/infobar.cc | 12 |
1 files changed, 10 insertions, 2 deletions
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) { |