From bde1160adae6d55fc12d781d5da667a92865d6d4 Mon Sep 17 00:00:00 2001
From: "pkasting@chromium.org"
 <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 4 May 2012 19:05:42 +0000
Subject: Fix unsafe infobar usage pattern in ExtensionDevToolsClientHost. 
 Listening for infobar deletion to determine when it's appropriate to remove
 an infobar from the InfoBarTabHelper doesn't work correctly in two cases: (1)
 when the infobar is in the midst of closing (when it's already unowned but
 not yet deleted) and (2) if it's leaked due to being closed while in a
 background tab (will be fixed someday by refactoring changes I'm still
 waiting to land).

Also safely handle the case of AddInfoBar() failing, by plumbing a return code back.

BUG=none
TEST=none
Review URL: https://chromiumcodereview.appspot.com/10264025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@135383 0039d316-1c4b-4281-b951-d872f2087c98
---
 chrome/browser/infobars/infobar_tab_helper.cc | 16 ++++++++--------
 chrome/browser/infobars/infobar_tab_helper.h  |  8 ++++++--
 2 files changed, 14 insertions(+), 10 deletions(-)

(limited to 'chrome/browser/infobars')

diff --git a/chrome/browser/infobars/infobar_tab_helper.cc b/chrome/browser/infobars/infobar_tab_helper.cc
index 5140ad0..c567cb7 100644
--- a/chrome/browser/infobars/infobar_tab_helper.cc
+++ b/chrome/browser/infobars/infobar_tab_helper.cc
@@ -32,10 +32,10 @@ InfoBarTabHelper::~InfoBarTabHelper() {
   RemoveAllInfoBars(false);
 }
 
-void InfoBarTabHelper::AddInfoBar(InfoBarDelegate* delegate) {
+bool InfoBarTabHelper::AddInfoBar(InfoBarDelegate* delegate) {
   if (!infobars_enabled_) {
     delegate->InfoBarClosed();
-    return;
+    return false;
   }
 
   for (InfoBars::const_iterator i(infobars_.begin()); i != infobars_.end();
@@ -43,7 +43,7 @@ void InfoBarTabHelper::AddInfoBar(InfoBarDelegate* delegate) {
     if ((*i)->EqualsDelegate(delegate)) {
       DCHECK_NE(*i, delegate);
       delegate->InfoBarClosed();
-      return;
+      return false;
     }
   }
 
@@ -64,18 +64,17 @@ void InfoBarTabHelper::AddInfoBar(InfoBarDelegate* delegate) {
       chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_ADDED,
       content::Source<InfoBarTabHelper>(this),
       content::Details<InfoBarAddedDetails>(delegate));
+  return true;
 }
 
 void InfoBarTabHelper::RemoveInfoBar(InfoBarDelegate* delegate) {
   RemoveInfoBarInternal(delegate, true);
 }
 
-void InfoBarTabHelper::ReplaceInfoBar(InfoBarDelegate* old_delegate,
+bool InfoBarTabHelper::ReplaceInfoBar(InfoBarDelegate* old_delegate,
                                       InfoBarDelegate* new_delegate) {
-  if (!infobars_enabled_) {
-    AddInfoBar(new_delegate);  // Deletes the delegate.
-    return;
-  }
+  if (!infobars_enabled_)
+    return AddInfoBar(new_delegate);  // Deletes the delegate.
 
   InfoBars::iterator i(std::find(infobars_.begin(), infobars_.end(),
                                  old_delegate));
@@ -92,6 +91,7 @@ void InfoBarTabHelper::ReplaceInfoBar(InfoBarDelegate* old_delegate,
       chrome::NOTIFICATION_TAB_CONTENTS_INFOBAR_REPLACED,
       content::Source<InfoBarTabHelper>(this),
       content::Details<InfoBarReplacedDetails>(&replaced_details));
+  return true;
 }
 
 InfoBarDelegate* InfoBarTabHelper::GetInfoBarDelegateAt(size_t index) {
diff --git a/chrome/browser/infobars/infobar_tab_helper.h b/chrome/browser/infobars/infobar_tab_helper.h
index 05d94d0..2f7d3a6 100644
--- a/chrome/browser/infobars/infobar_tab_helper.h
+++ b/chrome/browser/infobars/infobar_tab_helper.h
@@ -25,7 +25,9 @@ class InfoBarTabHelper : public content::WebContentsObserver,
   // If infobars are disabled for this tab or the tab already has a delegate
   // which returns true for InfoBarDelegate::EqualsDelegate(delegate),
   // |delegate| is closed immediately without being added.
-  void AddInfoBar(InfoBarDelegate* delegate);
+  //
+  // Returns whether |delegate| was successfully added.
+  bool AddInfoBar(InfoBarDelegate* delegate);
 
   // Removes the InfoBar for the specified |delegate|.
   //
@@ -39,8 +41,10 @@ class InfoBarTabHelper : public content::WebContentsObserver,
   // If infobars are disabled for this tab, |new_delegate| is closed immediately
   // without being added, and nothing else happens.
   //
+  // Returns whether |new_delegate| was successfully added.
+  //
   // NOTE: This does not perform any EqualsDelegate() checks like AddInfoBar().
-  void ReplaceInfoBar(InfoBarDelegate* old_delegate,
+  bool ReplaceInfoBar(InfoBarDelegate* old_delegate,
                       InfoBarDelegate* new_delegate);
 
   // Enumeration and access functions.
-- 
cgit v1.1