diff options
34 files changed, 362 insertions, 231 deletions
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc index 761911d..ac2622e 100644 --- a/chrome/browser/automation/testing_automation_provider.cc +++ b/chrome/browser/automation/testing_automation_provider.cc @@ -4296,7 +4296,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 b5830d5..82cd702 100644 --- a/chrome/browser/chromeos/notifications/notification_browsertest.cc +++ b/chrome/browser/chromeos/notifications/notification_browsertest.cc @@ -15,7 +15,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/browser/ui/browser.h" #include "chrome/common/notification_service.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 08d25db..8aa82d1 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,16 +260,24 @@ 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()); } + + notification_registrar_.Add(this, NotificationType::PROFILE_DESTROYED, + Source<Profile>(profile_)); } void DesktopNotificationService::StopObserving() { if (!profile_->IsOffTheRecord()) { - registrar_.RemoveAll(); + prefs_registrar_.RemoveAll(); } + notification_registrar_.RemoveAll(); } void DesktopNotificationService::GrantPermission(const GURL& origin) { @@ -303,11 +311,24 @@ 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()); + } else if (NotificationType::PROFILE_DESTROYED == type) { + StopObserving(); + } +} + +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) { NotifySettingsChange(); std::vector<GURL> allowed_origins(GetAllowedOrigins()); @@ -318,7 +339,7 @@ void DesktopNotificationService::Observe(NotificationType type, prefs_cache_.get(), &NotificationsPrefsCache::SetCacheAllowedOrigins, allowed_origins)); - } else if (name == prefs::kDesktopNotificationDeniedOrigins) { + } else if (pref_name == prefs::kDesktopNotificationDeniedOrigins) { NotifySettingsChange(); std::vector<GURL> denied_origins(GetBlockedOrigins()); @@ -329,7 +350,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), @@ -561,9 +582,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 51e3493..a7a4ce2 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); @@ -146,7 +148,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_exceptions_table_model.cc b/chrome/browser/notifications/notification_exceptions_table_model.cc index 1f8b228..a3326f9 100644 --- a/chrome/browser/notifications/notification_exceptions_table_model.cc +++ b/chrome/browser/notifications/notification_exceptions_table_model.cc @@ -11,6 +11,8 @@ #include "chrome/common/content_settings.h" #include "chrome/common/content_settings_helper.h" #include "chrome/common/content_settings_types.h" +#include "chrome/common/notification_service.h" +#include "chrome/common/notification_type.h" #include "chrome/common/url_constants.h" #include "grit/generated_resources.h" 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 91cc23d..bc2655a 100644 --- a/chrome/browser/task_manager/task_manager_browsertest.cc +++ b/chrome/browser/task_manager/task_manager_browsertest.cc @@ -12,6 +12,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 c394b9a..d2696d4 100644 --- a/chrome/browser/ui/views/notifications/balloon_view.cc +++ b/chrome/browser/ui/views/notifications/balloon_view.cc @@ -14,6 +14,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 960411c..8cbfa84 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2303,6 +2303,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', |