summaryrefslogtreecommitdiffstats
path: root/chrome/browser/infobars
diff options
context:
space:
mode:
authorpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-09-30 23:29:18 +0000
committerpkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-09-30 23:29:18 +0000
commitb0349dfbcee4b07bb2d466f4c8d422a26868111d (patch)
treeafd8def3a010d119124eb58e832304067124c997 /chrome/browser/infobars
parentcaf4c05195faad46eaa650c2194b0dc4c3ba80be (diff)
downloadchromium_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.cc12
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) {