diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-06 03:01:48 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-06 03:01:48 +0000 |
commit | 39308cbb5bb0d795fbe0790d03e99b0d5932fb0e (patch) | |
tree | bd37514d76675cb4029d8de933f23d1eee5e5c3d /chrome/browser/notifications | |
parent | 0bfa772f76cf3f2bd2f9f570a8bf1595cde911b3 (diff) | |
download | chromium_src-39308cbb5bb0d795fbe0790d03e99b0d5932fb0e.zip chromium_src-39308cbb5bb0d795fbe0790d03e99b0d5932fb0e.tar.gz chromium_src-39308cbb5bb0d795fbe0790d03e99b0d5932fb0e.tar.bz2 |
Infobar system refactor.
This changes the ownership model of infobars so that InfoBars are long-lived and
own their InfoBarDelegates directly. The InfoBarService pseudo-owns the
InfoBars (instead of deleting them directly, it tells them when they're unowned
and expects them to delete themselves).
This fixes leaks when infobars are closed while not visible (e.g. in a
background tab) and in general makes the system clearer and easier to reason
about.
BUG=62154
TEST=none
TBR=sky
Review URL: https://codereview.chromium.org/103993003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239110 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/notifications')
-rw-r--r-- | chrome/browser/notifications/desktop_notification_service.cc | 18 | ||||
-rw-r--r-- | chrome/browser/notifications/notification_browsertest.cc | 13 |
2 files changed, 16 insertions, 15 deletions
diff --git a/chrome/browser/notifications/desktop_notification_service.cc b/chrome/browser/notifications/desktop_notification_service.cc index afbf55d..2aed905 100644 --- a/chrome/browser/notifications/desktop_notification_service.cc +++ b/chrome/browser/notifications/desktop_notification_service.cc @@ -17,6 +17,7 @@ #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/infobars/confirm_infobar_delegate.h" +#include "chrome/browser/infobars/infobar.h" #include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/notifications/desktop_notification_service_factory.h" #include "chrome/browser/notifications/notification.h" @@ -67,8 +68,8 @@ using blink::WebTextDirection; // permissions. class NotificationPermissionInfoBarDelegate : public ConfirmInfoBarDelegate { public: - // Creates a notification permission infobar delegate and adds it to - // |infobar_service|. + // Creates a notification permission infobar and delegate and adds the infobar + // to |infobar_service|. static void Create(InfoBarService* infobar_service, DesktopNotificationService* notification_service, const GURL& origin, @@ -79,7 +80,6 @@ class NotificationPermissionInfoBarDelegate : public ConfirmInfoBarDelegate { private: NotificationPermissionInfoBarDelegate( - InfoBarService* infobar_service, DesktopNotificationService* notification_service, const GURL& origin, const string16& display_name, @@ -127,21 +127,21 @@ void NotificationPermissionInfoBarDelegate::Create( int process_id, int route_id, int callback_context) { - infobar_service->AddInfoBar(scoped_ptr<InfoBarDelegate>( - new NotificationPermissionInfoBarDelegate( - infobar_service, notification_service, origin, display_name, - process_id, route_id, callback_context))); + infobar_service->AddInfoBar(ConfirmInfoBarDelegate::CreateInfoBar( + scoped_ptr<ConfirmInfoBarDelegate>( + new NotificationPermissionInfoBarDelegate( + notification_service, origin, display_name, process_id, route_id, + callback_context)))); } NotificationPermissionInfoBarDelegate::NotificationPermissionInfoBarDelegate( - InfoBarService* infobar_service, DesktopNotificationService* notification_service, const GURL& origin, const string16& display_name, int process_id, int route_id, int callback_context) - : ConfirmInfoBarDelegate(infobar_service), + : ConfirmInfoBarDelegate(), origin_(origin), display_name_(display_name), notification_service_(notification_service), diff --git a/chrome/browser/notifications/notification_browsertest.cc b/chrome/browser/notifications/notification_browsertest.cc index de06963..f475b4e 100644 --- a/chrome/browser/notifications/notification_browsertest.cc +++ b/chrome/browser/notifications/notification_browsertest.cc @@ -16,6 +16,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/infobars/confirm_infobar_delegate.h" +#include "chrome/browser/infobars/infobar.h" #include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/notifications/balloon.h" #include "chrome/browser/notifications/balloon_collection.h" @@ -320,7 +321,7 @@ void NotificationsTest::VerifyInfoBar(const Browser* browser, int index) { ASSERT_EQ(1U, infobar_service->infobar_count()); ConfirmInfoBarDelegate* confirm_infobar = - infobar_service->infobar_at(0)->AsConfirmInfoBarDelegate(); + infobar_service->infobar_at(0)->delegate()->AsConfirmInfoBarDelegate(); ASSERT_TRUE(confirm_infobar); int buttons = confirm_infobar->GetButtons(); EXPECT_TRUE(buttons & ConfirmInfoBarDelegate::BUTTON_OK); @@ -406,12 +407,12 @@ bool NotificationsTest::PerformActionOnInfoBar( return false; } - InfoBarDelegate* infobar_delegate = - infobar_service->infobar_at(infobar_index); + InfoBar* infobar = infobar_service->infobar_at(infobar_index); + InfoBarDelegate* infobar_delegate = infobar->delegate(); switch (action) { case DISMISS: infobar_delegate->InfoBarDismissed(); - infobar_service->RemoveInfoBar(infobar_delegate); + infobar_service->RemoveInfoBar(infobar); return true; case ALLOW: { @@ -420,7 +421,7 @@ bool NotificationsTest::PerformActionOnInfoBar( if (!confirm_infobar_delegate) { ADD_FAILURE(); } else if (confirm_infobar_delegate->Accept()) { - infobar_service->RemoveInfoBar(infobar_delegate); + infobar_service->RemoveInfoBar(infobar); return true; } } @@ -431,7 +432,7 @@ bool NotificationsTest::PerformActionOnInfoBar( if (!confirm_infobar_delegate) { ADD_FAILURE(); } else if (confirm_infobar_delegate->Cancel()) { - infobar_service->RemoveInfoBar(infobar_delegate); + infobar_service->RemoveInfoBar(infobar); return true; } } |