Sends updated event for click on notifications.

BUG=232231
TEST=manually

Review URL: https://chromiumcodereview.appspot.com/14017014

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

ui/message_center/message_center_impl.cc

@@ -212,7 +212,7 @@ void MessageCenterImpl::ExpandNotification(const std::string& id) {
 
 void MessageCenterImpl::ClickOnNotification(const std::string& id) {
   if (HasPopupNotifications())
-    notification_list_->MarkSinglePopupAsShown(id, true);
+    MarkSinglePopupAsShown(id, true);
   FOR_EACH_OBSERVER(MessageCenterObserver, observer_list_,
                     OnNotificationClicked(id));
 }
@@ -220,7 +220,7 @@ void MessageCenterImpl::ClickOnNotification(const std::string& id) {
 void MessageCenterImpl::ClickOnNotificationButton(const std::string& id,
                                               int button_index) {
   if (HasPopupNotifications())
-    notification_list_->MarkSinglePopupAsShown(id, true);
+    MarkSinglePopupAsShown(id, true);
   FOR_EACH_OBSERVER(MessageCenterObserver, observer_list_,
                     OnNotificationButtonClicked(id, button_index));
 }

ui/message_center/views/message_popup_collection.cc

@@ -387,10 +387,10 @@ void MessagePopupCollection::OnNotificationUpdated(
   if (toast_iter == toasts_.end())
     return;
 
-  NotificationList::Notifications notifications =
-      message_center_->GetNotifications();
+  NotificationList::PopupNotifications notifications =
+      message_center_->GetPopupNotifications();
   bool updated = false;
-  for (NotificationList::Notifications::iterator iter =
+  for (NotificationList::PopupNotifications::iterator iter =
            notifications.begin(); iter != notifications.end(); ++iter) {
     if ((*iter)->id() != notification_id)
       continue;
@@ -403,15 +403,29 @@ void MessagePopupCollection::OnNotificationUpdated(
     updated = true;
   }
 
-  if (updated) {
-    RepositionWidgets();
-    // Reposition could create extra space which allows additional widgets.
-    UpdateWidgets();
+  // OnNotificationUpdated() can be called when a notification is excluded from
+  // the popup notification list but still remains in the full notification
+  // list. In that case the widget for the notification has to be closed here.
+  if (!updated) {
+    views::Widget* widget = toast_iter->second->GetWidget();
+    widget->RemoveObserver(this);
+    widgets_.erase(std::find(widgets_.begin(), widgets_.end(), widget));
+    widget->Close();
+    toasts_.erase(toast_iter);
   }
+
+  RepositionWidgets();
+  // Reposition could create extra space which allows additional widgets.
+  UpdateWidgets();
 }
 
 void MessagePopupCollection::SetWorkAreaForTest(const gfx::Rect& work_area) {
   work_area_ = work_area;
 }
 
+views::Widget* MessagePopupCollection::GetWidgetForId(const std::string& id) {
+  ToastContainer::const_iterator iter = toasts_.find(id);
+  return (iter == toasts_.end()) ? NULL : iter->second->GetWidget();
+}
+
 }  // namespace message_center

ui/message_center/views/message_popup_collection.h

@@ -28,6 +28,7 @@ FORWARD_DECLARE_TEST(WebNotificationTrayTest, ManyPopupNotifications);
 namespace message_center {
 namespace test {
 class MessagePopupCollectionTest;
+class MessagePopupCollectionWidgetsTest;
 }
 
 class MessageCenter;
@@ -56,6 +57,7 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection
   FRIEND_TEST_ALL_PREFIXES(ash::WebNotificationTrayTest,
                            ManyPopupNotifications);
   friend class test::MessagePopupCollectionTest;
+  friend class test::MessagePopupCollectionWidgetsTest;
   typedef std::map<std::string, ToastContentsView*> ToastContainer;
 
   void CloseAllWidgets();
@@ -86,6 +88,7 @@ class MESSAGE_CENTER_EXPORT MessagePopupCollection
       const std::string& notification_id) OVERRIDE;
 
   void SetWorkAreaForTest(const gfx::Rect& work_area);
+  views::Widget* GetWidgetForId(const std::string& id);
 
   gfx::NativeView parent_;
   MessageCenter* message_center_;

ui/message_center/views/message_popup_collection_unittest.cc

@@ -7,6 +7,8 @@
 #include <list>
 
 #include "base/message_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/rect.h"
 #include "ui/message_center/fake_message_center.h"
@@ -17,6 +19,48 @@ namespace message_center {
 namespace test {
 
 class MessagePopupCollectionTest : public views::ViewsTestBase {
+ public:
+  virtual void SetUp() OVERRIDE {
+    views::ViewsTestBase::SetUp();
+    MessageCenter::Initialize();
+    collection_.reset(
+        new MessagePopupCollection(GetContext(), MessageCenter::Get()));
+    collection_->SetWorkAreaForTest(gfx::Rect(0, 0, 1280, 1024));
+    id_ = 0;
+  }
+
+  virtual void TearDown() OVERRIDE {
+    collection_->CloseAllWidgets();
+    collection_.reset();
+    MessageCenter::Shutdown();
+    views::ViewsTestBase::TearDown();
+  }
+
+ protected:
+  size_t GetToastCounts() {
+    return collection_->toasts_.size();
+  }
+
+  bool IsToastShown(const std::string& id) {
+    views::Widget* widget = collection_->GetWidgetForId(id);
+    return widget && widget->IsVisible();
+  }
+
+  std::string AddNotification() {
+    std::string id = base::IntToString(id_++);
+    MessageCenter::Get()->AddNotification(
+        NOTIFICATION_TYPE_BASE_FORMAT, id, UTF8ToUTF16("test title"),
+        UTF8ToUTF16("test message"), string16() /* display_source */,
+        "" /* extension_id */, NULL);
+    return id;
+  }
+
+ private:
+  scoped_ptr<MessagePopupCollection> collection_;
+  int id_;
+};
+
+class MessagePopupCollectionWidgetsTest : public views::ViewsTestBase {
  public:
   virtual void SetUp() OVERRIDE {
     views::ViewsTestBase::SetUp();
@@ -69,7 +113,27 @@ class MessagePopupCollectionTest : public views::ViewsTestBase {
   scoped_ptr<MessagePopupCollection> collection_;
 };
 
-TEST_F(MessagePopupCollectionTest, RepositionWidgets) {
+TEST_F(MessagePopupCollectionTest, DismissOnClick) {
+  std::string id1 = AddNotification();
+  std::string id2 = AddNotification();
+  EXPECT_EQ(2u, GetToastCounts());
+  EXPECT_TRUE(IsToastShown(id1));
+  EXPECT_TRUE(IsToastShown(id2));
+
+  MessageCenter::Get()->ClickOnNotification(id2);
+  RunPendingMessages();
+  EXPECT_EQ(1u, GetToastCounts());
+  EXPECT_TRUE(IsToastShown(id1));
+  EXPECT_FALSE(IsToastShown(id2));
+
+  MessageCenter::Get()->ClickOnNotificationButton(id1, 0);
+  RunPendingMessages();
+  EXPECT_EQ(0u, GetToastCounts());
+  EXPECT_FALSE(IsToastShown(id1));
+  EXPECT_FALSE(IsToastShown(id2));
+}
+
+TEST_F(MessagePopupCollectionWidgetsTest, RepositionWidgets) {
   std::vector<gfx::Rect> rects;
   std::list<views::Widget*> widgets;
   rects.push_back(gfx::Rect(0, 0, 10, 10));
@@ -85,7 +149,7 @@ TEST_F(MessagePopupCollectionTest, RepositionWidgets) {
   EXPECT_EQ("0,60 10x40", GetWidgetRectAt(3).ToString());
 }
 
-TEST_F(MessagePopupCollectionTest, RepositionWidgetsWithTargetDown) {
+TEST_F(MessagePopupCollectionWidgetsTest, RepositionWidgetsWithTargetDown) {
   std::vector<gfx::Rect> rects;
   std::list<views::Widget*> widgets;
   rects.push_back(gfx::Rect(0, 180, 10, 10));
@@ -99,7 +163,7 @@ TEST_F(MessagePopupCollectionTest, RepositionWidgetsWithTargetDown) {
   EXPECT_EQ("0,110 10x40", GetWidgetRectAt(2).ToString());
 }
 
-TEST_F(MessagePopupCollectionTest, RepositionWidgetsWithTargetDownAll) {
+TEST_F(MessagePopupCollectionWidgetsTest, RepositionWidgetsWithTargetDownAll) {
   std::vector<gfx::Rect> rects;
   std::list<views::Widget*> widgets;
   rects.push_back(gfx::Rect(0, 150, 10, 20));
@@ -113,7 +177,7 @@ TEST_F(MessagePopupCollectionTest, RepositionWidgetsWithTargetDownAll) {
   EXPECT_EQ("0,90 10x40", GetWidgetRectAt(2).ToString());
 }
 
-TEST_F(MessagePopupCollectionTest, RepositionWidgetsWithTargetUp) {
+TEST_F(MessagePopupCollectionWidgetsTest, RepositionWidgetsWithTargetUp) {
   std::vector<gfx::Rect> rects;
   std::list<views::Widget*> widgets;
   rects.push_back(gfx::Rect(0, 180, 10, 10));