Skip to content

Commit

Permalink
[Notification] Enable changes while the message center opens
Browse files Browse the repository at this point in the history
Previously, while the message center opens, almost all changes are stored into the queue and delayed until the message center closes due to lack of implementations of relayout. This patch introduces relayout and make cush changes enabled.

BUG=525771, 372422
TEST=unit_tests, browser_tests passes

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

Cr-Commit-Position: refs/heads/master@{#351309}
  • Loading branch information
yoshikig authored and Commit bot committed Sep 29, 2015
1 parent dcd73e5 commit 91af408
Show file tree
Hide file tree
Showing 14 changed files with 504 additions and 85 deletions.
2 changes: 2 additions & 0 deletions ui/message_center/fake_message_center.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,6 @@ void FakeMessageCenter::PausePopupTimers() {}

void FakeMessageCenter::DisableTimersForTest() {}

void FakeMessageCenter::DisableChangeQueueForTest() {}

} // namespace message_center
1 change: 1 addition & 0 deletions ui/message_center/fake_message_center.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class FakeMessageCenter : public MessageCenter {

protected:
void DisableTimersForTest() override;
void DisableChangeQueueForTest() override;

private:
const NotificationList::Notifications empty_notifications_;
Expand Down
2 changes: 2 additions & 0 deletions ui/message_center/message_center.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,11 @@ class MESSAGE_CENTER_EXPORT MessageCenter {
protected:
friend class ::DownloadNotification;
friend class MessageCenterImplTest;
friend class MessageCenterImplTestWithoutChangeQueue;
friend class TrayViewControllerTest;
friend class test::MessagePopupCollectionTest;
virtual void DisableTimersForTest() = 0;
virtual void DisableChangeQueueForTest() = 0;

MessageCenter();
virtual ~MessageCenter();
Expand Down
46 changes: 34 additions & 12 deletions ui/message_center/message_center_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
#include <algorithm>
#include <deque>

#include "base/command_line.h"
#include "base/memory/scoped_vector.h"
#include "base/observer_list.h"
#include "ui/message_center/message_center_style.h"
#include "ui/message_center/message_center_switches.h"
#include "ui/message_center/message_center_types.h"
#include "ui/message_center/notification.h"
#include "ui/message_center/notification_blocker.h"
Expand Down Expand Up @@ -351,7 +353,11 @@ MessageCenterImpl::MessageCenterImpl()
popup_timers_controller_(new PopupTimersController(this)),
settings_provider_(NULL) {
notification_list_.reset(new NotificationList());
notification_queue_.reset(new internal::ChangeQueue());

if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableMessageCenterChangesWhileOpen)) {
notification_queue_.reset(new internal::ChangeQueue());
}
}

MessageCenterImpl::~MessageCenterImpl() {
Expand Down Expand Up @@ -430,8 +436,10 @@ void MessageCenterImpl::SetVisibility(Visibility visibility) {
MessageCenterObserver, observer_list_, OnNotificationUpdated(id));
}

if (visibility == VISIBILITY_TRANSIENT)
if (notification_queue_ &&
visibility == VISIBILITY_TRANSIENT) {
notification_queue_->ApplyChanges(this);
}

FOR_EACH_OBSERVER(MessageCenterObserver,
observer_list_,
Expand Down Expand Up @@ -481,7 +489,8 @@ NotificationList::PopupNotifications
}

void MessageCenterImpl::ForceNotificationFlush(const std::string& id) {
notification_queue_->ApplyChangesForId(this, id);
if (notification_queue_)
notification_queue_->ApplyChangesForId(this, id);
}

