diff options
| author | tim@chromium.org <tim@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-01-21 23:20:48 +0000 |
|---|---|---|
| committer | tim@chromium.org <tim@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-01-21 23:20:48 +0000 |
| commit | b618713c323f0a5e85fb53300719e8ff1cdb410b (patch) | |
| tree | 6782542670381c37d6b64667268f092470c57efc | |
| parent | 930eefad81129f1c8db7621f8a02e02b855adf90 (diff) | |
| download | chromium_src-b618713c323f0a5e85fb53300719e8ff1cdb410b.zip chromium_src-b618713c323f0a5e85fb53300719e8ff1cdb410b.tar.gz chromium_src-b618713c323f0a5e85fb53300719e8ff1cdb410b.tar.bz2 | |
Fix issue 6520 by ensuring that InfoBarClosed is called whenever a tab is closed or a navigation to a URL that is not supported by the current TabContentsType takes place by invoking it on all InfoBarDelegates from TabContents::Destroy(), which is the best least-common-denominator I've found for doing this work. Alternatively I could have made InfoBarDelegate listen for TAB_CONTENTS_DESTROYED and self-invoke InfoBarClosed but that seemed unorthodox (relatively speaking).
This prevents InfoBarDelegates from leaking in these cases.
BUG=6520
Review URL: http://codereview.chromium.org/18381
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@8406 0039d316-1c4b-4281-b951-d872f2087c98
| -rw-r--r-- | chrome/browser/alternate_nav_url_fetcher.cc | 9 | ||||
| -rw-r--r-- | chrome/browser/password_manager/password_manager.cc | 17 | ||||
| -rw-r--r-- | chrome/browser/ssl/ssl_manager.cc | 2 | ||||
| -rw-r--r-- | chrome/browser/tab_contents/infobar_delegate.h | 2 | ||||
| -rw-r--r-- | chrome/browser/tab_contents/tab_contents.cc | 9 |
5 files changed, 12 insertions, 27 deletions
diff --git a/chrome/browser/alternate_nav_url_fetcher.cc b/chrome/browser/alternate_nav_url_fetcher.cc index 24e6a4b..d7642b6 100644 --- a/chrome/browser/alternate_nav_url_fetcher.cc +++ b/chrome/browser/alternate_nav_url_fetcher.cc @@ -39,8 +39,6 @@ void AlternateNavURLFetcher::Observe(NotificationType type, NotificationService::AllSources()); registrar_.Add(this, NOTIFY_NAV_ENTRY_COMMITTED, Source<NavigationController>(controller_)); - registrar_.Add(this, NOTIFY_TAB_CLOSED, - Source<NavigationController>(controller_)); DCHECK_EQ(NOT_STARTED, state_); state_ = IN_PROGRESS; @@ -58,13 +56,6 @@ void AlternateNavURLFetcher::Observe(NotificationType type, ShowInfobarIfPossible(); break; - case NOTIFY_TAB_CLOSED: - // We listen either for tab closed or navigation committed to know when to - // delete ourselves. Here, the tab closed, so we can just give up with - // all our waiting for notifications and delete ourselves. - delete this; - break; - default: NOTREACHED(); } diff --git a/chrome/browser/password_manager/password_manager.cc b/chrome/browser/password_manager/password_manager.cc index 8756a34..87ed5f2 100644 --- a/chrome/browser/password_manager/password_manager.cc +++ b/chrome/browser/password_manager/password_manager.cc @@ -25,15 +25,12 @@ // login is one we already know about, the end of the line is // provisional_save_manager_ because we just update it on success and so such // forms never end up in an infobar. -class SavePasswordInfoBarDelegate : public ConfirmInfoBarDelegate, - public NotificationObserver { +class SavePasswordInfoBarDelegate : public ConfirmInfoBarDelegate { public: SavePasswordInfoBarDelegate(TabContents* tab_contents, PasswordFormManager* form_to_save) : ConfirmInfoBarDelegate(tab_contents), form_to_save_(form_to_save) { - registrar_.Add(this, NOTIFY_TAB_CLOSED, - Source<NavigationController>(tab_contents->controller())); } virtual ~SavePasswordInfoBarDelegate() { } @@ -76,24 +73,12 @@ class SavePasswordInfoBarDelegate : public ConfirmInfoBarDelegate, form_to_save_->PermanentlyBlacklist(); return true; } - - // NotificationObserver - virtual void Observe(NotificationType type, const NotificationSource& source, - const NotificationDetails& details) { - DCHECK(type == NOTIFY_TAB_CLOSED); - // We don't get InfoBarClosed notification when a tab is closed, so we need - // to clean up after ourselves now. - // TODO(timsteele): This should not be necessary; see http://crbug.com/6520 - delete this; - } private: // The PasswordFormManager managing the form we're asking the user about, // and should update as per her decision. scoped_ptr<PasswordFormManager> form_to_save_; - NotificationRegistrar registrar_; - DISALLOW_COPY_AND_ASSIGN(SavePasswordInfoBarDelegate); }; diff --git a/chrome/browser/ssl/ssl_manager.cc b/chrome/browser/ssl/ssl_manager.cc index 835e3f0..b2fd246 100644 --- a/chrome/browser/ssl/ssl_manager.cc +++ b/chrome/browser/ssl/ssl_manager.cc @@ -34,8 +34,6 @@ #include "webkit/glue/resource_type.h" #include "generated_resources.h" -// TODO(timsteele): SSLInfoBarDelegate can leak when a tab is closed, see -// http://crbug.com/6520 class SSLInfoBarDelegate : public ConfirmInfoBarDelegate { public: SSLInfoBarDelegate(TabContents* contents, diff --git a/chrome/browser/tab_contents/infobar_delegate.h b/chrome/browser/tab_contents/infobar_delegate.h index fa3f332..5ec5309 100644 --- a/chrome/browser/tab_contents/infobar_delegate.h +++ b/chrome/browser/tab_contents/infobar_delegate.h @@ -85,6 +85,8 @@ class InfoBarDelegate { // stored using StoreActiveEntryUniqueID automatically. explicit InfoBarDelegate(TabContents* contents); + virtual ~InfoBarDelegate() { } + // Store the unique id for the active entry in the specified TabContents, to // be used later upon navigation to determine if this InfoBarDelegate should // be expired from |contents_|. diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index 70988e2..8afcfef 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -88,6 +88,15 @@ void TabContents::Destroy() { window->CloseConstrainedWindow(); } + // Notify any lasting InfobarDelegates that have not yet been removed that + // whatever infobar they were handling in this TabContents has closed, + // because the TabContents is going away entirely. + for (int i = 0; i < infobar_delegate_count(); ++i) { + InfoBarDelegate* delegate = GetInfoBarDelegateAt(i); + delegate->InfoBarClosed(); + } + infobar_delegates_.clear(); + // Notify any observer that have a reference on this tab contents. NotificationService::current()->Notify(NOTIFY_TAB_CONTENTS_DESTROYED, Source<TabContents>(this), |