diff options
author | johnnyg@chromium.org <johnnyg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-11-12 00:24:12 +0000 |
---|---|---|
committer | johnnyg@chromium.org <johnnyg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-11-12 00:24:12 +0000 |
commit | 521b6efb91ea0c7cf11f4fc49c92fc32d8850595 (patch) | |
tree | af1c68798d74de08ec141b05ad65b1732dfae320 | |
parent | 16b59d5636b47694e846961165c5cd66e1f16976 (diff) | |
download | chromium_src-521b6efb91ea0c7cf11f4fc49c92fc32d8850595.zip chromium_src-521b6efb91ea0c7cf11f4fc49c92fc32d8850595.tar.gz chromium_src-521b6efb91ea0c7cf11f4fc49c92fc32d8850595.tar.bz2 |
When an extension is uninstalled, close all desktop notifications from that extension.
This change also refactors the balloon collection code to remove duplication between chrome and chromeos.
Removes some gross removal code which was using fake notifications just to get the right ID.
BUG=58266
TEST=open notifications from extension, uninstall extensions
Review URL: http://codereview.chromium.org/4635007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@65879 0039d316-1c4b-4281-b951-d872f2087c98
33 files changed, 355 insertions, 231 deletions
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc index 1b8b5b9..932baa1 100644 --- a/chrome/browser/automation/testing_automation_provider.cc +++ b/chrome/browser/automation/testing_automation_provider.cc @@ -4259,7 +4259,7 @@ void TestingAutomationProvider::CloseNotification( // This will delete itself when finished. new OnNotificationBalloonCountObserver( this, reply_message, collection, balloon_count - 1); - manager->Cancel(balloons[index]->notification()); + manager->CancelById(balloons[index]->notification().notification_id()); } // Refer to WaitForNotificationCount() in chrome/test/pyautolib/pyauto.py for diff --git a/chrome/browser/chromeos/notifications/balloon_collection_impl.cc b/chrome/browser/chromeos/notifications/balloon_collection_impl.cc index 56a3d1c..14da2d6 100644 --- a/chrome/browser/chromeos/notifications/balloon_collection_impl.cc +++ b/chrome/browser/chromeos/notifications/balloon_collection_impl.cc @@ -7,7 +7,6 @@ #include <algorithm> #include "base/logging.h" -#include "base/stl_util-inl.h" #include "chrome/browser/browser_list.h" #include "chrome/browser/chromeos/notifications/balloon_view.h" #include "chrome/browser/chromeos/notifications/notification_panel.h" @@ -24,17 +23,6 @@ namespace { const int kVerticalEdgeMargin = 5; const int kHorizontalEdgeMargin = 5; -class NotificationMatcher { - public: - explicit NotificationMatcher(const Notification& notification) - : notification_(notification) {} - bool operator()(const Balloon* b) const { - return notification_.IsSame(b->notification()); - } - private: - Notification notification_; -}; - } // namespace namespace chromeos { @@ -52,7 +40,7 @@ BalloonCollectionImpl::~BalloonCollectionImpl() { void BalloonCollectionImpl::Add(const Notification& notification, Profile* profile) { Balloon* new_balloon = MakeBalloon(notification, profile); - balloons_.push_back(new_balloon); + base_.Add(new_balloon); new_balloon->Show(); notification_ui_->Add(new_balloon); @@ -65,13 +53,13 @@ bool BalloonCollectionImpl::AddDOMUIMessageCallback( const Notification& notification, const std::string& message, MessageCallback* callback) { - Balloons::iterator iter = FindBalloon(notification); - if (iter == balloons_.end()) { + Balloon* balloon = FindBalloon(notification); + if (!balloon) { delete callback; return false; } BalloonViewHost* host = - static_cast<BalloonViewHost*>((*iter)->view()->GetHost()); + static_cast<BalloonViewHost*>(balloon->view()->GetHost()); return host->AddDOMUIMessageCallback(message, callback); } @@ -80,10 +68,11 @@ void BalloonCollectionImpl::AddSystemNotification( Profile* profile, bool sticky, bool control) { + Balloon* new_balloon = new Balloon(notification, profile, this); new_balloon->set_view( new chromeos::BalloonViewImpl(sticky, control, true)); - balloons_.push_back(new_balloon); + base_.Add(new_balloon); new_balloon->Show(); notification_ui_->Add(new_balloon); @@ -94,10 +83,9 @@ void BalloonCollectionImpl::AddSystemNotification( bool BalloonCollectionImpl::UpdateNotification( const Notification& notification) { - Balloons::iterator iter = FindBalloon(notification); - if (iter == balloons_.end()) + Balloon* balloon = FindBalloon(notification); + if (!balloon) return false; - Balloon* balloon = *iter; balloon->Update(notification); notification_ui_->Update(balloon); return true; @@ -105,10 +93,9 @@ bool BalloonCollectionImpl::UpdateNotification( bool BalloonCollectionImpl::UpdateAndShowNotification( const Notification& notification) { - Balloons::iterator iter = FindBalloon(notification); - if (iter == balloons_.end()) + Balloon* balloon = FindBalloon(notification); + if (!balloon) return false; - Balloon* balloon = *iter; balloon->Update(notification); bool updated = notification_ui_->Update(balloon); DCHECK(updated); @@ -116,15 +103,12 @@ bool BalloonCollectionImpl::UpdateAndShowNotification( return true; } -bool BalloonCollectionImpl::Remove(const Notification& notification) { - Balloons::iterator iter = FindBalloon(notification); - if (iter != balloons_.end()) { - // Balloon.CloseByScript() will cause OnBalloonClosed() to be called on - // this object, which will remove it from the collection and free it. - (*iter)->CloseByScript(); - return true; - } - return false; +bool BalloonCollectionImpl::RemoveById(const std::string& id) { + return base_.CloseById(id); +} + +bool BalloonCollectionImpl::RemoveBySourceOrigin(const GURL& origin) { + return base_.CloseAllBySourceOrigin(origin); } bool BalloonCollectionImpl::HasSpace() const { @@ -137,15 +121,9 @@ void BalloonCollectionImpl::ResizeBalloon(Balloon* balloon, } void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) { - // We want to free the balloon when finished. - scoped_ptr<Balloon> closed(source); - notification_ui_->Remove(source); + base_.Remove(source); - Balloons::iterator iter = FindBalloon(source->notification()); - if (iter != balloons_.end()) { - balloons_.erase(iter); - } // There may be no listener in a unit test. if (space_change_listener_) space_change_listener_->OnBalloonSpaceChanged(); @@ -170,7 +148,6 @@ void BalloonCollectionImpl::Shutdown() { // themselves from the parent. DVLOG(1) << "Shutting down notification UI"; notification_ui_.reset(); - STLDeleteElements(&balloons_); } Balloon* BalloonCollectionImpl::MakeBalloon(const Notification& notification, @@ -180,13 +157,6 @@ Balloon* BalloonCollectionImpl::MakeBalloon(const Notification& notification, return new_balloon; } -std::deque<Balloon*>::iterator BalloonCollectionImpl::FindBalloon( - const Notification& notification) { - return std::find_if(balloons_.begin(), - balloons_.end(), - NotificationMatcher(notification)); -} - } // namespace chromeos // static diff --git a/chrome/browser/chromeos/notifications/balloon_collection_impl.h b/chrome/browser/chromeos/notifications/balloon_collection_impl.h index 79cfe14..c9c74b5 100644 --- a/chrome/browser/chromeos/notifications/balloon_collection_impl.h +++ b/chrome/browser/chromeos/notifications/balloon_collection_impl.h @@ -12,6 +12,7 @@ #include "base/scoped_ptr.h" #include "chrome/browser/chromeos/notifications/balloon_view_host.h" #include "chrome/browser/notifications/balloon_collection.h" +#include "chrome/browser/notifications/balloon_collection_base.h" #include "chrome/common/notification_registrar.h" #include "gfx/point.h" #include "gfx/rect.h" @@ -60,12 +61,13 @@ class BalloonCollectionImpl : public BalloonCollection, // BalloonCollectionInterface overrides virtual void Add(const Notification& notification, Profile* profile); - virtual bool Remove(const Notification& notification); + virtual bool RemoveById(const std::string& id); + virtual bool RemoveBySourceOrigin(const GURL& origin); virtual bool HasSpace() const; virtual void ResizeBalloon(Balloon* balloon, const gfx::Size& size); virtual void DisplayChanged() {} virtual void OnBalloonClosed(Balloon* source); - virtual const Balloons& GetActiveBalloons() { return balloons_; } + virtual const Balloons& GetActiveBalloons() { return base_.balloons(); } // NotificationObserver overrides: virtual void Observe(NotificationType type, @@ -115,19 +117,18 @@ class BalloonCollectionImpl : public BalloonCollection, virtual Balloon* MakeBalloon(const Notification& notification, Profile* profile); + // Base implementation for the collection of active balloons. + BalloonCollectionBase base_; + private: friend class NotificationPanelTester; // Shutdown the notification ui. void Shutdown(); - // The number of balloons being displayed. - int count() const { return balloons_.size(); } - - Balloons::iterator FindBalloon(const Notification& notification); - - // Queue of active balloons. - Balloons balloons_; + Balloon* FindBalloon(const Notification& notification) { + return base_.FindBalloon(notification); + } scoped_ptr<NotificationUI> notification_ui_; diff --git a/chrome/browser/chromeos/notifications/balloon_view.cc b/chrome/browser/chromeos/notifications/balloon_view.cc index 24a6004..ff5ea0f 100644 --- a/chrome/browser/chromeos/notifications/balloon_view.cc +++ b/chrome/browser/chromeos/notifications/balloon_view.cc @@ -15,6 +15,7 @@ #include "chrome/browser/chromeos/notifications/notification_panel.h" #include "chrome/browser/notifications/balloon.h" #include "chrome/browser/notifications/desktop_notification_service.h" +#include "chrome/browser/notifications/notification.h" #include "chrome/browser/profile.h" #include "chrome/browser/renderer_host/render_view_host.h" #include "chrome/browser/renderer_host/render_widget_host_view.h" @@ -300,7 +301,8 @@ void BalloonViewImpl::Observe(NotificationType type, // BalloonViewImpl public. bool BalloonViewImpl::IsFor(const Notification& notification) const { - return balloon_->notification().IsSame(notification); + return balloon_->notification().notification_id() == + notification.notification_id(); } void BalloonViewImpl::Activated() { diff --git a/chrome/browser/chromeos/notifications/desktop_notifications_unittest.cc b/chrome/browser/chromeos/notifications/desktop_notifications_unittest.cc index 8c0a4be..377828b 100644 --- a/chrome/browser/chromeos/notifications/desktop_notifications_unittest.cc +++ b/chrome/browser/chromeos/notifications/desktop_notifications_unittest.cc @@ -24,33 +24,24 @@ class MockNotificationUI : public BalloonCollectionImpl::NotificationUI { virtual void SetActiveView(BalloonViewImpl* view) {} }; -MockBalloonCollection::MockBalloonCollection() - : log_proxy_(new LoggingNotificationProxy()) { +MockBalloonCollection::MockBalloonCollection() { set_notification_ui(new MockNotificationUI()); } void MockBalloonCollection::Add(const Notification& notification, Profile* profile) { - // Swap in the logging proxy for the purpose of logging calls that + // Swap in a logging proxy for the purpose of logging calls that // would be made into javascript, then pass this down to the // balloon collection. - Notification test_notification(notification.origin_url(), - notification.content_url(), - notification.display_source(), - string16(), /* replace_id */ - log_proxy_.get()); + Notification test_notification( + notification.origin_url(), + notification.content_url(), + notification.display_source(), + notification.replace_id(), + new LoggingNotificationProxy(notification.notification_id())); BalloonCollectionImpl::Add(test_notification, profile); } -bool MockBalloonCollection::Remove(const Notification& notification) { - Notification test_notification(notification.origin_url(), - notification.content_url(), - notification.display_source(), - string16(), /* replace_id */ - log_proxy_.get()); - return BalloonCollectionImpl::Remove(test_notification); -} - Balloon* MockBalloonCollection::MakeBalloon(const Notification& notification, Profile* profile) { // Start with a normal balloon but mock out the view. diff --git a/chrome/browser/chromeos/notifications/desktop_notifications_unittest.h b/chrome/browser/chromeos/notifications/desktop_notifications_unittest.h index 8e0d7ae..5471b3b 100644 --- a/chrome/browser/chromeos/notifications/desktop_notifications_unittest.h +++ b/chrome/browser/chromeos/notifications/desktop_notifications_unittest.h @@ -17,7 +17,6 @@ #include "chrome/browser/notifications/balloon.h" #include "chrome/browser/notifications/desktop_notification_service.h" #include "chrome/browser/notifications/notification.h" -#include "chrome/browser/notifications/notification_object_proxy.h" #include "chrome/browser/notifications/notification_test_util.h" #include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/browser/notifications/notifications_prefs_cache.h" @@ -28,7 +27,7 @@ namespace chromeos { class DesktopNotificationsTest; -typedef LoggingNotificationProxyBase<DesktopNotificationsTest> +typedef LoggingNotificationDelegate<DesktopNotificationsTest> LoggingNotificationProxy; // Test version of the balloon collection which counts the number @@ -40,7 +39,6 @@ class MockBalloonCollection : public BalloonCollectionImpl { // BalloonCollectionImpl overrides virtual void Add(const Notification& notification, Profile* profile); - virtual bool Remove(const Notification& notification); virtual Balloon* MakeBalloon(const Notification& notification, Profile* profile); virtual void OnBalloonClosed(Balloon* source); @@ -54,7 +52,6 @@ class MockBalloonCollection : public BalloonCollectionImpl { private: std::set<Balloon*> balloons_; - scoped_refptr<LoggingNotificationProxy> log_proxy_; }; class DesktopNotificationsTest : public testing::Test { diff --git a/chrome/browser/chromeos/notifications/notification_browsertest.cc b/chrome/browser/chromeos/notifications/notification_browsertest.cc index 5901454..3a8758f 100644 --- a/chrome/browser/chromeos/notifications/notification_browsertest.cc +++ b/chrome/browser/chromeos/notifications/notification_browsertest.cc @@ -16,7 +16,7 @@ #include "chrome/browser/chromeos/notifications/balloon_view.h" #include "chrome/browser/chromeos/notifications/notification_panel.h" #include "chrome/browser/chromeos/notifications/system_notification_factory.h" -#include "chrome/browser/notifications/notification_delegate.h" +#include "chrome/browser/notifications/notification_test_util.h" #include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/common/notification_service.h" #include "chrome/test/in_process_browser_test.h" @@ -24,22 +24,6 @@ namespace { -class MockNotificationDelegate : public NotificationDelegate { - public: - explicit MockNotificationDelegate(const std::string& id) : id_(id) {} - - virtual void Display() {} - virtual void Error() {} - virtual void Close(bool by_user) {} - virtual void Click() {} - virtual std::string id() const { return id_; } - - private: - std::string id_; - - DISALLOW_COPY_AND_ASSIGN(MockNotificationDelegate); -}; - // The name of ChromeOS's window manager. const char* kChromeOsWindowManagerName = "chromeos-wm"; @@ -162,14 +146,14 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, TestBasic) { EXPECT_EQ(2, tester->GetNotificationCount()); EXPECT_EQ(NotificationPanel::STICKY_AND_NEW, tester->state()); - collection->Remove(NewMockNotification("1")); + collection->RemoveById("1"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(1, tester->GetNewNotificationCount()); EXPECT_EQ(1, tester->GetNotificationCount()); EXPECT_EQ(NotificationPanel::STICKY_AND_NEW, tester->state()); - collection->Remove(NewMockNotification("2")); + collection->RemoveById("2"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(0, tester->GetNewNotificationCount()); EXPECT_EQ(0, tester->GetNotificationCount()); @@ -198,7 +182,7 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, TestKeepSizeState) { panel->OnMouseMotion(gfx::Point(10, 10)); EXPECT_EQ(NotificationPanel::KEEP_SIZE, tester->state()); - collection->Remove(NewMockNotification("1")); + collection->RemoveById("1"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(1, tester->GetNewNotificationCount()); EXPECT_EQ(1, tester->GetNotificationCount()); @@ -210,20 +194,20 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, TestKeepSizeState) { EXPECT_EQ(2, tester->GetNotificationCount()); EXPECT_EQ(NotificationPanel::KEEP_SIZE, tester->state()); - collection->Remove(NewMockNotification("1")); + collection->RemoveById("1"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(1, tester->GetNewNotificationCount()); EXPECT_EQ(1, tester->GetNotificationCount()); EXPECT_EQ(NotificationPanel::KEEP_SIZE, tester->state()); - collection->Remove(NewMockNotification("2")); + collection->RemoveById("2"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(0, tester->GetNotificationCount()); EXPECT_EQ(NotificationPanel::CLOSED, tester->state()); collection->Add(NewMockNotification("3"), browser()->profile()); EXPECT_EQ(NotificationPanel::STICKY_AND_NEW, tester->state()); - collection->Remove(NewMockNotification("3")); + collection->RemoveById("3"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(0, tester->GetNotificationCount()); @@ -257,10 +241,7 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, TestSystemNotification) { EXPECT_EQ(1, tester->GetStickyNotificationCount()); // Dismiss the notification. - // TODO(oshima): Consider updating API to Remove(NotificationDelegate) - // or Remove(std::string id); - collection->Remove(Notification(GURL(), GURL(), string16(), string16(), - delegate.get())); + collection->RemoveById(delegate->id()); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(0, tester->GetStickyNotificationCount()); @@ -287,11 +268,11 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, TestStateTransition1) { ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(NotificationPanel::MINIMIZED, tester->state()); - collection->Remove(NewMockNotification("2")); + collection->RemoveById("2"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(NotificationPanel::MINIMIZED, tester->state()); - collection->Remove(NewMockNotification("1")); + collection->RemoveById("1"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(0, tester->GetNotificationCount()); EXPECT_EQ(NotificationPanel::CLOSED, tester->state()); @@ -357,20 +338,19 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, FLAKY_TestStateTransition2) { ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(NotificationPanel::STICKY_AND_NEW, tester->state()); - collection->Remove(NewMockNotification("1")); + collection->RemoveById("1"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(NotificationPanel::STICKY_AND_NEW, tester->state()); // Removing the system notification should minimize the panel. - collection->Remove(NewMockNotification("3")); + collection->RemoveById("3"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(1, tester->GetNotificationCount()); EXPECT_EQ(NotificationPanel::MINIMIZED, tester->state()); WaitForPanelState(tester, PanelController::MINIMIZED); // Removing the last notification. Should close the panel. - - collection->Remove(NewMockNotification("2")); + collection->RemoveById("2"); ui_test_utils::RunAllPendingInMessageLoop(); EXPECT_EQ(0, tester->GetNotificationCount()); EXPECT_EQ(NotificationPanel::CLOSED, tester->state()); diff --git a/chrome/browser/chromeos/notifications/notification_panel.cc b/chrome/browser/chromeos/notifications/notification_panel.cc index 1ff6014..6ab3b31 100644 --- a/chrome/browser/chromeos/notifications/notification_panel.cc +++ b/chrome/browser/chromeos/notifications/notification_panel.cc @@ -848,10 +848,8 @@ PanelController* NotificationPanelTester::GetPanelController() const { BalloonViewImpl* NotificationPanelTester::GetBalloonView( BalloonCollectionImpl* collection, const Notification& notification) { - BalloonCollectionImpl::Balloons::iterator iter = - collection->FindBalloon(notification); - DCHECK(iter != collection->balloons_.end()); - Balloon* balloon = (*iter); + Balloon* balloon = collection->FindBalloon(notification); + DCHECK(balloon); return GetBalloonViewOf(balloon); } diff --git a/chrome/browser/chromeos/notifications/system_notification.cc b/chrome/browser/chromeos/notifications/system_notification.cc index f83c414..0d85687 100644 --- a/chrome/browser/chromeos/notifications/system_notification.cc +++ b/chrome/browser/chromeos/notifications/system_notification.cc @@ -65,9 +65,7 @@ void SystemNotification::Show(const string16& message, void SystemNotification::Hide() { if (visible_) { - collection_->Remove(Notification(GURL(), GURL(), string16(), string16(), - delegate_.get())); - + collection_->RemoveById(delegate_->id()); visible_ = false; urgent_ = false; } diff --git a/chrome/browser/cocoa/notifications/balloon_controller.mm b/chrome/browser/cocoa/notifications/balloon_controller.mm index 3f8a8b0..048888c 100644 --- a/chrome/browser/cocoa/notifications/balloon_controller.mm +++ b/chrome/browser/cocoa/notifications/balloon_controller.mm @@ -17,6 +17,7 @@ #include "chrome/browser/cocoa/notifications/balloon_view_host_mac.h" #include "chrome/browser/notifications/balloon.h" #include "chrome/browser/notifications/desktop_notification_service.h" +#include "chrome/browser/notifications/notification.h" #include "chrome/browser/notifications/notification_options_menu_model.h" #include "chrome/browser/profile.h" #include "chrome/browser/renderer_host/render_view_host.h" diff --git a/chrome/browser/cocoa/notifications/balloon_controller_unittest.mm b/chrome/browser/cocoa/notifications/balloon_controller_unittest.mm index 6f6a1ab..31f7c15 100644 --- a/chrome/browser/cocoa/notifications/balloon_controller_unittest.mm +++ b/chrome/browser/cocoa/notifications/balloon_controller_unittest.mm @@ -30,7 +30,8 @@ namespace { class MockBalloonCollection : public BalloonCollection { virtual void Add(const Notification& notification, Profile* profile) {} - virtual bool Remove(const Notification& notification) { return false; } + virtual bool RemoveById(const std::string& id) { return false; } + virtual bool RemoveBySourceOrigin(const GURL& origin) { return false; } virtual bool HasSpace() const { return true; } virtual void ResizeBalloon(Balloon* balloon, const gfx::Size& size) {}; virtual void DisplayChanged() {} diff --git a/chrome/browser/gtk/notifications/balloon_view_gtk.cc b/chrome/browser/gtk/notifications/balloon_view_gtk.cc index 795b4ea..9161be5 100644 --- a/chrome/browser/gtk/notifications/balloon_view_gtk.cc +++ b/chrome/browser/gtk/notifications/balloon_view_gtk.cc @@ -27,6 +27,7 @@ #include "chrome/browser/gtk/rounded_window.h" #include "chrome/browser/notifications/balloon.h" #include "chrome/browser/notifications/desktop_notification_service.h" +#include "chrome/browser/notifications/notification.h" #include "chrome/browser/notifications/notification_options_menu_model.h" #include "chrome/browser/profile.h" #include "chrome/browser/renderer_host/render_view_host.h" diff --git a/chrome/browser/notifications/balloon_collection.cc b/chrome/browser/notifications/balloon_collection.cc index a125837..7d926ed 100644 --- a/chrome/browser/notifications/balloon_collection.cc +++ b/chrome/browser/notifications/balloon_collection.cc @@ -45,7 +45,6 @@ BalloonCollectionImpl::BalloonCollectionImpl() } BalloonCollectionImpl::~BalloonCollectionImpl() { - STLDeleteElements(&balloons_); } void BalloonCollectionImpl::Add(const Notification& notification, @@ -59,10 +58,11 @@ void BalloonCollectionImpl::Add(const Notification& notification, new_balloon->SetPosition(layout_.OffScreenLocation(), false); new_balloon->Show(); #if USE_OFFSETS - if (balloons_.size() > 0) - new_balloon->set_offset(balloons_[balloons_.size() - 1]->offset()); + int count = base_.count(); + if (count > 0) + new_balloon->set_offset(base_.balloons()[count - 1]->offset()); #endif - balloons_.push_back(new_balloon); + base_.Add(new_balloon); PositionBalloons(false); // There may be no listener in a unit test. @@ -74,28 +74,24 @@ void BalloonCollectionImpl::Add(const Notification& notification, on_collection_changed_callback_->Run(); } -bool BalloonCollectionImpl::Remove(const Notification& notification) { - Balloons::iterator iter; - for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) { - if (notification.IsSame((*iter)->notification())) { - // Balloon.CloseByScript() will cause OnBalloonClosed() to be called on - // this object, which will remove it from the collection and free it. - (*iter)->CloseByScript(); - return true; - } - } - return false; +bool BalloonCollectionImpl::RemoveById(const std::string& id) { + return base_.CloseById(id); +} + +bool BalloonCollectionImpl::RemoveBySourceOrigin(const GURL& origin) { + return base_.CloseAllBySourceOrigin(origin); } bool BalloonCollectionImpl::HasSpace() const { - if (count() < kMinAllowedBalloonCount) + int count = base_.count(); + if (count < kMinAllowedBalloonCount) return true; int max_balloon_size = 0; int total_size = 0; layout_.GetMaxLinearSize(&max_balloon_size, &total_size); - int current_max_size = max_balloon_size * count(); + int current_max_size = max_balloon_size * count; int max_allowed_size = static_cast<int>(total_size * kPercentBalloonFillFactor); return current_max_size < max_allowed_size - max_balloon_size; @@ -114,16 +110,16 @@ void BalloonCollectionImpl::DisplayChanged() { void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) { // We want to free the balloon when finished. - scoped_ptr<Balloon> closed(source); - Balloons::iterator it = balloons_.begin(); + const Balloons& balloons = base_.balloons(); + Balloons::const_iterator it = balloons.begin(); #if USE_OFFSETS gfx::Point offset; bool apply_offset = false; - while (it != balloons_.end()) { + while (it != balloons.end()) { if (*it == source) { - it = balloons_.erase(it); - if (it != balloons_.end()) { + ++it; + if (it != balloons.end()) { apply_offset = true; offset.set_y((source)->offset().y() - (*it)->offset().y() + (*it)->content_size().height() - source->content_size().height()); @@ -138,15 +134,9 @@ void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) { // leaves the balloon area. if (apply_offset) AddMessageLoopObserver(); -#else - for (; it != balloons_.end(); ++it) { - if (*it == source) { - balloons_.erase(it); - break; - } - } #endif + base_.Remove(source); PositionBalloons(true); // There may be no listener in a unit test. @@ -159,9 +149,13 @@ void BalloonCollectionImpl::OnBalloonClosed(Balloon* source) { } void BalloonCollectionImpl::PositionBalloonsInternal(bool reposition) { + const Balloons& balloons = base_.balloons(); + layout_.RefreshSystemMetrics(); gfx::Point origin = layout_.GetLayoutOrigin(); - for (Balloons::iterator it = balloons_.begin(); it != balloons_.end(); ++it) { + for (Balloons::const_iterator it = balloons.begin(); + it != balloons.end(); + ++it) { gfx::Point upper_left = layout_.NextPosition((*it)->GetViewSize(), &origin); (*it)->SetPosition(upper_left, reposition); } @@ -188,7 +182,10 @@ void BalloonCollectionImpl::CancelOffsets() { // Unhook from listening to all UI events. RemoveMessageLoopObserver(); - for (Balloons::iterator it = balloons_.begin(); it != balloons_.end(); ++it) + const Balloons& balloons = base_.balloons(); + for (Balloons::const_iterator it = balloons.begin(); + it != balloons.end(); + ++it) (*it)->set_offset(gfx::Point(0, 0)); PositionBalloons(true); diff --git a/chrome/browser/notifications/balloon_collection.h b/chrome/browser/notifications/balloon_collection.h index 499b937..2d75686 100644 --- a/chrome/browser/notifications/balloon_collection.h +++ b/chrome/browser/notifications/balloon_collection.h @@ -9,11 +9,13 @@ #pragma once #include <deque> +#include <string> #include "base/callback.h" #include "base/scoped_ptr.h" class Balloon; +class GURL; class Notification; class Profile; @@ -44,9 +46,13 @@ class BalloonCollection { virtual void Add(const Notification& notification, Profile* profile) = 0; - // Removes a balloon from the collection if present. Returns + // Removes any balloons that have this notification id. Returns // true if anything was removed. - virtual bool Remove(const Notification& notification) = 0; + virtual bool RemoveById(const std::string& id) = 0; + + // Removes any balloons that have this source origin. Returns + // true if anything was removed. + virtual bool RemoveBySourceOrigin(const GURL& source_origin) = 0; // Is there room to add another notification? virtual bool HasSpace() const = 0; diff --git a/chrome/browser/notifications/balloon_collection_base.cc b/chrome/browser/notifications/balloon_collection_base.cc new file mode 100644 index 0000000..94765c0 --- /dev/null +++ b/chrome/browser/notifications/balloon_collection_base.cc @@ -0,0 +1,76 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/notifications/balloon_collection_base.h" + +#include "base/stl_util-inl.h" +#include "chrome/browser/notifications/balloon.h" +#include "chrome/browser/notifications/notification.h" +#include "googleurl/src/gurl.h" + +BalloonCollectionBase::BalloonCollectionBase() { +} + +BalloonCollectionBase::~BalloonCollectionBase() { + STLDeleteElements(&balloons_); +} + +void BalloonCollectionBase::Add(Balloon* balloon) { + balloons_.push_back(balloon); +} + +void BalloonCollectionBase::Remove(Balloon* balloon) { + // Free after removing. + scoped_ptr<Balloon> to_delete(balloon); + Balloons::iterator iter; + for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) { + if ((*iter) == balloon) { + balloons_.erase(iter); + return; + } + } +} + +bool BalloonCollectionBase::CloseById(const std::string& id) { + // Use a local list of balloons to close to avoid breaking + // iterator changes on each close. + Balloons to_close; + Balloons::iterator iter; + for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) { + if ((*iter)->notification().notification_id() == id) + to_close.push_back(*iter); + } + for (iter = to_close.begin(); iter != to_close.end(); ++iter) + (*iter)->CloseByScript(); + + return !to_close.empty(); +} + +bool BalloonCollectionBase::CloseAllBySourceOrigin( + const GURL& source_origin) { + // Use a local list of balloons to close to avoid breaking + // iterator changes on each close. + Balloons to_close; + Balloons::iterator iter; + for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) { + if ((*iter)->notification().origin_url() == source_origin) + to_close.push_back(*iter); + } + for (iter = to_close.begin(); iter != to_close.end(); ++iter) + (*iter)->CloseByScript(); + + return !to_close.empty(); +} + +Balloon* BalloonCollectionBase::FindBalloon( + const Notification& notification) { + Balloons::iterator iter; + for (iter = balloons_.begin(); iter != balloons_.end(); ++iter) { + if ((*iter)->notification().notification_id() == + notification.notification_id()) { + return *iter; + } + } + return NULL; +} diff --git a/chrome/browser/notifications/balloon_collection_base.h b/chrome/browser/notifications/balloon_collection_base.h new file mode 100644 index 0000000..8511030 --- /dev/null +++ b/chrome/browser/notifications/balloon_collection_base.h @@ -0,0 +1,62 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Handles the visible notification (or balloons). + +#ifndef CHROME_BROWSER_NOTIFICATIONS_BALLOON_COLLECTION_BASE_H_ +#define CHROME_BROWSER_NOTIFICATIONS_BALLOON_COLLECTION_BASE_H_ +#pragma once + +#include <deque> +#include <string> + +#include "base/basictypes.h" + +class Balloon; +class GURL; +class Notification; + +// This class provides support for implementing a BalloonCollection +// including the parts common between Chrome UI and ChromeOS UI. +class BalloonCollectionBase { + public: + BalloonCollectionBase(); + virtual ~BalloonCollectionBase(); + + typedef std::deque<Balloon*> Balloons; + + // Adds a balloon to the collection. Takes ownership of pointer. + virtual void Add(Balloon* balloon); + + // Removes a balloon from the collection (if present). Frees + // the pointer after removal. + virtual void Remove(Balloon* balloon); + + // Finds any balloon matching the given notification id, and + // calls CloseByScript on it. Returns true if anything was + // found. + virtual bool CloseById(const std::string& id); + + // Finds all balloons matching the given notification source, + // and calls CloseByScript on them. Returns true if anything + // was found. + virtual bool CloseAllBySourceOrigin(const GURL& source_origin); + + const Balloons& balloons() const { return balloons_; } + + // Returns the balloon matching the given notification, or + // NULL if there is no matching balloon. + Balloon* FindBalloon(const Notification& notification); + + // The number of balloons being displayed. + int count() const { return static_cast<int>(balloons_.size()); } + + private: + // Queue of active balloons. Pointers are owned by this class. + Balloons balloons_; + + DISALLOW_COPY_AND_ASSIGN(BalloonCollectionBase); +}; + +#endif // CHROME_BROWSER_NOTIFICATIONS_BALLOON_COLLECTION_BASE_H_ diff --git a/chrome/browser/notifications/balloon_collection_impl.h b/chrome/browser/notifications/balloon_collection_impl.h index a9c7afc..3596a25 100644 --- a/chrome/browser/notifications/balloon_collection_impl.h +++ b/chrome/browser/notifications/balloon_collection_impl.h @@ -13,6 +13,7 @@ #include "base/basictypes.h" #include "base/message_loop.h" #include "chrome/browser/notifications/balloon_collection.h" +#include "chrome/browser/notifications/balloon_collection_base.h" #include "gfx/point.h" #include "gfx/rect.h" @@ -41,14 +42,13 @@ class BalloonCollectionImpl : public BalloonCollection // BalloonCollection interface. virtual void Add(const Notification& notification, Profile* profile); - virtual bool Remove(const Notification& notification); + virtual bool RemoveById(const std::string& id); + virtual bool RemoveBySourceOrigin(const GURL& source_origin); virtual bool HasSpace() const; virtual void ResizeBalloon(Balloon* balloon, const gfx::Size& size); virtual void DisplayChanged(); virtual void OnBalloonClosed(Balloon* source); - virtual const Balloons& GetActiveBalloons() { - return balloons_; - } + virtual const Balloons& GetActiveBalloons() { return base_.balloons(); } // MessageLoopForUI::Observer interface. #if defined(OS_WIN) @@ -135,9 +135,6 @@ class BalloonCollectionImpl : public BalloonCollection Profile* profile); private: - // The number of balloons being displayed. - int count() const { return balloons_.size(); } - // Adjusts the positions of the balloons (e.g., when one is closed). // Implemented by each platform for specific UI requirements. void PositionBalloons(bool is_reposition); @@ -150,6 +147,12 @@ class BalloonCollectionImpl : public BalloonCollection static gfx::Rect GetMacWorkArea(); #endif + // Base implementation for the collection of active balloons. + BalloonCollectionBase base_; + + // The layout parameters for balloons in this collection. + Layout layout_; + #if USE_OFFSETS // Start and stop observing all UI events. void AddMessageLoopObserver(); @@ -163,16 +166,7 @@ class BalloonCollectionImpl : public BalloonCollection // Is the current cursor in the balloon area? bool IsCursorInBalloonCollection() const; -#endif - // Queue of active balloons. - typedef std::deque<Balloon*> Balloons; - Balloons balloons_; - - // The layout parameters for balloons in this collection. - Layout layout_; - -#if USE_OFFSETS // Factory for generating delayed reposition tasks on mouse motion. ScopedRunnableMethodFactory<BalloonCollectionImpl> reposition_factory_; diff --git a/chrome/browser/notifications/balloon_collection_linux.cc b/chrome/browser/notifications/balloon_collection_linux.cc index f15c713..08354a0 100644 --- a/chrome/browser/notifications/balloon_collection_linux.cc +++ b/chrome/browser/notifications/balloon_collection_linux.cc @@ -46,10 +46,11 @@ void BalloonCollectionImpl::DidProcessEvent(GdkEvent* event) { } bool BalloonCollectionImpl::IsCursorInBalloonCollection() const { - if (balloons_.empty()) + const Balloons& balloons = base_.balloons(); + if (balloons.empty()) return false; - gfx::Point upper_left = balloons_[balloons_.size() - 1]->GetPosition(); + gfx::Point upper_left = balloons[balloons.size() - 1]->GetPosition(); gfx::Point lower_right = layout_.GetLayoutOrigin(); gfx::Rect bounds = gfx::Rect(upper_left.x(), diff --git a/chrome/browser/notifications/balloon_collection_win.cc b/chrome/browser/notifications/balloon_collection_win.cc index 8915662..07bcd18 100644 --- a/chrome/browser/notifications/balloon_collection_win.cc +++ b/chrome/browser/notifications/balloon_collection_win.cc @@ -44,10 +44,11 @@ void BalloonCollectionImpl::DidProcessMessage(const MSG& msg) { } bool BalloonCollectionImpl::IsCursorInBalloonCollection() const { - if (balloons_.empty()) + const Balloons& balloons = base_.balloons(); + if (balloons.empty()) return false; - gfx::Point upper_left = balloons_[balloons_.size() - 1]->GetPosition(); + gfx::Point upper_left = balloons[balloons.size() - 1]->GetPosition(); gfx::Point lower_right = layout_.GetLayoutOrigin(); gfx::Rect bounds = gfx::Rect(upper_left.x(), diff --git a/chrome/browser/notifications/desktop_notification_service.cc b/chrome/browser/notifications/desktop_notification_service.cc index e7530e4..539462f 100644 --- a/chrome/browser/notifications/desktop_notification_service.cc +++ b/chrome/browser/notifications/desktop_notification_service.cc @@ -215,7 +215,7 @@ DesktopNotificationService::DesktopNotificationService(Profile* profile, NotificationUIManager* ui_manager) : profile_(profile), ui_manager_(ui_manager) { - registrar_.Init(profile_->GetPrefs()); + prefs_registrar_.Init(profile_->GetPrefs()); InitPrefs(); StartObserving(); } @@ -260,15 +260,20 @@ void DesktopNotificationService::InitPrefs() { void DesktopNotificationService::StartObserving() { if (!profile_->IsOffTheRecord()) { - registrar_.Add(prefs::kDesktopNotificationDefaultContentSetting, this); - registrar_.Add(prefs::kDesktopNotificationAllowedOrigins, this); - registrar_.Add(prefs::kDesktopNotificationDeniedOrigins, this); + prefs_registrar_.Add(prefs::kDesktopNotificationDefaultContentSetting, + this); + prefs_registrar_.Add(prefs::kDesktopNotificationAllowedOrigins, this); + prefs_registrar_.Add(prefs::kDesktopNotificationDeniedOrigins, this); + + notification_registrar_.Add(this, NotificationType::EXTENSION_UNLOADED, + NotificationService::AllSources()); } } void DesktopNotificationService::StopObserving() { if (!profile_->IsOffTheRecord()) { - registrar_.RemoveAll(); + prefs_registrar_.RemoveAll(); + notification_registrar_.RemoveAll(); } } @@ -299,11 +304,22 @@ void DesktopNotificationService::DenyPermission(const GURL& origin) { void DesktopNotificationService::Observe(NotificationType type, const NotificationSource& source, const NotificationDetails& details) { - DCHECK(NotificationType::PREF_CHANGED == type); + if (NotificationType::PREF_CHANGED == type) { + const std::string& name = *Details<std::string>(details).ptr(); + OnPrefsChanged(name); + } else if (NotificationType::EXTENSION_UNLOADED == type) { + // Remove all notifications currently shown or queued by the extension + // which was unloaded. + Extension* extension = Details<Extension>(details).ptr(); + if (extension) + ui_manager_->CancelAllBySourceOrigin(extension->url()); + } +} + +void DesktopNotificationService::OnPrefsChanged(const std::string& pref_name) { PrefService* prefs = profile_->GetPrefs(); - const std::string& name = *Details<std::string>(details).ptr(); - if (name == prefs::kDesktopNotificationAllowedOrigins) { + if (pref_name == prefs::kDesktopNotificationAllowedOrigins) { NotificationService::current()->Notify( NotificationType::DESKTOP_NOTIFICATION_SETTINGS_CHANGED, Source<DesktopNotificationService>(this), @@ -317,7 +333,7 @@ void DesktopNotificationService::Observe(NotificationType type, prefs_cache_.get(), &NotificationsPrefsCache::SetCacheAllowedOrigins, allowed_origins)); - } else if (name == prefs::kDesktopNotificationDeniedOrigins) { + } else if (pref_name == prefs::kDesktopNotificationDeniedOrigins) { NotificationService::current()->Notify( NotificationType::DESKTOP_NOTIFICATION_SETTINGS_CHANGED, Source<DesktopNotificationService>(this), @@ -331,7 +347,7 @@ void DesktopNotificationService::Observe(NotificationType type, prefs_cache_.get(), &NotificationsPrefsCache::SetCacheDeniedOrigins, denied_origins)); - } else if (name == prefs::kDesktopNotificationDefaultContentSetting) { + } else if (pref_name == prefs::kDesktopNotificationDefaultContentSetting) { NotificationService::current()->Notify( NotificationType::DESKTOP_NOTIFICATION_DEFAULT_CHANGED, Source<DesktopNotificationService>(this), @@ -563,9 +579,7 @@ bool DesktopNotificationService::CancelDesktopNotification( scoped_refptr<NotificationObjectProxy> proxy( new NotificationObjectProxy(process_id, route_id, notification_id, false)); - // TODO(johnnyg): clean up this "empty" notification. - Notification notif(GURL(), GURL(), string16(), string16(), proxy); - return ui_manager_->Cancel(notif); + return ui_manager_->CancelById(proxy->id()); } diff --git a/chrome/browser/notifications/desktop_notification_service.h b/chrome/browser/notifications/desktop_notification_service.h index d92d7e6..53fa56f 100644 --- a/chrome/browser/notifications/desktop_notification_service.h +++ b/chrome/browser/notifications/desktop_notification_service.h @@ -6,24 +6,24 @@ #define CHROME_BROWSER_NOTIFICATIONS_DESKTOP_NOTIFICATION_SERVICE_H_ #pragma once +#include <string> #include <vector> #include "base/basictypes.h" +#include "base/ref_counted.h" #include "base/string16.h" -#include "chrome/browser/notifications/notification.h" #include "chrome/browser/prefs/pref_change_registrar.h" #include "chrome/common/content_settings.h" #include "chrome/common/notification_observer.h" #include "chrome/common/notification_registrar.h" -#include "chrome/common/notification_service.h" #include "googleurl/src/gurl.h" #include "third_party/WebKit/WebKit/chromium/public/WebTextDirection.h" +class Notification; class NotificationUIManager; class NotificationsPrefsCache; class PrefService; class Profile; -class Task; class TabContents; struct ViewHostMsg_ShowNotification_Params; @@ -121,6 +121,8 @@ class DesktopNotificationService : public NotificationObserver { void StartObserving(); void StopObserving(); + void OnPrefsChanged(const std::string& pref_name); + // Takes a notification object and shows it in the UI. void ShowNotification(const Notification& notification); @@ -143,7 +145,8 @@ class DesktopNotificationService : public NotificationObserver { // UI for desktop toasts. NotificationUIManager* ui_manager_; - PrefChangeRegistrar registrar_; + PrefChangeRegistrar prefs_registrar_; + NotificationRegistrar notification_registrar_; DISALLOW_COPY_AND_ASSIGN(DesktopNotificationService); }; diff --git a/chrome/browser/notifications/desktop_notifications_unittest.cc b/chrome/browser/notifications/desktop_notifications_unittest.cc index 99466901..ebb6ab8 100644 --- a/chrome/browser/notifications/desktop_notifications_unittest.cc +++ b/chrome/browser/notifications/desktop_notifications_unittest.cc @@ -16,26 +16,18 @@ std::string DesktopNotificationsTest::log_output_; void MockBalloonCollection::Add(const Notification& notification, Profile* profile) { - // Swap in the logging proxy for the purpose of logging calls that + // Swap in a logging proxy for the purpose of logging calls that // would be made into javascript, then pass this down to the // balloon collection. - Notification test_notification(notification.origin_url(), - notification.content_url(), - notification.display_source(), - string16(), /* replace_id */ - log_proxy_.get()); + Notification test_notification( + notification.origin_url(), + notification.content_url(), + notification.display_source(), + notification.replace_id(), + new LoggingNotificationProxy(notification.notification_id())); BalloonCollectionImpl::Add(test_notification, profile); } -bool MockBalloonCollection::Remove(const Notification& notification) { - Notification test_notification(notification.origin_url(), - notification.content_url(), - notification.display_source(), - string16(), /* replace_id */ - log_proxy_.get()); - return BalloonCollectionImpl::Remove(test_notification); -} - Balloon* MockBalloonCollection::MakeBalloon(const Notification& notification, Profile* profile) { // Start with a normal balloon but mock out the view. diff --git a/chrome/browser/notifications/desktop_notifications_unittest.h b/chrome/browser/notifications/desktop_notifications_unittest.h index 3b02b7b..9f556e4 100644 --- a/chrome/browser/notifications/desktop_notifications_unittest.h +++ b/chrome/browser/notifications/desktop_notifications_unittest.h @@ -22,15 +22,14 @@ #include "testing/gtest/include/gtest/gtest.h" class DesktopNotificationsTest; -typedef LoggingNotificationProxyBase<DesktopNotificationsTest> +typedef LoggingNotificationDelegate<DesktopNotificationsTest> LoggingNotificationProxy; // Test version of the balloon collection which counts the number // of notifications that are added to it. class MockBalloonCollection : public BalloonCollectionImpl { public: - MockBalloonCollection() : - log_proxy_(new LoggingNotificationProxy()) {} + MockBalloonCollection() {} // Our mock collection has an area large enough for a fixed number // of balloons. @@ -40,7 +39,6 @@ class MockBalloonCollection : public BalloonCollectionImpl { // BalloonCollectionImpl overrides virtual void Add(const Notification& notification, Profile* profile); - virtual bool Remove(const Notification& notification); virtual bool HasSpace() const { return count() < kMockBalloonSpace; } virtual Balloon* MakeBalloon(const Notification& notification, Profile* profile); @@ -63,7 +61,6 @@ class MockBalloonCollection : public BalloonCollectionImpl { private: std::deque<Balloon*> balloons_; - scoped_refptr<LoggingNotificationProxy> log_proxy_; }; class DesktopNotificationsTest : public testing::Test { diff --git a/chrome/browser/notifications/notification.cc b/chrome/browser/notifications/notification.cc index c429efa..e6d69a6 100644 --- a/chrome/browser/notifications/notification.cc +++ b/chrome/browser/notifications/notification.cc @@ -34,7 +34,3 @@ Notification& Notification::operator=(const Notification& notification) { delegate_ = notification.delegate(); return *this; } - -bool Notification::IsSame(const Notification& other) const { - return delegate()->id() == other.delegate()->id(); -} diff --git a/chrome/browser/notifications/notification.h b/chrome/browser/notifications/notification.h index 7b0ab8c..2040cf3 100644 --- a/chrome/browser/notifications/notification.h +++ b/chrome/browser/notifications/notification.h @@ -6,6 +6,8 @@ #define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_H_ #pragma once +#include <string> + #include "base/basictypes.h" #include "chrome/browser/notifications/notification_object_proxy.h" #include "googleurl/src/gurl.h" @@ -42,7 +44,7 @@ class Notification { void Click() const { delegate()->Click(); } void Close(bool by_user) const { delegate()->Close(by_user); } - bool IsSame(const Notification& other) const; + std::string notification_id() const { return delegate()->id(); } private: NotificationDelegate* delegate() const { return delegate_.get(); } diff --git a/chrome/browser/notifications/notification_options_menu_model.cc b/chrome/browser/notifications/notification_options_menu_model.cc index 97d269c..4370e63 100644 --- a/chrome/browser/notifications/notification_options_menu_model.cc +++ b/chrome/browser/notifications/notification_options_menu_model.cc @@ -10,6 +10,7 @@ #include "chrome/browser/browser_list.h" #include "chrome/browser/extensions/extensions_service.h" #include "chrome/browser/notifications/desktop_notification_service.h" +#include "chrome/browser/notifications/notification.h" #include "chrome/browser/notifications/notifications_prefs_cache.h" #include "chrome/browser/profile.h" #include "chrome/common/content_settings_types.h" diff --git a/chrome/browser/notifications/notification_test_util.h b/chrome/browser/notifications/notification_test_util.h index 4cbf600..fbaec4e 100644 --- a/chrome/browser/notifications/notification_test_util.h +++ b/chrome/browser/notifications/notification_test_util.h @@ -6,6 +6,8 @@ #define CHROME_BROWSER_NOTIFICATIONS_NOTIFICATION_TEST_UTIL_H_ #pragma once +#include <string> + #include "chrome/browser/notifications/notification_object_proxy.h" #include "chrome/browser/notifications/balloon.h" #include "gfx/size.h" @@ -14,7 +16,7 @@ // the notification events are not important. class MockNotificationDelegate : public NotificationDelegate { public: - explicit MockNotificationDelegate(std::string id) : id_(id) {} + explicit MockNotificationDelegate(const std::string& id) : id_(id) {} virtual ~MockNotificationDelegate() {} // NotificationDelegate interface. @@ -26,6 +28,8 @@ class MockNotificationDelegate : public NotificationDelegate { private: std::string id_; + + DISALLOW_COPY_AND_ASSIGN(MockNotificationDelegate); }; // Mock implementation of Javascript object proxy which logs events that @@ -35,10 +39,11 @@ class MockNotificationDelegate : public NotificationDelegate { // |Logger| class provided in template must implement method // static void log(string); template<class Logger> -class LoggingNotificationProxyBase : public NotificationObjectProxy { +class LoggingNotificationDelegate : public NotificationDelegate { public: - LoggingNotificationProxyBase() : - NotificationObjectProxy(0, 0, 0, false) {} + explicit LoggingNotificationDelegate(std::string id) + : notification_id_(id) { + } // NotificationObjectProxy override virtual void Display() { @@ -47,12 +52,22 @@ class LoggingNotificationProxyBase : public NotificationObjectProxy { virtual void Error() { Logger::log("notification error\n"); } + virtual void Click() { + Logger::log("notification clicked\n"); + } virtual void Close(bool by_user) { if (by_user) Logger::log("notification closed by user\n"); else Logger::log("notification closed by script\n"); } + virtual std::string id() const { + return notification_id_; + } + private: + std::string notification_id_; + + DISALLOW_COPY_AND_ASSIGN(LoggingNotificationDelegate); }; // Test version of a balloon view which doesn't do anything diff --git a/chrome/browser/notifications/notification_ui_manager.cc b/chrome/browser/notifications/notification_ui_manager.cc index adc44cf..27f7626 100644 --- a/chrome/browser/notifications/notification_ui_manager.cc +++ b/chrome/browser/notifications/notification_ui_manager.cc @@ -66,17 +66,34 @@ void NotificationUIManager::Add(const Notification& notification, CheckAndShowNotifications(); } -bool NotificationUIManager::Cancel(const Notification& notification) { - // First look through the notifications that haven't been shown. If not - // found there, call to the active balloon collection to tear it down. +bool NotificationUIManager::CancelById(const std::string& id) { + // See if this ID hasn't been shown yet. NotificationDeque::iterator iter; for (iter = show_queue_.begin(); iter != show_queue_.end(); ++iter) { - if (notification.IsSame((*iter)->notification())) { + if ((*iter)->notification().notification_id() == id) { show_queue_.erase(iter); return true; } } - return balloon_collection_->Remove(notification); + // If it has been shown, remove it from the balloon collections. + return balloon_collection_->RemoveById(id); +} + +bool NotificationUIManager::CancelAllBySourceOrigin(const GURL& source) { + // Same pattern as CancelById, but more complicated than the above + // because there may be multiple notifications from the same source. + bool removed = false; + NotificationDeque::iterator iter; + for (iter = show_queue_.begin(); iter != show_queue_.end(); ++iter) { + if ((*iter)->notification().origin_url() == source) { + iter = show_queue_.erase(iter); + removed = true; + } else { + ++iter; + } + } + + return balloon_collection_->RemoveBySourceOrigin(source) || removed; } void NotificationUIManager::CheckAndShowNotifications() { diff --git a/chrome/browser/notifications/notification_ui_manager.h b/chrome/browser/notifications/notification_ui_manager.h index 10687a9..6d46198 100644 --- a/chrome/browser/notifications/notification_ui_manager.h +++ b/chrome/browser/notifications/notification_ui_manager.h @@ -7,6 +7,7 @@ #pragma once #include <deque> +#include <string> #include "base/id_map.h" #include "base/scoped_ptr.h" @@ -43,8 +44,14 @@ class NotificationUIManager virtual void Add(const Notification& notification, Profile* profile); - // Removes a notification. - virtual bool Cancel(const Notification& notification); + // Removes any notifications matching the supplied ID, either currently + // displayed or in the queue. Returns true if anything was removed. + virtual bool CancelById(const std::string& notification_id); + + // Removes any notifications matching the supplied source origin + // (which could be an extension ID), either currently displayed or in the + // queue. Returns true if anything was removed. + virtual bool CancelAllBySourceOrigin(const GURL& source_origin); // Returns balloon collection. BalloonCollection* balloon_collection() { diff --git a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc index 9607e3f..c27b1a3 100644 --- a/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc +++ b/chrome/browser/printing/cloud_print/cloud_print_proxy_service.cc @@ -124,9 +124,8 @@ void CloudPrintProxyService::OnTokenExpiredNotificationClick() { void CloudPrintProxyService::TokenExpiredNotificationDone(bool keep_alive) { if (token_expired_delegate_.get()) { - g_browser_process->notification_ui_manager()->Cancel( - Notification(GURL(), GURL(), string16(), string16(), - token_expired_delegate_.get())); + g_browser_process->notification_ui_manager()->CancelById( + token_expired_delegate_->id()); token_expired_delegate_ = NULL; if (!keep_alive) BrowserList::EndKeepAlive(); diff --git a/chrome/browser/task_manager/task_manager_browsertest.cc b/chrome/browser/task_manager/task_manager_browsertest.cc index 7230254..fa256f2 100644 --- a/chrome/browser/task_manager/task_manager_browsertest.cc +++ b/chrome/browser/task_manager/task_manager_browsertest.cc @@ -15,6 +15,7 @@ #include "chrome/browser/extensions/crashed_extension_infobar.h" #include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/notifications/desktop_notification_service.h" +#include "chrome/browser/notifications/notification.h" #include "chrome/browser/notifications/notification_test_util.h" #include "chrome/browser/notifications/notification_ui_manager.h" #include "chrome/browser/profile.h" @@ -200,9 +201,9 @@ IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, NoticeNotificationChanges) { WaitForResourceChange(3); notifications->Add(n2, browser()->profile()); WaitForResourceChange(4); - notifications->Cancel(n1); + notifications->CancelById(n1.notification_id()); WaitForResourceChange(3); - notifications->Cancel(n2); + notifications->CancelById(n2.notification_id()); WaitForResourceChange(2); } diff --git a/chrome/browser/ui/views/notifications/balloon_view.cc b/chrome/browser/ui/views/notifications/balloon_view.cc index cf0442b..d8296b1 100644 --- a/chrome/browser/ui/views/notifications/balloon_view.cc +++ b/chrome/browser/ui/views/notifications/balloon_view.cc @@ -13,6 +13,7 @@ #include "chrome/browser/notifications/balloon.h" #include "chrome/browser/notifications/balloon_collection.h" #include "chrome/browser/notifications/desktop_notification_service.h" +#include "chrome/browser/notifications/notification.h" #include "chrome/browser/notifications/notification_options_menu_model.h" #include "chrome/browser/profile.h" #include "chrome/browser/renderer_host/render_view_host.h" diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index e17ae8c9..ec81d3a 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2295,6 +2295,8 @@ 'browser/notifications/balloon_host.cc', 'browser/notifications/balloon_collection.cc', 'browser/notifications/balloon_collection.h', + 'browser/notifications/balloon_collection_base.cc', + 'browser/notifications/balloon_collection_base.h', 'browser/notifications/balloon_collection_impl.h', 'browser/notifications/balloon_collection_win.cc', 'browser/notifications/balloon_collection_mac.mm', |