Do not allow updates to notifications while center is open.

This will enable a stable interface while the user is interacting with the message center. BUG=246637 Review URL: https://chromiumcodereview.appspot.com/16502003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204722 0039d316-1c4b-4281-b951-d872f2087c98
marcosholgado · Jun 7, 2013 · d560d4d · d560d4d
1 parent afc6652
commit d560d4d
Show file tree

Hide file tree

Showing 10 changed files with 102 additions and 2 deletions.
diff --git a/chrome/browser/notifications/message_center_notification_manager.cc b/chrome/browser/notifications/message_center_notification_manager.cc
@@ -133,16 +133,26 @@ void MessageCenterNotificationManager::CancelAll() {
 
 bool MessageCenterNotificationManager::ShowNotification(
     const Notification& notification, Profile* profile) {
-  // There is always space in MessageCenter, it never rejects Notifications.
+  if (message_center_->IsMessageCenterVisible())
+    return false;
+
+  if (UpdateNotification(notification, profile))
+    return true;
+
   AddProfileNotification(
       new ProfileNotification(profile, notification, message_center_));
   return true;
 }
 
 bool MessageCenterNotificationManager::UpdateNotification(
     const Notification& notification, Profile* profile) {
+  if (message_center_->IsMessageCenterVisible())
+    return false;
+
   const string16& replace_id = notification.replace_id();
-  DCHECK(!replace_id.empty());
+  if (replace_id.empty())
+    return false;
+
   const GURL origin_url = notification.origin_url();
   DCHECK(origin_url.is_valid());
 
@@ -266,6 +276,12 @@ void MessageCenterNotificationManager::OnNotificationRemoved(
     RemoveProfileNotification(iter->second, by_user);
 }
 
+void MessageCenterNotificationManager::OnNotificationCenterClosed() {
+  // When the center is open it halts all notifications, so we need to listen
+  // for events indicating it's been closed.
+  CheckAndShowNotifications();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // ImageDownloads
 

diff --git a/chrome/browser/notifications/message_center_notification_manager.h b/chrome/browser/notifications/message_center_notification_manager.h
@@ -58,6 +58,7 @@ class MessageCenterNotificationManager
   // MessageCenterObserver
   virtual void OnNotificationRemoved(const std::string& notification_id,
                                      bool by_user) OVERRIDE;
+  virtual void OnNotificationCenterClosed() OVERRIDE;
 
  private:
   class ImageDownloadsObserver {

diff --git a/chrome/browser/notifications/message_center_notifications_browsertest.cc b/chrome/browser/notifications/message_center_notifications_browsertest.cc
@@ -5,6 +5,8 @@
 #include <string>
 
 #include "base/command_line.h"
+#include "base/message_loop.h"
+#include "base/run_loop.h"
 #include "base/string_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
@@ -19,6 +21,34 @@
 #include "ui/message_center/message_center_switches.h"
 #include "ui/message_center/message_center_util.h"
 
+class TestAddObserver : public message_center::MessageCenterObserver {
+ public:
+  TestAddObserver(const std::string& id,
+                  message_center::MessageCenter* message_center)
+      : id_(id), message_center_(message_center) {
+    quit_closure_ = run_loop_.QuitClosure();
+    message_center_->AddObserver(this);
+  }
+
+  virtual ~TestAddObserver() { message_center_->RemoveObserver(this); }
+
+  virtual void OnNotificationAdded(const std::string& id) OVERRIDE {
+    log_ += "_" + id;
+    if (id == id_)
+      base::MessageLoop::current()->PostTask(FROM_HERE, quit_closure_);
+  }
+
+  void Run() { run_loop_.Run(); }
+  const std::string log() const { return log_; }
+
+ private:
+  std::string id_;
+  std::string log_;
+  message_center::MessageCenter* message_center_;
+  base::RunLoop run_loop_;
+  base::Closure quit_closure_;
+};
+
 class MessageCenterNotificationsTest : public InProcessBrowserTest {
  public:
   MessageCenterNotificationsTest() {}
@@ -199,4 +229,34 @@ IN_PROC_BROWSER_TEST_F(MessageCenterNotificationsTest,
   delegate2->Release();
 }
 
+// MessaceCenter-specific test.
+#if defined(RUN_MESSAGE_CENTER_TESTS)
+#define MAYBE_QueueWhenCenterVisible QueueWhenCenterVisible
+#else
+#define MAYBE_QueueWhenCenterVisible DISABLED_QueueWhenCenterVisible
+#endif
+
+IN_PROC_BROWSER_TEST_F(MessageCenterNotificationsTest,
+                       MAYBE_QueueWhenCenterVisible) {
+  EXPECT_TRUE(NotificationUIManager::DelegatesToMessageCenter());
+  TestAddObserver observer("n2", message_center());
+
+  TestDelegate* delegate;
+  TestDelegate* delegate2;
+
+  manager()->Add(CreateTestNotification("n", &delegate), profile());
+  message_center()->SetMessageCenterVisible(true);
+  manager()->Add(CreateTestNotification("n2", &delegate2), profile());
+
+  EXPECT_EQ("_n", observer.log());
+
+  message_center()->SetMessageCenterVisible(false);
+  observer.Run();
+
+  EXPECT_EQ("_n_n2", observer.log());
+
+  delegate->Release();
+  delegate2->Release();
+}
+
 #endif  // !defined(OS_MACOSX)
diff --git a/ui/message_center/fake_message_center.cc b/ui/message_center/fake_message_center.cc
@@ -124,4 +124,8 @@ void FakeMessageCenter::EnterQuietModeWithExpire(
 void FakeMessageCenter::SetMessageCenterVisible(bool visible) {
 }
 
+bool FakeMessageCenter::IsMessageCenterVisible() {
+  return false;
+}
+
 }  // namespace message_center
diff --git a/ui/message_center/fake_message_center.h b/ui/message_center/fake_message_center.h
@@ -61,6 +61,7 @@ class FakeMessageCenter : public MessageCenter {
   virtual void EnterQuietModeWithExpire(
       const base::TimeDelta& expires_in) OVERRIDE;
   virtual void SetMessageCenterVisible(bool visible) OVERRIDE;
+  virtual bool IsMessageCenterVisible() OVERRIDE;
 
  private:
   const NotificationList::Notifications empty_notifications_;

diff --git a/ui/message_center/message_center.h b/ui/message_center/message_center.h
@@ -138,6 +138,9 @@ class MESSAGE_CENTER_EXPORT MessageCenter {
   // This affects whether or not a message has been "read".
   virtual void SetMessageCenterVisible(bool visible) = 0;
 
+  // Allows querying the visibility of the center.
+  virtual bool IsMessageCenterVisible() = 0;
+
  protected:
   MessageCenter();
   virtual ~MessageCenter();

diff --git a/ui/message_center/message_center_impl.cc b/ui/message_center/message_center_impl.cc
@@ -41,6 +41,15 @@ void MessageCenterImpl::SetMessageCenterVisible(bool visible) {
     FOR_EACH_OBSERVER(MessageCenterObserver, observer_list_,
                       OnNotificationUpdated(*iter));
   }
+
+  if (!visible) {
+    FOR_EACH_OBSERVER(
+        MessageCenterObserver, observer_list_, OnNotificationCenterClosed());
+  }
+}
+
+bool MessageCenterImpl::IsMessageCenterVisible() {
+  return notification_list_->is_message_center_visible();
 }
 
 size_t MessageCenterImpl::NotificationCount() const {

diff --git a/ui/message_center/message_center_impl.h b/ui/message_center/message_center_impl.h
@@ -24,6 +24,7 @@ class MessageCenterImpl : public MessageCenter {
   virtual void AddObserver(MessageCenterObserver* observer) OVERRIDE;
   virtual void RemoveObserver(MessageCenterObserver* observer) OVERRIDE;
   virtual void SetMessageCenterVisible(bool visible) OVERRIDE;
+  virtual bool IsMessageCenterVisible() OVERRIDE;
   virtual size_t NotificationCount() const OVERRIDE;
   virtual size_t UnreadNotificationCount() const OVERRIDE;
   virtual bool HasPopupNotifications() const OVERRIDE;

diff --git a/ui/message_center/message_center_observer.h b/ui/message_center/message_center_observer.h
@@ -41,6 +41,10 @@ class MESSAGE_CENTER_EXPORT MessageCenterObserver {
   // Called when the notification associated with |notification_id| is actually
   // displayed.
   virtual void OnNotificationDisplayed(const std::string& notification_id) {}
+
+  // Called when the notification list is no longer being displayed as a
+  // notification center.
+  virtual void OnNotificationCenterClosed() {}
 };
 
 }  // namespace message_center

diff --git a/ui/message_center/notification_list.h b/ui/message_center/notification_list.h
@@ -126,6 +126,7 @@ class MESSAGE_CENTER_EXPORT NotificationList {
   const Notifications& GetNotifications();
   size_t NotificationCount() const;
   size_t unread_count() const { return unread_count_; }
+  bool is_message_center_visible() const { return message_center_visible_; }
 
  private:
   friend class test::NotificationListTest;