diff options
6 files changed, 80 insertions, 39 deletions
diff --git a/chrome/browser/chromeos/notifications/balloon_collection_impl.cc b/chrome/browser/chromeos/notifications/balloon_collection_impl.cc index 83317aa..a0847f8 100644 --- a/chrome/browser/chromeos/notifications/balloon_collection_impl.cc +++ b/chrome/browser/chromeos/notifications/balloon_collection_impl.cc @@ -83,13 +83,25 @@ void BalloonCollectionImpl::AddSystemNotification( bool BalloonCollectionImpl::UpdateNotification( const Notification& notification) { Balloons::iterator iter = FindBalloon(notification); - if (iter != balloons_.end()) { - Balloon* balloon = *iter; - balloon->Update(notification); - notification_ui_->Update(balloon); - return true; - } - return false; + if (iter == balloons_.end()) + return false; + Balloon* balloon = *iter; + balloon->Update(notification); + notification_ui_->Update(balloon); + return true; +} + +bool BalloonCollectionImpl::UpdateAndShowNotification( + const Notification& notification) { + Balloons::iterator iter = FindBalloon(notification); + if (iter == balloons_.end()) + return false; + Balloon* balloon = *iter; + balloon->Update(notification); + bool updated = notification_ui_->Update(balloon); + DCHECK(updated); + notification_ui_->Show(balloon); + return true; } bool BalloonCollectionImpl::Remove(const Notification& notification) { diff --git a/chrome/browser/chromeos/notifications/balloon_collection_impl.h b/chrome/browser/chromeos/notifications/balloon_collection_impl.h index df9b534..7ae7763 100644 --- a/chrome/browser/chromeos/notifications/balloon_collection_impl.h +++ b/chrome/browser/chromeos/notifications/balloon_collection_impl.h @@ -36,10 +36,11 @@ class BalloonCollectionImpl : public BalloonCollection, NotificationUI() {} virtual ~NotificationUI() {} - // Add, remove and resize the balloon. + // Add, remove, resize and show the balloon. virtual void Add(Balloon* balloon) = 0; virtual bool Update(Balloon* balloon) = 0; virtual void Remove(Balloon* balloon) = 0; + virtual void Show(Balloon* balloon) = 0; // Resize notification from webkit. virtual void ResizeNotification(Balloon* balloon, @@ -76,12 +77,18 @@ class BalloonCollectionImpl : public BalloonCollection, void AddSystemNotification(const Notification& notification, Profile* profile, bool sticky, bool controls); - // Update the notification's content. It uses + // Updates the notification's content. It uses // NotificationDelegate::id() to check the equality of notifications. // Returns true if the notification has been updated. False if - // no corresponding notification is found. + // no corresponding notification is found. This will not change the + // visibility of the notification. bool UpdateNotification(const Notification& notification); + // Updates and shows the notification. It will open the notification panel + // if it's closed or minimized, and scroll the viewport so that + // the updated notification is visible. + bool UpdateAndShowNotification(const Notification& notification); + // Injects notification ui. Used to inject a mock implementation in tests. void set_notification_ui(NotificationUI* ui) { notification_ui_.reset(ui); diff --git a/chrome/browser/chromeos/notifications/desktop_notifications_unittest.cc b/chrome/browser/chromeos/notifications/desktop_notifications_unittest.cc index 424ea0e..5b0a200 100644 --- a/chrome/browser/chromeos/notifications/desktop_notifications_unittest.cc +++ b/chrome/browser/chromeos/notifications/desktop_notifications_unittest.cc @@ -14,6 +14,7 @@ class MockNotificationUI : public BalloonCollectionImpl::NotificationUI { virtual void Add(Balloon* balloon) {} virtual bool Update(Balloon* balloon) { return false; } virtual void Remove(Balloon* balloon) {} + virtual void Show(Balloon* balloon) {} virtual void ResizeNotification(Balloon* balloon, const gfx::Size& size) {} virtual void SetActiveView(BalloonViewImpl* view) {} diff --git a/chrome/browser/chromeos/notifications/notification_browsertest.cc b/chrome/browser/chromeos/notifications/notification_browsertest.cc index 52a67f1..35d7833 100644 --- a/chrome/browser/chromeos/notifications/notification_browsertest.cc +++ b/chrome/browser/chromeos/notifications/notification_browsertest.cc @@ -239,6 +239,13 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, TestSystemNotification) { EXPECT_EQ(1, tester->GetStickyNotificationCount()); + Notification update_and_show = SystemNotificationFactory::Create( + GURL(), ASCIIToUTF16("Title"), ASCIIToUTF16("updated and shown"), + delegate.get()); + collection->UpdateAndShowNotification(update_and_show); + + EXPECT_EQ(1, tester->GetStickyNotificationCount()); + // Dismiss the notification. // TODO(oshima): Consider updating API to Remove(NotificationDelegate) // or Remove(std::string id); @@ -440,24 +447,45 @@ IN_PROC_BROWSER_TEST_F(NotificationTest, TestScrollBalloonToVisible) { EXPECT_TRUE(tester->IsVisible(view)); } } - // updated notification must be also visible + // Update should not change the visibility for (int i = 0; i < create_count; i++) { { SCOPED_TRACE(StringPrintf("update n%d", i)); - std::string id = StringPrintf("n%d", i); - EXPECT_TRUE(collection->UpdateNotification(NewMockNotification(id))); + Notification notify = NewMockNotification(StringPrintf("n%d", i)); + // The last shown notification is sticky, which makes all non sticky + // invisible. + EXPECT_TRUE(collection->UpdateNotification(notify)); ui_test_utils::RunAllPendingInMessageLoop(); - BalloonViewImpl* view = - tester->GetBalloonView(collection, NewMockNotification(id)); - EXPECT_TRUE(tester->IsVisible(view)); + BalloonViewImpl* view = tester->GetBalloonView(collection, notify); + EXPECT_FALSE(tester->IsVisible(view)); } { SCOPED_TRACE(StringPrintf("update s%d", i)); - std::string id = StringPrintf("s%d", i); - EXPECT_TRUE(collection->UpdateNotification(NewMockNotification(id))); + Notification notify = NewMockNotification(StringPrintf("s%d", i)); + BalloonViewImpl* view = tester->GetBalloonView(collection, notify); + bool currently_visible = tester->IsVisible(view); + EXPECT_TRUE(collection->UpdateNotification(notify)); ui_test_utils::RunAllPendingInMessageLoop(); - BalloonViewImpl* view = - tester->GetBalloonView(collection, NewMockNotification(id)); + EXPECT_EQ(view, tester->GetBalloonView(collection, notify)); + EXPECT_EQ(currently_visible, tester->IsVisible(view)); + } + } + // UpdateAndShowNotification makes notification visible + for (int i = 0; i < create_count; i++) { + { + SCOPED_TRACE(StringPrintf("update and show n%d", i)); + Notification notify = NewMockNotification(StringPrintf("n%d", i)); + EXPECT_TRUE(collection->UpdateAndShowNotification(notify)); + ui_test_utils::RunAllPendingInMessageLoop(); + BalloonViewImpl* view = tester->GetBalloonView(collection, notify); + EXPECT_TRUE(tester->IsVisible(view)); + } + { + SCOPED_TRACE(StringPrintf("update and show s%d", i)); + Notification notify = NewMockNotification(StringPrintf("s%d", i)); + EXPECT_TRUE(collection->UpdateAndShowNotification(notify)); + ui_test_utils::RunAllPendingInMessageLoop(); + BalloonViewImpl* view = tester->GetBalloonView(collection, notify); EXPECT_TRUE(tester->IsVisible(view)); } } diff --git a/chrome/browser/chromeos/notifications/notification_panel.cc b/chrome/browser/chromeos/notifications/notification_panel.cc index 67f68c6..46b44709 100644 --- a/chrome/browser/chromeos/notifications/notification_panel.cc +++ b/chrome/browser/chromeos/notifications/notification_panel.cc @@ -502,23 +502,11 @@ void NotificationPanel::Add(Balloon* balloon) { UpdatePanel(false); UpdateControl(); StartStaleTimer(balloon); - if (is_visible()) - scroll_to_ = balloon; + scroll_to_ = balloon; } bool NotificationPanel::Update(Balloon* balloon) { - if (balloon_container_->Update(balloon)) { - if (state_ == CLOSED || state_ == MINIMIZED) - SET_STATE(STICKY_AND_NEW); - Show(); - UpdatePanel(true); - StartStaleTimer(balloon); - if (is_visible()) - ScrollBalloonToVisible(balloon); - return true; - } else { - return false; - } + return balloon_container_->Update(balloon); } void NotificationPanel::Remove(Balloon* balloon) { @@ -545,6 +533,15 @@ void NotificationPanel::Remove(Balloon* balloon) { UpdateControl(); } +void NotificationPanel::Show(Balloon* balloon) { + if (state_ == CLOSED || state_ == MINIMIZED) + SET_STATE(STICKY_AND_NEW); + Show(); + UpdatePanel(true); + StartStaleTimer(balloon); + ScrollBalloonToVisible(balloon); +} + void NotificationPanel::ResizeNotification( Balloon* balloon, const gfx::Size& size) { // restrict to the min & max sizes diff --git a/chrome/browser/chromeos/notifications/notification_panel.h b/chrome/browser/chromeos/notifications/notification_panel.h index 3d10a00..7b795e45 100644 --- a/chrome/browser/chromeos/notifications/notification_panel.h +++ b/chrome/browser/chromeos/notifications/notification_panel.h @@ -91,6 +91,7 @@ class NotificationPanel : public PanelController::Delegate, virtual void Add(Balloon* balloon); virtual bool Update(Balloon* balloon); virtual void Remove(Balloon* balloon); + virtual void Show(Balloon* balloon); virtual void ResizeNotification(Balloon* balloon, const gfx::Size& size); virtual void SetActiveView(BalloonViewImpl* view); @@ -151,11 +152,6 @@ class NotificationPanel : public PanelController::Delegate, // Mark the given notification as stale. void MarkStale(const Notification& notification); - // True if the panel is visible. - bool is_visible() { - return state_ != CLOSED && state_ != MINIMIZED; - } - // Contains all notifications. This is owned by the panel so that we can // re-attach to the widget when closing and opening the panel. scoped_ptr<BalloonContainer> balloon_container_; |