//------------------------------------------------------------------------------
Expand All @@ -492,7 +501,8 @@ void MessageCenterImpl::AddNotification(scoped_ptr<Notification> notification) {
for (size_t i = 0; i < blockers_.size(); ++i)
blockers_[i]->CheckState();

if (notification_list_->is_message_center_visible()) {
if (notification_queue_ &&
notification_list_->is_message_center_visible()) {
notification_queue_->AddNotification(notification.Pass());
return;
}
Expand Down Expand Up @@ -527,7 +537,8 @@ void MessageCenterImpl::UpdateNotification(
for (size_t i = 0; i < blockers_.size(); ++i)
blockers_[i]->CheckState();

if (notification_list_->is_message_center_visible()) {
if (notification_queue_ &&
notification_list_->is_message_center_visible()) {
// We will allow notifications that are progress types (and stay progress
// types) to be updated even if the message center is open. There are 3
// requirements here:
Expand Down Expand Up @@ -574,7 +585,8 @@ void MessageCenterImpl::UpdateNotificationImmediately(

void MessageCenterImpl::RemoveNotification(const std::string& id,
bool by_user) {
if (!by_user && notification_list_->is_message_center_visible()) {
if (notification_queue_ && !by_user &&
notification_list_->is_message_center_visible()) {
notification_queue_->EraseNotification(id, by_user);
return;
}
Expand Down Expand Up @@ -655,8 +667,10 @@ void MessageCenterImpl::RemoveNotifications(
void MessageCenterImpl::SetNotificationIcon(const std::string& notification_id,
const gfx::Image& image) {
bool updated = false;
Notification* queue_notification = notification_queue_->GetLatestNotification(
notification_id);
Notification* queue_notification =
notification_queue_
? notification_queue_->GetLatestNotification(notification_id)
: NULL;

if (queue_notification) {
queue_notification->set_icon(image);
Expand All @@ -674,8 +688,10 @@ void MessageCenterImpl::SetNotificationIcon(const std::string& notification_id,
void MessageCenterImpl::SetNotificationImage(const std::string& notification_id,
const gfx::Image& image) {
bool updated = false;
Notification* queue_notification = notification_queue_->GetLatestNotification(
notification_id);
Notification* queue_notification =
notification_queue_
? notification_queue_->GetLatestNotification(notification_id)
: NULL;

if (queue_notification) {
queue_notification->set_image(image);
Expand All @@ -694,8 +710,10 @@ void MessageCenterImpl::SetNotificationButtonIcon(
const std::string& notification_id, int button_index,
const gfx::Image& image) {
bool updated = false;
Notification* queue_notification = notification_queue_->GetLatestNotification(
notification_id);
Notification* queue_notification =
notification_queue_
? notification_queue_->GetLatestNotification(notification_id)
: NULL;

if (queue_notification) {
queue_notification->SetButtonIcon(button_index, image);
Expand Down Expand Up @@ -848,4 +866,8 @@ void MessageCenterImpl::DisableTimersForTest() {
popup_timers_controller_.reset();
}

void MessageCenterImpl::DisableChangeQueueForTest() {
notification_queue_.reset();
}

} // namespace message_center
1 change: 1 addition & 0 deletions ui/message_center/message_center_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class MessageCenterImpl : public MessageCenter,

protected:
void DisableTimersForTest() override;
void DisableChangeQueueForTest() override;

private:
struct NotificationCache {
Expand Down
68 changes: 68 additions & 0 deletions ui/message_center/message_center_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ class MessageCenterImplTest : public testing::Test,
DISALLOW_COPY_AND_ASSIGN(MessageCenterImplTest);
};

class MessageCenterImplTestWithoutChangeQueue : public MessageCenterImplTest {
public:
MessageCenterImplTestWithoutChangeQueue() {}
~MessageCenterImplTestWithoutChangeQueue() override {}

void SetUp() override {
MessageCenterImplTest::SetUp();
message_center()->DisableChangeQueueForTest();
}

private:
DISALLOW_COPY_AND_ASSIGN(MessageCenterImplTestWithoutChangeQueue);
};

namespace {

class ToggledNotificationBlocker : public NotificationBlocker {
Expand Down Expand Up @@ -1093,5 +1107,59 @@ TEST_F(MessageCenterImplTest, NotifierEnabledChanged) {
ASSERT_EQ(0u, message_center()->NotificationCount());
}

TEST_F(MessageCenterImplTestWithoutChangeQueue,
UpdateWhileMessageCenterVisible) {
std::string id("id1");
std::string id2("id2");
NotifierId notifier_id1(NotifierId::APPLICATION, "app1");

// First, add and update a notification to ensure updates happen
// normally.
scoped_ptr<Notification> notification(CreateSimpleNotification(id));
message_center()->AddNotification(notification.Pass());
notification.reset(CreateSimpleNotification(id2));
message_center()->UpdateNotification(id, notification.Pass());
EXPECT_TRUE(message_center()->FindVisibleNotificationById(id2));
EXPECT_FALSE(message_center()->FindVisibleNotificationById(id));

// Then open the message center.
message_center()->SetVisibility(VISIBILITY_MESSAGE_CENTER);

// Then update a notification; the update should have propagated.
notification.reset(CreateSimpleNotification(id));
message_center()->UpdateNotification(id2, notification.Pass());
EXPECT_FALSE(message_center()->FindVisibleNotificationById(id2));
EXPECT_TRUE(message_center()->FindVisibleNotificationById(id));
}

TEST_F(MessageCenterImplTestWithoutChangeQueue, AddWhileMessageCenterVisible) {
std::string id("id1");

// Then open the message center.
message_center()->SetVisibility(VISIBILITY_MESSAGE_CENTER);

// Add a notification and confirm the adding should have propagated.
scoped_ptr<Notification> notification(CreateSimpleNotification(id));
message_center()->AddNotification(notification.Pass());
EXPECT_TRUE(message_center()->FindVisibleNotificationById(id));
}

TEST_F(MessageCenterImplTestWithoutChangeQueue,
RemoveWhileMessageCenterVisible) {
std::string id("id1");

// First, add a notification to ensure updates happen normally.
scoped_ptr<Notification> notification(CreateSimpleNotification(id));
message_center()->AddNotification(notification.Pass());
EXPECT_TRUE(message_center()->FindVisibleNotificationById(id));

// Then open the message center.
message_center()->SetVisibility(VISIBILITY_MESSAGE_CENTER);

// Then update a notification; the update should have propagated.
message_center()->RemoveNotification(id, false);
EXPECT_FALSE(message_center()->FindVisibleNotificationById(id));
}

} // namespace internal
} // namespace message_center
5 changes: 5 additions & 0 deletions ui/message_center/message_center_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@

namespace switches {

// Enables notification changes while the message center opens. This flag will
// removed once the feature gets stable.
const char kEnableMessageCenterChangesWhileOpen[] =
"enable-message-center-changes-while-open";

// Enables message center to always move other notifications upwards when a
// notification is removed, no matter whether the message center is displayed
// top down or not.
Expand Down
1 change: 1 addition & 0 deletions ui/message_center/message_center_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

namespace switches {

MESSAGE_CENTER_EXPORT extern const char kEnableMessageCenterChangesWhileOpen[];
MESSAGE_CENTER_EXPORT extern const char
kEnableMessageCenterAlwaysScrollUpUponNotificationRemoval[];

Expand Down
9 changes: 9 additions & 0 deletions ui/message_center/views/message_center_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,15 @@ void MessageCenterView::OnNotificationUpdated(const std::string& id) {
if (view_iter == notification_views_.end())
return;
NotificationView* view = view_iter->second;
// Set the item on the mouse cursor as the reposition target so that it
// should stick to the current position over the update.
for (const auto& hover_id_view : notification_views_) {
NotificationView* hover_view = hover_id_view.second;
if (hover_view->is_hover()) {
message_list_view_->SetRepositionTarget(hover_view->bounds());
break;
}
}
// TODO(dimich): add MessageCenter::GetVisibleNotificationById(id)
const NotificationList::Notifications& notifications =
message_center_->GetVisibleNotifications();
Expand Down
Loading

0 comments on commit 91af408

Please sign in to comment.