Reset the unread count when notifications are marked.

The last patch was reverted due to a crash during tests. That's
because some tests get a const reference of the visible notifications
and call MarkSinglePopupAsShown with a for-loop. So, recreating
the list cache during MarkSinglePopupAsShown will break the
const reference.

Instead, what we really want was to just update the unread count.
This CL introduces a new method to update the unread count only.

R=dewittj@chromium.org
BUG=327055
TEST=new test case covers

Review URL: https://codereview.chromium.org/113033002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@240452 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 50 insertions, 2 deletions

diff --git a/ui/message_center/message_center_impl.cc b/ui/message_center/message_center_impl.cc
index 45e91c4..22c473d 100644
--- a/ui/message_center/message_center_impl.cc
+++ b/ui/message_center/message_center_impl.cc
@@ -401,9 +401,14 @@ MessageCenterImpl::NotificationCache::~NotificationCache() {}
 void MessageCenterImpl::NotificationCache::Rebuild(
     const NotificationList::Notifications& notifications) {
   visible_notifications = notifications;
+  RecountUnread();
+}
+
+void MessageCenterImpl::NotificationCache::RecountUnread() {
   unread_count = 0;
   for (NotificationList::Notifications::const_iterator iter =
-           notifications.begin(); iter != notifications.end(); ++iter) {
+           visible_notifications.begin();
+       iter != visible_notifications.end(); ++iter) {
     if (!(*iter)->IsRead())
       ++unread_count;
   }
@@ -456,7 +461,14 @@ void MessageCenterImpl::OnBlockingStateChanged(NotificationBlocker* blocker) {
 
   for (std::list<std::string>::const_iterator iter = blocked_ids.begin();
        iter != blocked_ids.end(); ++iter) {
-    MarkSinglePopupAsShown((*iter), true);
+    // Do not call MessageCenterImpl::MarkSinglePopupAsShown() directly here
+    // just for performance reason. MessageCenterImpl::MarkSinglePopupAsShown()
+    // calls NotificationList::MarkSinglePopupAsShown() and then updates the
+    // unread count, but the whole cache will be recreated below.
+    notification_list_->MarkSinglePopupAsShown((*iter), true);
+    FOR_EACH_OBSERVER(MessageCenterObserver,
+                      observer_list_,
+                      OnNotificationUpdated(*iter));
   }
   notification_cache_.Rebuild(
       notification_list_->GetVisibleNotifications(blockers_));
@@ -469,6 +481,7 @@ void MessageCenterImpl::SetVisibility(Visibility visibility) {
   std::set<std::string> updated_ids;
   notification_list_->SetMessageCenterVisible(
       (visibility == VISIBILITY_MESSAGE_CENTER), &updated_ids);
+  notification_cache_.RecountUnread();
 
   for (std::set<std::string>::const_iterator iter = updated_ids.begin();
        iter != updated_ids.end();
@@ -793,6 +806,7 @@ void MessageCenterImpl::MarkSinglePopupAsShown(const std::string& id,
   if (!HasNotification(id))
     return;
   notification_list_->MarkSinglePopupAsShown(id, mark_notification_as_read);
+  notification_cache_.RecountUnread();
   FOR_EACH_OBSERVER(
       MessageCenterObserver, observer_list_, OnNotificationUpdated(id));
 }
@@ -803,6 +817,7 @@ void MessageCenterImpl::DisplayedNotification(const std::string& id) {
 
   if (HasPopupNotifications())
     notification_list_->MarkSinglePopupAsDisplayed(id);
+  notification_cache_.RecountUnread();
   NotificationDelegate* delegate =
       notification_list_->GetNotificationDelegate(id);
   if (delegate)
diff --git a/ui/message_center/message_center_impl.h b/ui/message_center/message_center_impl.h
index 3768d96..faee658 100644
--- a/ui/message_center/message_center_impl.h
+++ b/ui/message_center/message_center_impl.h
@@ -199,6 +199,7 @@ class MessageCenterImpl : public MessageCenter,
     NotificationCache();
     ~NotificationCache();
     void Rebuild(const NotificationList::Notifications& notificaitons);
+    void RecountUnread();
 
     NotificationList::Notifications visible_notifications;
     size_t unread_count;
diff --git a/ui/message_center/message_center_impl_unittest.cc b/ui/message_center/message_center_impl_unittest.cc
index 64b2909..b26c446 100644
--- a/ui/message_center/message_center_impl_unittest.cc
+++ b/ui/message_center/message_center_impl_unittest.cc
@@ -726,5 +726,37 @@ TEST_F(MessageCenterImplTest, QueuedDirectUpdates) {
   EXPECT_EQ(new_size, buttons[1].icon.Size());
 }
 
+TEST_F(MessageCenterImplTest, CachedUnreadCount) {
+  message_center()->AddNotification(
+      scoped_ptr<Notification>(CreateSimpleNotification("id1")));
+  message_center()->AddNotification(
+      scoped_ptr<Notification>(CreateSimpleNotification("id2")));
+  message_center()->AddNotification(
+      scoped_ptr<Notification>(CreateSimpleNotification("id3")));
+  ASSERT_EQ(3u, message_center()->UnreadNotificationCount());
+
+  // Mark 'displayed' on all notifications by using for-loop. This shouldn't
+  // recreate |notifications| inside of the loop.
+  const NotificationList::Notifications& notifications =
+      message_center()->GetVisibleNotifications();
+  for (NotificationList::Notifications::const_iterator iter =
+           notifications.begin(); iter != notifications.end(); ++iter) {
+    message_center()->DisplayedNotification((*iter)->id());
+  }
+  EXPECT_EQ(0u, message_center()->UnreadNotificationCount());
+
+  // Imitate the timeout, which recovers the unread count. Again, this shouldn't
+  // recreate |notifications| inside of the loop.
+  for (NotificationList::Notifications::const_iterator iter =
+           notifications.begin(); iter != notifications.end(); ++iter) {
+    message_center()->MarkSinglePopupAsShown((*iter)->id(), false);
+  }
+  EXPECT_EQ(3u, message_center()->UnreadNotificationCount());
+
+  // Opening the message center will reset the unread count.
+  message_center()->SetVisibility(VISIBILITY_MESSAGE_CENTER);
+  EXPECT_EQ(0u, message_center()->UnreadNotificationCount());
+}
+
 }  // namespace internal
 }  // namespace message_center